Limit chunk update packets (#1128)

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

@@ -1381,11 +1381,7 @@ public class Entity implements Viewable, Tickable, Schedulable, Snapshotable, Ev
             // Entity moved in a new chunk
             final Chunk newChunk = instance.getChunk(newChunkX, newChunkZ);
             Check.notNull(newChunk, "The entity {0} tried to move in an unloaded chunk at {1}", getEntityId(), newPosition);
-            if (this instanceof Player player) { // Update visible chunks
+            if (this instanceof Player player) player.sendChunkUpdates(newChunk);
-                player.sendPacket(new UpdateViewPositionPacket(newChunkX, newChunkZ));
-                ChunkUtils.forDifferingChunksInRange(newChunkX, newChunkZ, lastChunkX, lastChunkZ,
-                        MinecraftServer.getChunkViewDistance(), player.chunkAdder, player.chunkRemover);
-            }
             refreshCurrentChunk(newChunk);
         }
     }

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

@@ -72,6 +72,7 @@ import net.minestom.server.timer.Scheduler;
 import net.minestom.server.utils.MathUtils;
 import net.minestom.server.utils.PacketUtils;
 import net.minestom.server.utils.async.AsyncUtils;
+import net.minestom.server.utils.chunk.ChunkUpdateLimitChecker;
 import net.minestom.server.utils.chunk.ChunkUtils;
 import net.minestom.server.utils.function.IntegerBiConsumer;
 import net.minestom.server.utils.identity.NamedAndIdentified;
@@ -122,6 +123,11 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
 
     private DimensionType dimensionType;
     private GameMode gameMode;
