Async Knockback and Hit Detection

[Rastrian]

Description

Added structures to enable async knockback and hit detection modifications. This basically is used by most of bigger PvP servers, pratically just change the behavior of how hits and knockback packets will perform in network manager.

• [X] Async Knockback.
• [X] Async Hit Detection.
• [X] Adjustable TPS of Knockback and Hit Detection threads.
• [X] Support to /tps.

How has this been tested?

Tested PVP In-game, you can notice some improvement above 60-80 ms. Help pvp servers with high player count pvps also, with a good performance above other packets.

If you don't understand how TPS relation works in minecraft, i recommend you watch this video:

You can see also an spigot discussion about this: https://www.spigotmc.org/threads/over-20-tps.95756/ (i have commented in this 6y ago kekw)

Additional Comments

This project is a draft, and need some testing, if you wan't to contribute with some code review, it will help me a lot. You can get an pre-compiled jar with changes clicking here, to access our github actions.

Checklist:

• [x] I have reviewed my code thoroughly.
• [x] I have tested my code.
• [x] My changes generate no new warnings

[andreasdc]

Is it better to send packet from hit thread or pass it to netty?

[Rastrian]

Is it better to send packet from hit thread or pass it to netty?

The idea is inject the packet into hit thread and them flush it into netty, like an priority packet. You can see this better here: https://github.com/Argarian-Network/NachoSpigot/blob/async-kb-hit/NachoSpigot-Server/src/main/java/me/rastrian/dev/threads/netty/Spigot404Write.java#L29

[andreasdc]

Is it better to send packet from hit thread or pass it to netty?

The idea is inject the packet into hit thread and them flush it into netty, like an priority packet. You can see this better here: https://github.com/Argarian-Network/NachoSpigot/blob/async-kb-hit/NachoSpigot-Server/src/main/java/me/rastrian/dev/threads/netty/Spigot404Write.java#L29

What's the sense in that?

[Rastrian]

Is it better to send packet from hit thread or pass it to netty?

The idea is inject the packet into hit thread and them flush it into netty, like an priority packet. You can see this better here: https://github.com/Argarian-Network/NachoSpigot/blob/async-kb-hit/NachoSpigot-Server/src/main/java/me/rastrian/dev/threads/netty/Spigot404Write.java#L29

What's the sense in that?

Re-order the packets to make knockback / hit detection prioritary. As I said, many PvP servers do this, like CosmicPvP.

This also can be replicated in other kind of packets, i was talking with @Lucaskyy today in the possibility to apply async movement improvements (+ fix getCubes and other related behaviors), but if I do this, gonna be in other PR.

[kevstfnl]

PacketPlayInUseEntity is already async, see "instant-interaction" in nacho.yml.

[Rastrian]

PacketPlayInUseEntity is already async, see "instant-interaction" in nacho.yml.

There's just a main difference here, your implementation apply into the main thread, not to an dedicated thread with netty packet injection/priority. It's not wrong, but it's less interactive with performing methods.

I'm integrating now with tps command, for server monitoring the usage of these threads, and you can setup an custom tps value (instead of the traditional 20 tps).

[kevstfnl]

PacketPlayInUseEntity is already async, see "instant-interaction" in nacho.yml.

There's just a main difference here, your implementation apply into the main thread, not to an dedicated thread with netty packet injection/priority. It's not wrong, but it's less interactive with performing methods.

I'm integrating now with tps command, for server monitoring the usage of these threads, and you can setup an custom tps value (instead of the traditional 20 tps).

Ok, but in terms of performance and the final result for the players, is it really useful to allocate in thread just for knockback ?

[Rastrian]

PacketPlayInUseEntity is already async, see "instant-interaction" in nacho.yml.

There's just a main difference here, your implementation apply into the main thread, not to an dedicated thread with netty packet injection/priority. It's not wrong, but it's less interactive with performing methods. I'm integrating now with tps command, for server monitoring the usage of these threads, and you can setup an custom tps value (instead of the traditional 20 tps).

Ok, but in terms of performance and the final result for the players, is it really useful to allocate in thread just for knockback ?

Yes because i'm not allocating this in Minecraft netty-threads, im creating an parallel thread for process this packet, and injecting this into netty channel. https://github.com/Argarian-Network/NachoSpigot/blob/async-kb-hit/NachoSpigot-Server/src/main/java/me/rastrian/dev/threads/netty/Spigot404Write.java#L29

With a custom ticking time this operation can be faster, but with more resource cost, you can change this config later, but it's adjustable.

[Rastrian]

Added TPS command support. Commit: https://github.com/CobbleSword/NachoSpigot/pull/254/commits/a2828cc992050662be1fe3f3df6efb2736e126a3

[Sculas]

Looks good! I will go to my PC soon and do a proper review! (currently on phone)

[ghost]

