Fix Entity#takeKnockback and gravity (#943)

src/main/java/net/minestom/server/entity/Entity.java

@@ -549,7 +549,6 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
 
     private void velocityTick() {
         this.gravityTickCount = onGround ? 0 : gravityTickCount + 1;
-        if (PlayerUtils.isSocketClient(this)) return;
         if (vehicle != null) return;
 
         final boolean noGravity = hasNoGravity();
@@ -558,19 +557,19 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
             return;
         }
         final float tps = MinecraftServer.TICK_PER_SECOND;
+        final Pos positionBeforeMove = getPosition();
         final Vec currentVelocity = getVelocity();
-        final Vec deltaPos = new Vec(
+        final boolean wasOnGround = this.onGround;
-                currentVelocity.x() / tps,
+        final Vec deltaPos = currentVelocity.div(tps);
-                currentVelocity.y() / tps - (noGravity ? 0 : gravityAcceleration),
-                currentVelocity.z() / tps
-        );
 
         final Pos newPosition;
         final Vec newVelocity;
         if (this.hasPhysics) {
             final var physicsResult = CollisionUtils.handlePhysics(this, deltaPos, lastPhysicsResult);
             this.lastPhysicsResult = physicsResult;
-            this.onGround = physicsResult.isOnGround();
+            if (!PlayerUtils.isSocketClient(this))
+                this.onGround = physicsResult.isOnGround();
+
             newPosition = physicsResult.newPosition();
             newVelocity = physicsResult.newVelocity();
         } else {
@@ -582,11 +581,12 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
         final Pos finalVelocityPosition = CollisionUtils.applyWorldBorder(instance, position, newPosition);
         final boolean positionChanged = !finalVelocityPosition.samePoint(position);
         if (!positionChanged) {
-            if (!hasVelocity && newVelocity.isZero()) {
+            if (hasVelocity || newVelocity.isZero()) {
-                return;
+                this.velocity = noGravity ? Vec.ZERO : new Vec(
-            }
+                        0,
-            if (hasVelocity) {
+                        -gravityAcceleration * tps * (1 - gravityDragPerTick),
-                this.velocity = Vec.ZERO;
+                        0
+                );
                 sendPacketToViewers(getVelocityPacket());
                 return;
             }
@@ -604,33 +604,45 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
                 this.position = finalVelocityPosition;
                 refreshCoordinate(finalVelocityPosition);
             } else {
-                refreshPosition(finalVelocityPosition, true);
+                if (!PlayerUtils.isSocketClient(this))
+                    refreshPosition(finalVelocityPosition, true);
             }
         }
 
         // Update velocity
         if (hasVelocity || !newVelocity.isZero()) {
-            final double airDrag = this instanceof LivingEntity ? 0.91 : 0.98;
+            updateVelocity(wasOnGround, positionBeforeMove, newVelocity);
-            final double drag = this.onGround ?
-                    finalChunk.getBlock(position).registry().friction() : airDrag;
-            this.velocity = newVelocity
-                    // Convert from block/tick to block/sec
-                    .mul(tps)
-                    // Apply drag
-                    .apply((x, y, z) -> new Vec(
-                            x * drag,
-                            !noGravity ? y * (1 - gravityDragPerTick) : y,
-                            z * drag
-                    ))
-                    // Prevent infinitely decreasing velocity
-                    .apply(Vec.Operator.EPSILON);
         }
         // Verify if velocity packet has to be sent
-        if (hasVelocity || gravityTickCount > 0) {
+        if (!(this instanceof Player) && (hasVelocity || gravityTickCount > 0)) {
             sendPacketToViewers(getVelocityPacket());
         }
     }
 
+    protected void updateVelocity(boolean wasOnGround, Pos positionBeforeMove, Vec newVelocity) {
+        EntitySpawnType type = entityType.registry().spawnType();
+        final double airDrag = type == EntitySpawnType.LIVING || type == EntitySpawnType.PLAYER ? 0.91 : 0.98;
+        final double drag;
+        if (wasOnGround) {
+            final Chunk chunk = ChunkUtils.retrieve(instance, currentChunk, position);
+            synchronized (chunk) {
+                drag = chunk.getBlock(positionBeforeMove.sub(0, 0.5000001, 0)).registry().friction() * airDrag;
+            }
+        } else drag = airDrag;
+
+        this.velocity = newVelocity
+                // Apply drag
+                .apply((x, y, z) -> new Vec(
+                        x * drag,
+                        !hasNoGravity() ? (y - gravityAcceleration) * (1 - gravityDragPerTick) : y,
+                        z * drag
+                ))
+                // Convert from block/tick to block/sec
+                .mul(MinecraftServer.TICK_PER_SECOND)
+                // Prevent infinitely decreasing velocity
+                .apply(Vec.Operator.EPSILON);
+    }
+
     private void touchTick() {
         // TODO do not call every tick (it is pretty expensive)
         final Pos position = this.position;
@@ -1243,14 +1255,7 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
         if (position.equals(lastSyncedPosition)) return;
         this.position = position;
         this.previousPosition = previousPosition;
-        if (!position.samePoint(previousPosition)) {
+        if (!position.samePoint(previousPosition)) refreshCoordinate(position);
-            refreshCoordinate(position);
-            // Update player velocity
-            if (PlayerUtils.isSocketClient(this)) {
-                // Calculate from client
-                this.velocity = position.sub(previousPosition).asVec().mul(MinecraftServer.TICK_PER_SECOND);
-            }
-        }
         // Update viewers
         final boolean viewChange = !position.sameView(lastSyncedPosition);
         final double distanceX = Math.abs(position.x() - lastSyncedPosition.x());
@@ -1576,14 +1581,15 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
      * @param x        knockback on x axle, for default knockback use the following formula <pre>sin(attacker.yaw * (pi/180))</pre>
      * @param z        knockback on z axle, for default knockback use the following formula <pre>-cos(attacker.yaw * (pi/180))</pre>
      */
-    public void takeKnockback(final float strength, final double x, final double z) {
+    public void takeKnockback(float strength, final double x, final double z) {
         if (strength > 0) {
             //TODO check possible side effects of unnatural TPS (other than 20TPS)
-            final Vec velocityModifier = new Vec(x, z)
+            strength *= MinecraftServer.TICK_PER_SECOND;
-                    .normalize()
+            final Vec velocityModifier = new Vec(x, z).normalize().mul(strength);
-                    .mul(strength * MinecraftServer.TICK_PER_SECOND / 2);
+            final double verticalLimit = .4d * MinecraftServer.TICK_PER_SECOND;
+
             setVelocity(new Vec(velocity.x() / 2d - velocityModifier.x(),
-                    onGround ? Math.min(.4d, velocity.y() / 2d + strength) * MinecraftServer.TICK_PER_SECOND : velocity.y(),
+                    onGround ? Math.min(verticalLimit, velocity.y() / 2d + strength) : velocity.y(),
                     velocity.z() / 2d - velocityModifier.z()
             ));
         }

src/test/java/net/minestom/server/entity/EntityVelocityIntegrationTest.java

@@ -0,0 +1,128 @@
+package net.minestom.server.entity;
+
+import net.minestom.server.api.Env;
+import net.minestom.server.api.EnvTest;
+import net.minestom.server.coordinate.Pos;
+import net.minestom.server.coordinate.Vec;
+import net.minestom.server.instance.Instance;
+import net.minestom.server.utils.chunk.ChunkUtils;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+@EnvTest
+public class EntityVelocityIntegrationTest {
+    @Test
+    public void gravity(Env env) {
+        var instance = env.createFlatInstance();
+        loadChunks(instance);
+
+        var entity = new Entity(EntityTypes.ZOMBIE);
+        entity.setInstance(instance, new Pos(0, 42, 0)).join();
+        env.tick(); // Ensure velocity downwards is present
+
+        testMovement(env, entity, new Vec[] {
+                new Vec(0.0, 42.0, 0.0),
+                new Vec(0.0, 41.92159999847412, 0.0),
+                new Vec(0.0, 41.76636799395752, 0.0),
+                new Vec(0.0, 41.53584062504456, 0.0),
+                new Vec(0.0, 41.231523797587016, 0.0),
+                new Vec(0.0, 40.85489329934836, 0.0),
+                new Vec(0.0, 40.40739540236494, 0.0),
+                new Vec(0.0, 40.0, 0.0)
+        });
+    }
+
+    @Test
+    public void singleKnockback(Env env) {
+        var instance = env.createFlatInstance();
+        loadChunks(instance);
+
+        var entity = new Entity(EntityTypes.ZOMBIE);
+        entity.setInstance(instance, new Pos(0, 40, 0)).join();
+        env.tick();
+        env.tick(); // Ensures the entity is onGround
+        entity.takeKnockback(0.4f, 0, -1);
+
+        testMovement(env, entity, new Vec[] {
+                new Vec(0.0, 40.0, 0.0),
+                new Vec(0.0, 40.360800005197525, 0.4000000059604645),
+                new Vec(0.0, 40.63598401564693, 0.6184000345826153),
+                new Vec(0.0, 40.827264349610196, 0.8171440663565412),
+                new Vec(0.0, 40.9363190790167, 0.9980011404830835),
+                new Vec(0.0, 40.96479271438924, 1.1625810826814025),
+                new Vec(0.0, 40.914296876071546, 1.3123488343981535),
+                new Vec(0.0, 40.7864109520312, 1.4486374923882126),
+                new Vec(0.0, 40.58268274250654, 1.5726601747334787),
+                new Vec(0.0, 40.304629091760695, 1.685520818920295),
+                new Vec(0.0, 40.0, 1.7882240080901861),
+                new Vec(0.0, 40.0, 1.8816839129282854),
+                new Vec(0.0, 40.0, 1.9327130268970532),
+                new Vec(0.0, 40.0, 1.9605749263602332),
+                new Vec(0.0, 40.0, 1.9757875252341128),
+                new Vec(0.0, 40.0, 1.9840936051840241),
+                new Vec(0.0, 40.0, 1.9886287253634418),
+                new Vec(0.0, 40.0, 1.9886287253634418),
+        });
+    }
+
+    @Test
+    public void doubleKnockback(Env env) {
+        var instance = env.createFlatInstance();
+        loadChunks(instance);
+
+        var entity = new Entity(EntityTypes.ZOMBIE);
+        entity.setInstance(instance, new Pos(0, 40, 0)).join();
+        env.tick();
+        env.tick(); // Ensures the entity is onGround
+        entity.takeKnockback(0.4f, 0, -1);
+        entity.takeKnockback(0.5f, 0, -1);
+
+        testMovement(env, entity, new Vec[] {
+                new Vec(0.0, 40.0, 0.0),
+                new Vec(0.0, 40.4, 0.7000000029802322),
+                new Vec(0.0, 40.71360000610351, 1.0822000490009787),
+                new Vec(0.0, 40.94252801654052, 1.4300021009034531),
+                new Vec(0.0, 41.088477469609366, 1.7465019772561767),
+                new Vec(0.0, 41.153107934874726, 2.0345168730376946),
+                new Vec(0.0, 41.138045790541625, 2.2966104357523673),
+                new Vec(0.0, 41.04488488728202, 2.5351155846963964),
+                new Vec(0.0, 40.87518719878482, 2.7521552764905097),
+                new Vec(0.0, 40.630483459294965, 2.949661401715245),
+                new Vec(0.0, 40.312273788401676, 3.1293919808495585),
+                new Vec(0.0, 40.0, 3.292946812575406),
+                new Vec(0.0, 40.0, 3.441781713735323),
+                new Vec(0.0, 40.0, 3.523045579207649),
+                new Vec(0.0, 40.0, 3.56741565490924),
+                new Vec(0.0, 40.0, 3.5916417190562298),
+                new Vec(0.0, 40.0, 3.6048691516168874),
+                new Vec(0.0, 40.0, 3.6120913306338815),
+                new Vec(0.0, 40.0, 3.616034640835186)
+        });
+    }
+
+    private void testMovement(Env env, Entity entity, Vec[] sample) {
+        final double epsilon = 0.003;
+        for (Vec vec : sample) {
+            assertEquals(vec.x(), entity.getPosition().x(), epsilon);
+            assertEquals(vec.y(), entity.getPosition().y(), epsilon);
+            assertEquals(vec.z(), entity.getPosition().z(), epsilon);
+            env.tick();
+        }
+    }
+
+    private void loadChunks(Instance instance) {
+        ChunkUtils.optionalLoadAll(instance, new long[] {
+                ChunkUtils.getChunkIndex(-1, -1),
+                ChunkUtils.getChunkIndex(-1, 0),
+                ChunkUtils.getChunkIndex(-1, 1),
+                ChunkUtils.getChunkIndex(0, -1),
+                ChunkUtils.getChunkIndex(0, 0),
+                ChunkUtils.getChunkIndex(0, 1),
+                ChunkUtils.getChunkIndex(1, -1),
+                ChunkUtils.getChunkIndex(1, 0),
+                ChunkUtils.getChunkIndex(1, 1),
+        }, null).join();
+    }
+}