+    /**
+     * Keeps track of what chunks are sent to the client, this defines the center of the loaded area
+     * in the range of {@link MinecraftServer#getChunkViewDistance()}
+     */
+    private Vec chunksLoadedByClient = Vec.ZERO;
     final IntegerBiConsumer chunkAdder = (chunkX, chunkZ) -> {
         // Load new chunks
         this.instance.loadOptionalChunk(chunkX, chunkZ).thenAccept(chunk -> {
@@ -168,6 +174,7 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
 
     // Game state (https://wiki.vg/Protocol#Change_Game_State)
     private boolean enableRespawnScreen;
+    private final ChunkUpdateLimitChecker chunkUpdateLimitChecker = new ChunkUpdateLimitChecker(6);
 
     // Experience orb pickup
     protected Cooldown experiencePickupCooldown = new Cooldown(Duration.of(10, TimeUnit.SERVER_TICK));
@@ -613,7 +620,10 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
         super.setInstance(instance, spawnPosition);
 
         if (updateChunks) {
-            sendPacket(new UpdateViewPositionPacket(spawnPosition.chunkX(), spawnPosition.chunkZ()));
+            final int chunkX = spawnPosition.chunkX();
+            final int chunkZ = spawnPosition.chunkZ();
+            chunksLoadedByClient = new Vec(chunkX, chunkZ);
+            sendPacket(new UpdateViewPositionPacket(chunkX, chunkZ));
             ChunkUtils.forChunksInRange(spawnPosition, MinecraftServer.getChunkViewDistance(), chunkAdder);
         }
 
@@ -2022,6 +2032,18 @@ public class Player extends LivingEntity implements CommandSender, Localizable,
         return this;
     }
 
+    protected void sendChunkUpdates(Chunk newChunk) {
+        if (chunkUpdateLimitChecker.addToHistory(newChunk)) {
+            final int newX = newChunk.getChunkX();
+            final int newZ = newChunk.getChunkZ();
+            final Vec old = chunksLoadedByClient;
+            sendPacket(new UpdateViewPositionPacket(newX, newZ));
+            ChunkUtils.forDifferingChunksInRange(newX, newZ, (int) old.x(), (int) old.z(),
+                    MinecraftServer.getChunkViewDistance(), chunkAdder, chunkRemover);
+            this.chunksLoadedByClient = new Vec(newX, newZ);
+        }
+    }
+
     /**
      * Represents the main or off hand of the player.
      */

src/main/java/net/minestom/server/instance/InstanceContainer.java

@@ -339,7 +339,6 @@ public class InstanceContainer extends Instance {
                     MinecraftServer.getExceptionManager().handleException(e);
                 } finally {
                     // End generation
-                    chunk.sendChunk();
                     refreshLastBlockChangeTime();
                     resultFuture.complete(chunk);
                 }

src/main/java/net/minestom/server/utils/chunk/ChunkUpdateLimitChecker.java

@@ -0,0 +1,35 @@
+package net.minestom.server.utils.chunk;
+
+import net.minestom.server.instance.Chunk;
+import org.jetbrains.annotations.ApiStatus;
+
+@ApiStatus.Internal
+public final class ChunkUpdateLimitChecker {
+    private final int historySize;
+    private final long[] chunkHistory;
+
+    public ChunkUpdateLimitChecker(int historySize) {
+        this.historySize = historySize;
+        this.chunkHistory = new long[historySize];
+    }
+
+    /**
+     * Adds the chunk to the history
+     *
+     * @param chunk chunk to add
+     * @return {@code true} if it's a new chunk in the history
+     */
+    public boolean addToHistory(Chunk chunk) {
+        final long index = ChunkUtils.getChunkIndex(chunk);
+        boolean result = true;
+        final int lastIndex = historySize - 1;
+        for (int i = 0; i < lastIndex; i++) {
+            if (chunkHistory[i] == index) {
+                result = false;
+            }
+            chunkHistory[i] = chunkHistory[i + 1];
+        }
+        chunkHistory[lastIndex] = index;
+        return result;
+    }
+}

src/test/java/net/minestom/server/api/Collector.java

@@ -27,7 +27,7 @@ public interface Collector<T> {
 
     default void assertCount(int count) {
         List<T> elements = collect();
-        assertEquals(count, elements.size(), "Expected " + count + " element(s), got " + elements);
+        assertEquals(count, elements.size(), "Expected " + count + " element(s), got " + elements.size() + ": " + elements);
     }
 
     default void assertSingle() {

src/test/java/net/minestom/server/entity/player/PlayerMovementIntegrationTest.java

@@ -1,13 +1,30 @@
 package net.minestom.server.entity.player;
 
+import net.minestom.server.MinecraftServer;
+import net.minestom.server.api.Collector;
 import net.minestom.server.api.Env;
 import net.minestom.server.api.EnvTest;
+import net.minestom.server.api.TestConnection;
 import net.minestom.server.coordinate.Pos;
+import net.minestom.server.coordinate.Vec;
+import net.minestom.server.entity.Player;
+import net.minestom.server.instance.Chunk;
+import net.minestom.server.instance.Instance;
 import net.minestom.server.network.packet.client.play.ClientPlayerPositionPacket;
 import net.minestom.server.network.packet.client.play.ClientTeleportConfirmPacket;
+import net.minestom.server.network.packet.server.play.ChunkDataPacket;
 import net.minestom.server.network.packet.server.play.EntityPositionPacket;
+import net.minestom.server.utils.MathUtils;
+import net.minestom.server.utils.chunk.ChunkUtils;
+import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.Test;
 
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Future;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 @EnvTest
@@ -45,4 +62,58 @@ public class PlayerMovementIntegrationTest {
         // Position update should only be sent once per tick independently of the number of packets
         tracker.assertSingle();
     }
+
+    @Test
+    public void chunkUpdateDebounceTest(Env env) {
+        final Instance flatInstance = env.createFlatInstance();
+        final int viewDiameter = MinecraftServer.getChunkViewDistance() * 2 + 1;
+        // Preload all possible chunks to avoid issues due to async loading
+        Set<CompletableFuture<Chunk>> chunks = new HashSet<>();
+        ChunkUtils.forChunksInRange(0, 0, viewDiameter+2, (x, z) -> chunks.add(flatInstance.loadChunk(x, z)));
+        CompletableFuture.allOf(chunks.toArray(CompletableFuture[]::new)).join();
+        final TestConnection connection = env.createConnection();
+        final CompletableFuture<@NotNull Player> future = connection.connect(flatInstance, new Pos(0.5, 40, 0.5));
+        Collector<ChunkDataPacket> chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        final Player player = future.join();
+        // Initial join
+        chunkDataPacketCollector.assertCount(MathUtils.square(viewDiameter));
+        player.addPacketToQueue(new ClientTeleportConfirmPacket(player.getLastSentTeleportId()));
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(-0.5, 40, 0.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertCount(viewDiameter);
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(-0.5, 40, -0.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertCount(viewDiameter);
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(0.5, 40, -0.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertCount(viewDiameter);
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(0.5, 40, 0.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertEmpty();
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(0.5, 40, -0.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertEmpty();
+
+        // Move to next chunk
+        chunkDataPacketCollector = connection.trackIncoming(ChunkDataPacket.class);
+        // Abuse the fact that there is no delta check
+        player.addPacketToQueue(new ClientPlayerPositionPacket(new Vec(16.5, 40, -16.5), true));
+        player.interpretPacketQueue();
+        chunkDataPacketCollector.assertCount(viewDiameter * 2 - 1);
+    }
 }

src/test/java/net/minestom/server/utils/ChunkUtilsTest.java

@@ -0,0 +1,51 @@
+package net.minestom.server.utils;
+
+import net.minestom.server.utils.chunk.ChunkUtils;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.api.Assertions;
+
+import java.util.*;
+import java.util.stream.Stream;
+
+public class ChunkUtilsTest {
+
+    @ParameterizedTest
+    @MethodSource("testForDifferingChunksInRangeParams")
+    public void testForDifferingChunksInRange(int nx, int nz, int ox, int oz, int r) {
+        final Set<ChunkCoordinate> n = new HashSet<>();
+        final Set<ChunkCoordinate> o = new HashSet<>();
+        ChunkUtils.forChunksInRange(nx, nz, r, (x, z) -> n.add(new ChunkCoordinate(x, z)));
+        ChunkUtils.forChunksInRange(ox, oz, r, (x, z) -> o.add(new ChunkCoordinate(x, z)));
+
+        final List<ChunkCoordinate> actualNew = new ArrayList<>();
+        final List<ChunkCoordinate> actualOld = new ArrayList<>();
+        ChunkUtils.forDifferingChunksInRange(nx, nz, ox, oz, r, ((x, z) -> actualNew.add(new ChunkCoordinate(x, z))),
+                ((x, z) -> actualOld.add(new ChunkCoordinate(x, z))));
+
+        final Comparator<ChunkCoordinate> sorter = Comparator.comparingInt(ChunkCoordinate::x).thenComparingInt(ChunkCoordinate::z);
+        final List<ChunkCoordinate> expectedNew = n.stream().filter(x -> !o.contains(x)).sorted(sorter).toList();
+        final List<ChunkCoordinate> expectedOld = o.stream().filter(x -> !n.contains(x)).sorted(sorter).toList();
+
+        Assertions.assertIterableEquals(expectedNew, actualNew.stream().sorted(sorter).toList());
+        Assertions.assertIterableEquals(expectedOld, actualOld.stream().sorted(sorter).toList());
+    }
+
+    private static Stream<Arguments> testForDifferingChunksInRangeParams() {
+        return Stream.of(
+                Arguments.of(1, 0, 0, 0, 16),
+                Arguments.of(1, 1, 0, 0, 16),
+                Arguments.of(3, 1, 1, 0, 16),
+                Arguments.of(10, 1, 3, 5, 16),
+                Arguments.of(10, 10, -10, -10, 16),
+                Arguments.of(1, 0, 0, 0, 3),
+                Arguments.of(1, 1, 0, 0, 3),
+                Arguments.of(3, 1, 1, 0, 3),
+                Arguments.of(10, 1, 3, 5, 3),
+                Arguments.of(10, 10, -10, -10, 3)
+        );
+    }
+
+    private record ChunkCoordinate(int x, int z) {}
+}