Im on phone so it's so bad for code reviewing but do this in the method handle of NetworkManager if (this.isConnected()) { if (NachoConfig.asyncHitDetection && this.g() && packet instanceof PacketPlayInUseEntity && ((PacketPlayInUseEntity)packet).a() == PacketPlayInUseEntity.EnumEntityUseAction.ATTACK) { Nacho.hitDetectionThread.addPacket(packet, this, null); return; } if (NachoConfig.asyncKnockback && this.g() && (packet instanceof PacketPlayOutEntityVelocity || packet instanceof PacketPlayOutPosition || packet instanceof PacketPlayInFlying.PacketPlayInPosition || packet instanceof PacketPlayInFlying)) { Nacho.knockbackThread.addPacket(packet, this, null); return; } this.sendPacketQueue(); this.dispatchPacket(packet, null, Boolean.TRUE); } else {

[Rastrian]

Im on phone so it's so bad for code reviewing but do this in the method handle of NetworkManager if (this.isConnected()) { if (NachoConfig.asyncHitDetection && this.g() && packet instanceof PacketPlayInUseEntity && ((PacketPlayInUseEntity)packet).a() == PacketPlayInUseEntity.EnumEntityUseAction.ATTACK) { Nacho.hitDetectionThread.addPacket(packet, this, null); return; } if (NachoConfig.asyncKnockback && this.g() && (packet instanceof PacketPlayOutEntityVelocity || packet instanceof PacketPlayOutPosition || packet instanceof PacketPlayInFlying.PacketPlayInPosition || packet instanceof PacketPlayInFlying)) { Nacho.knockbackThread.addPacket(packet, this, null); return; } this.sendPacketQueue(); this.dispatchPacket(packet, null, Boolean.TRUE); } else {

Why i will do this? The idea it's basically put these functions in another thread. Even if player isn't connected, this doesn't cause any issue.

[ghost]

Im on phone so it's so bad for code reviewing but do this in the method handle of NetworkManager if (this.isConnected()) { if (NachoConfig.asyncHitDetection && this.g() && packet instanceof PacketPlayInUseEntity && ((PacketPlayInUseEntity)packet).a() == PacketPlayInUseEntity.EnumEntityUseAction.ATTACK) { Nacho.hitDetectionThread.addPacket(packet, this, null); return; } if (NachoConfig.asyncKnockback && this.g() && (packet instanceof PacketPlayOutEntityVelocity || packet instanceof PacketPlayOutPosition || packet instanceof PacketPlayInFlying.PacketPlayInPosition || packet instanceof PacketPlayInFlying)) { Nacho.knockbackThread.addPacket(packet, this, null); return; } this.sendPacketQueue(); this.dispatchPacket(packet, null, Boolean.TRUE); } else {

Why i will do this? The idea it's basically put these functions in another thread. Even if player isn't connected, this doesn't cause any issue.

you're doing unnecesary checks to if (this.g()), so moving it to isConnected saves two methods calls (And you're verifying that the player is online)

[Rastrian]

Im on phone so it's so bad for code reviewing but do this in the method handle of NetworkManager if (this.isConnected()) { if (NachoConfig.asyncHitDetection && this.g() && packet instanceof PacketPlayInUseEntity && ((PacketPlayInUseEntity)packet).a() == PacketPlayInUseEntity.EnumEntityUseAction.ATTACK) { Nacho.hitDetectionThread.addPacket(packet, this, null); return; } if (NachoConfig.asyncKnockback && this.g() && (packet instanceof PacketPlayOutEntityVelocity || packet instanceof PacketPlayOutPosition || packet instanceof PacketPlayInFlying.PacketPlayInPosition || packet instanceof PacketPlayInFlying)) { Nacho.knockbackThread.addPacket(packet, this, null); return; } this.sendPacketQueue(); this.dispatchPacket(packet, null, Boolean.TRUE); } else {

Why i will do this? The idea it's basically put these functions in another thread. Even if player isn't connected, this doesn't cause any issue.

you're doing unnecesary checks to if (this.g()), so moving it to isConnected saves two methods calls (And you're verifying that the player is online)

Oh i didn't notice, thx and sorry.

[kevstfnl]

I have big doubts about the performance, but I am now sure that the result is not as good as it was originally. With a simple timmings

Old:

New:

With this, here is the delay in nano seconds between the receipt of the packet playinuseentity and the packet playoutvelocity, we can deduce that the hitdetection is worse with your method than the old one.

[Rastrian]

I have big doubts about the performance, but I am now sure that the result is not as good as it was originally. With a simple timmings

Old:

New:

With this, here is the delay in nano seconds between the receipt of the packet playinuseentity and the packet playoutvelocity, we can deduce that the hitdetection is worse with your method than the old one.

You can explain me how are you testing it (implementation) ? if you want we can discuss in Nacho discord.

When you work with more than 20 tps (in this case, by default, Knockback and Hit Detection has 40 tps by config), your calcs change.

[Sculas]

Why close?

[Rastrian]

Why close?

Reworking.