From bcab1b199b782fcff21077f68bde6da53c12029a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Noel=20N=C3=A9meth?= Date: Sat, 4 Jun 2022 22:04:57 +0200 Subject: [PATCH] Limit chunk update packets (#1128) --- .../net/minestom/server/entity/Entity.java | 6 +- .../net/minestom/server/entity/Player.java | 24 ++++++- .../server/instance/InstanceContainer.java | 1 - .../utils/chunk/ChunkUpdateLimitChecker.java | 35 +++++++++ .../net/minestom/server/api/Collector.java | 2 +- .../player/PlayerMovementIntegrationTest.java | 71 +++++++++++++++++++ .../minestom/server/utils/ChunkUtilsTest.java | 51 +++++++++++++ 7 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 src/main/java/net/minestom/server/utils/chunk/ChunkUpdateLimitChecker.java create mode 100644 src/test/java/net/minestom/server/utils/ChunkUtilsTest.java diff --git a/src/main/java/net/minestom/server/entity/Entity.java b/src/main/java/net/minestom/server/entity/Entity.java index 7a6b75d04..70c0d2328 100644 --- a/src/main/java/net/minestom/server/entity/Entity.java +++ b/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 - player.sendPacket(new UpdateViewPositionPacket(newChunkX, newChunkZ)); - ChunkUtils.forDifferingChunksInRange(newChunkX, newChunkZ, lastChunkX, lastChunkZ, - MinecraftServer.getChunkViewDistance(), player.chunkAdder, player.chunkRemover); - } + if (this instanceof Player player) player.sendChunkUpdates(newChunk); refreshCurrentChunk(newChunk); } } diff --git a/src/main/java/net/minestom/server/entity/Player.java b/src/main/java/net/minestom/server/entity/Player.java index 8f06effd3..b0229695f 100644 --- a/src/main/java/net/minestom/server/entity/Player.java +++ b/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. */ diff --git a/src/main/java/net/minestom/server/instance/InstanceContainer.java b/src/main/java/net/minestom/server/instance/InstanceContainer.java index c81b50799..a33876125 100644 --- a/src/main/java/net/minestom/server/instance/InstanceContainer.java +++ b/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); } diff --git a/src/main/java/net/minestom/server/utils/chunk/ChunkUpdateLimitChecker.java b/src/main/java/net/minestom/server/utils/chunk/ChunkUpdateLimitChecker.java new file mode 100644 index 000000000..40dd52546 --- /dev/null +++ b/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; + } +} diff --git a/src/test/java/net/minestom/server/api/Collector.java b/src/test/java/net/minestom/server/api/Collector.java index 1c525679d..100284d30 100644 --- a/src/test/java/net/minestom/server/api/Collector.java +++ b/src/test/java/net/minestom/server/api/Collector.java @@ -27,7 +27,7 @@ public interface Collector { default void assertCount(int count) { List 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() { diff --git a/src/test/java/net/minestom/server/entity/player/PlayerMovementIntegrationTest.java b/src/test/java/net/minestom/server/entity/player/PlayerMovementIntegrationTest.java index f4ff119e7..8a6f9a7d3 100644 --- a/src/test/java/net/minestom/server/entity/player/PlayerMovementIntegrationTest.java +++ b/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> 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 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); + } } diff --git a/src/test/java/net/minestom/server/utils/ChunkUtilsTest.java b/src/test/java/net/minestom/server/utils/ChunkUtilsTest.java new file mode 100644 index 000000000..5327ad053 --- /dev/null +++ b/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 n = new HashSet<>(); + final Set 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 actualNew = new ArrayList<>(); + final List 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 sorter = Comparator.comparingInt(ChunkCoordinate::x).thenComparingInt(ChunkCoordinate::z); + final List expectedNew = n.stream().filter(x -> !o.contains(x)).sorted(sorter).toList(); + final List 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 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) {} +}