From dea6dee7f9cf22634d062f452c511e8a091233ed Mon Sep 17 00:00:00 2001 From: RaphiMC <50594595+RaphiMC@users.noreply.github.com> Date: Tue, 13 Aug 2024 23:54:23 +0200 Subject: [PATCH] Properly handle block change acknowledgements in 1.18.2 -> 1.19 --- .../providers/BukkitAckSequenceProvider.java | 11 ++++- .../provider/AckSequenceProvider.java | 27 ++++++++++-- .../rewriter/EntityPacketRewriter1_19.java | 11 ++++- .../rewriter/ItemPacketRewriter1_19.java | 43 +++++++++++++++---- .../rewriter/WorldPacketRewriter1_19.java | 33 +++++++++++++- .../storage/SequenceStorage.java | 35 ++++++++++++--- 6 files changed, 138 insertions(+), 22 deletions(-) diff --git a/bukkit/src/main/java/com/viaversion/viaversion/bukkit/providers/BukkitAckSequenceProvider.java b/bukkit/src/main/java/com/viaversion/viaversion/bukkit/providers/BukkitAckSequenceProvider.java index 21e491eb1..a03cb0c32 100644 --- a/bukkit/src/main/java/com/viaversion/viaversion/bukkit/providers/BukkitAckSequenceProvider.java +++ b/bukkit/src/main/java/com/viaversion/viaversion/bukkit/providers/BukkitAckSequenceProvider.java @@ -19,6 +19,7 @@ package com.viaversion.viaversion.bukkit.providers; import com.viaversion.viaversion.ViaVersionPlugin; import com.viaversion.viaversion.api.connection.UserConnection; +import com.viaversion.viaversion.api.minecraft.BlockPosition; import com.viaversion.viaversion.api.protocol.version.ProtocolVersion; import com.viaversion.viaversion.bukkit.tasks.v1_18_2to1_19.AckSequenceTask; import com.viaversion.viaversion.protocols.v1_18_2to1_19.provider.AckSequenceProvider; @@ -33,7 +34,9 @@ public final class BukkitAckSequenceProvider extends AckSequenceProvider { } @Override - public void handleSequence(final UserConnection connection, final int sequence) { + public void handleSequence(final UserConnection connection, final BlockPosition position, final int sequence) { + if (sequence <= 0) return; // Does not need to be acked + final SequenceStorage sequenceStorage = connection.get(SequenceStorage.class); final int previousSequence = sequenceStorage.setSequenceId(sequence); if (previousSequence == -1) { @@ -45,4 +48,10 @@ public final class BukkitAckSequenceProvider extends AckSequenceProvider { } } } + + @Override + public void handleBlockChange(final UserConnection connection, final BlockPosition position) { + // no op on bukkit + } + } diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/provider/AckSequenceProvider.java b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/provider/AckSequenceProvider.java index a1698cdf2..d5c9cb21e 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/provider/AckSequenceProvider.java +++ b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/provider/AckSequenceProvider.java @@ -18,18 +18,37 @@ package com.viaversion.viaversion.protocols.v1_18_2to1_19.provider; import com.viaversion.viaversion.api.connection.UserConnection; +import com.viaversion.viaversion.api.minecraft.BlockPosition; import com.viaversion.viaversion.api.platform.providers.Provider; import com.viaversion.viaversion.api.protocol.packet.PacketWrapper; import com.viaversion.viaversion.api.type.Types; import com.viaversion.viaversion.protocols.v1_18_2to1_19.Protocol1_18_2To1_19; import com.viaversion.viaversion.protocols.v1_18_2to1_19.packet.ClientboundPackets1_19; +import com.viaversion.viaversion.protocols.v1_18_2to1_19.storage.SequenceStorage; public class AckSequenceProvider implements Provider { - public void handleSequence(final UserConnection connection, final int sequence) { - final PacketWrapper ackPacket = PacketWrapper.create(ClientboundPackets1_19.BLOCK_CHANGED_ACK, connection); - ackPacket.write(Types.VAR_INT, sequence); - ackPacket.send(Protocol1_18_2To1_19.class); + public void handleSequence(final UserConnection connection, final BlockPosition position, final int sequence) { + if (sequence <= 0) return; // Does not need to be acked + + if (position == null) { + // Acknowledge immediately if the position isn't known + final PacketWrapper ackPacket = PacketWrapper.create(ClientboundPackets1_19.BLOCK_CHANGED_ACK, connection); + ackPacket.write(Types.VAR_INT, sequence); + ackPacket.send(Protocol1_18_2To1_19.class); + } else { + // Store sequence to be acknowledged when the block change is received + connection.get(SequenceStorage.class).addPendingBlockChange(position, sequence); + } + } + + public void handleBlockChange(final UserConnection connection, final BlockPosition position) { + final int sequence = connection.get(SequenceStorage.class).removePendingBlockChange(position); + if (sequence > 0) { // Acknowledge if pending sequence was found + final PacketWrapper ackPacket = PacketWrapper.create(ClientboundPackets1_19.BLOCK_CHANGED_ACK, connection); + ackPacket.write(Types.VAR_INT, sequence); + ackPacket.scheduleSend(Protocol1_18_2To1_19.class); + } } } diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/EntityPacketRewriter1_19.java b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/EntityPacketRewriter1_19.java index dc336c801..b770529b1 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/EntityPacketRewriter1_19.java +++ b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/EntityPacketRewriter1_19.java @@ -26,6 +26,7 @@ import com.viaversion.nbt.tag.NumberTag; import com.viaversion.viaversion.api.Via; import com.viaversion.viaversion.api.data.ParticleMappings; import com.viaversion.viaversion.api.data.entity.DimensionData; +import com.viaversion.viaversion.api.data.entity.EntityTracker; import com.viaversion.viaversion.api.minecraft.BlockPosition; import com.viaversion.viaversion.api.minecraft.Particle; import com.viaversion.viaversion.api.minecraft.entities.EntityType; @@ -41,6 +42,7 @@ import com.viaversion.viaversion.protocols.v1_17_1to1_18.packet.ClientboundPacke import com.viaversion.viaversion.protocols.v1_18_2to1_19.Protocol1_18_2To1_19; import com.viaversion.viaversion.protocols.v1_18_2to1_19.packet.ClientboundPackets1_19; import com.viaversion.viaversion.protocols.v1_18_2to1_19.storage.DimensionRegistryStorage; +import com.viaversion.viaversion.protocols.v1_18_2to1_19.storage.SequenceStorage; import com.viaversion.viaversion.rewriter.EntityRewriter; import com.viaversion.viaversion.util.Key; import com.viaversion.viaversion.util.Pair; @@ -229,6 +231,13 @@ public final class EntityPacketRewriter1_19 extends EntityRewriter { + final String world = wrapper.get(Types.STRING, 1); + final EntityTracker tracker = tracker(wrapper.user()); + if (tracker.currentWorld() != null && !tracker.currentWorld().equals(world)) { + wrapper.user().get(SequenceStorage.class).clearPendingBlockChanges(); + } + }); handler(worldDataTrackerHandlerByKey()); } }); @@ -349,4 +358,4 @@ public final class EntityPacketRewriter1_19 extends EntityRewriter { + final AckSequenceProvider ackSequenceProvider = Via.getManager().getProviders().get(AckSequenceProvider.class); + final BlockPosition position = wrapper.get(Types.BLOCK_POSITION1_14, 0); + final BlockPosition relativePosition = position.getRelative(getBlockFace(wrapper.get(Types.UNSIGNED_BYTE, 0))); + final int sequence = wrapper.read(Types.VAR_INT); + ackSequenceProvider.handleSequence(wrapper.user(), position, sequence); + ackSequenceProvider.handleSequence(wrapper.user(), relativePosition, sequence); + }); } }); protocol.registerServerbound(ServerboundPackets1_19.USE_ITEM_ON, new PacketHandlers() { @@ -126,25 +134,42 @@ public final class ItemPacketRewriter1_19 extends ItemRewriter { + final AckSequenceProvider ackSequenceProvider = Via.getManager().getProviders().get(AckSequenceProvider.class); + final BlockPosition position = wrapper.get(Types.BLOCK_POSITION1_14, 0); + final BlockPosition relativePosition = position.getRelative(getBlockFace(wrapper.get(Types.VAR_INT, 1))); + final int sequence = wrapper.read(Types.VAR_INT); + ackSequenceProvider.handleSequence(wrapper.user(), position, sequence); + ackSequenceProvider.handleSequence(wrapper.user(), relativePosition, sequence); + }); } }); protocol.registerServerbound(ServerboundPackets1_19.USE_ITEM, new PacketHandlers() { @Override public void register() { map(Types.VAR_INT); // Hand - handler(sequenceHandler()); + handler(wrapper -> { + final AckSequenceProvider ackSequenceProvider = Via.getManager().getProviders().get(AckSequenceProvider.class); + final int sequence = wrapper.read(Types.VAR_INT); + ackSequenceProvider.handleSequence(wrapper.user(), null, sequence); + }); } }); new RecipeRewriter<>(protocol).register(ClientboundPackets1_18.UPDATE_RECIPES); } - private PacketHandler sequenceHandler() { - return wrapper -> { - final int sequence = wrapper.read(Types.VAR_INT); - final AckSequenceProvider provider = Via.getManager().getProviders().get(AckSequenceProvider.class); - provider.handleSequence(wrapper.user(), sequence); + private BlockFace getBlockFace(int direction) { + direction = Math.abs(direction % 6); + return switch (direction) { + case 0 -> BlockFace.BOTTOM; + case 1 -> BlockFace.TOP; + case 2 -> BlockFace.NORTH; + case 3 -> BlockFace.SOUTH; + case 4 -> BlockFace.WEST; + case 5 -> BlockFace.EAST; + default -> throw new IllegalStateException("Unexpected value: " + direction); }; } + } diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/WorldPacketRewriter1_19.java b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/WorldPacketRewriter1_19.java index 98e52c008..a2542784a 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/WorldPacketRewriter1_19.java +++ b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/rewriter/WorldPacketRewriter1_19.java @@ -17,11 +17,16 @@ */ package com.viaversion.viaversion.protocols.v1_18_2to1_19.rewriter; +import com.viaversion.viaversion.api.Via; +import com.viaversion.viaversion.api.minecraft.BlockChangeRecord; +import com.viaversion.viaversion.api.minecraft.BlockPosition; +import com.viaversion.viaversion.api.protocol.remapper.PacketHandlers; import com.viaversion.viaversion.api.type.Types; import com.viaversion.viaversion.api.type.types.chunk.ChunkType1_18; import com.viaversion.viaversion.protocols.v1_17_1to1_18.packet.ClientboundPackets1_18; import com.viaversion.viaversion.protocols.v1_18_2to1_19.Protocol1_18_2To1_19; import com.viaversion.viaversion.protocols.v1_18_2to1_19.packet.ServerboundPackets1_19; +import com.viaversion.viaversion.protocols.v1_18_2to1_19.provider.AckSequenceProvider; import com.viaversion.viaversion.rewriter.BlockRewriter; public final class WorldPacketRewriter1_19 { @@ -29,11 +34,35 @@ public final class WorldPacketRewriter1_19 { public static void register(final Protocol1_18_2To1_19 protocol) { final BlockRewriter blockRewriter = BlockRewriter.for1_14(protocol); blockRewriter.registerBlockEvent(ClientboundPackets1_18.BLOCK_EVENT); - blockRewriter.registerBlockUpdate(ClientboundPackets1_18.BLOCK_UPDATE); - blockRewriter.registerSectionBlocksUpdate(ClientboundPackets1_18.SECTION_BLOCKS_UPDATE); blockRewriter.registerLevelEvent(ClientboundPackets1_18.LEVEL_EVENT, 1010, 2001); blockRewriter.registerLevelChunk1_19(ClientboundPackets1_18.LEVEL_CHUNK_WITH_LIGHT, ChunkType1_18::new); + protocol.registerClientbound(ClientboundPackets1_18.BLOCK_UPDATE, wrapper -> { + final BlockPosition position = wrapper.passthrough(Types.BLOCK_POSITION1_14); + wrapper.write(Types.VAR_INT, protocol.getMappingData().getNewBlockStateId(wrapper.read(Types.VAR_INT))); + Via.getManager().getProviders().get(AckSequenceProvider.class).handleBlockChange(wrapper.user(), position); + }); + protocol.registerClientbound(ClientboundPackets1_18.SECTION_BLOCKS_UPDATE, new PacketHandlers() { + @Override + public void register() { + map(Types.LONG); // Chunk position + map(Types.BOOLEAN); // Suppress light updates + handler(wrapper -> { + final AckSequenceProvider ackSequenceProvider = Via.getManager().getProviders().get(AckSequenceProvider.class); + final long chunkPosition = wrapper.get(Types.LONG, 0); + final int x = (int) ((chunkPosition >> 42) & 0x3FFFFFL) << 4; + final int z = (int) ((chunkPosition >> 20) & 0x3FFFFFL) << 4; + final int y = (int) (chunkPosition & 0xFFFL) << 4; + + for (BlockChangeRecord record : wrapper.passthrough(Types.VAR_LONG_BLOCK_CHANGE_ARRAY)) { + record.setBlockId(protocol.getMappingData().getNewBlockStateId(record.getBlockId())); + final BlockPosition position = new BlockPosition(x + record.getSectionX(), y + record.getSectionY(), z + record.getSectionZ()); + ackSequenceProvider.handleBlockChange(wrapper.user(), position); + } + }); + } + }); + protocol.cancelClientbound(ClientboundPackets1_18.BLOCK_BREAK_ACK); protocol.registerServerbound(ServerboundPackets1_19.SET_BEACON, wrapper -> { diff --git a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/storage/SequenceStorage.java b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/storage/SequenceStorage.java index 720cf5435..9513828aa 100644 --- a/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/storage/SequenceStorage.java +++ b/common/src/main/java/com/viaversion/viaversion/protocols/v1_18_2to1_19/storage/SequenceStorage.java @@ -18,17 +18,18 @@ package com.viaversion.viaversion.protocols.v1_18_2to1_19.storage; import com.viaversion.viaversion.api.connection.StorableObject; +import com.viaversion.viaversion.api.minecraft.BlockPosition; +import it.unimi.dsi.fastutil.objects.Object2IntLinkedOpenHashMap; +import it.unimi.dsi.fastutil.objects.Object2IntSortedMap; public final class SequenceStorage implements StorableObject { + // Bukkit fix private final Object lock = new Object(); private int sequenceId = -1; - public int sequenceId() { - synchronized (lock) { - return sequenceId; - } - } + // Protocol level fix + private final Object2IntSortedMap pendingBlockChanges = new Object2IntLinkedOpenHashMap<>(); public int setSequenceId(final int sequenceId) { synchronized (lock) { @@ -37,4 +38,28 @@ public final class SequenceStorage implements StorableObject { return previousSequence; } } + + public void addPendingBlockChange(final BlockPosition position, final int id) { + final int lastSequence = this.pendingBlockChanges.isEmpty() ? 0 : this.pendingBlockChanges.getInt(this.pendingBlockChanges.lastKey()); + if (id > 0 && id >= lastSequence && !this.pendingBlockChanges.containsKey(position)) { + // Store the last 200 pending block changes. Some may never get acked, so we need to limit the size. + while (this.pendingBlockChanges.size() > 200) { + this.pendingBlockChanges.removeInt(this.pendingBlockChanges.firstKey()); + } + this.pendingBlockChanges.put(position, id); + } + } + + public int removePendingBlockChange(final BlockPosition position) { + final int ackedSequence = this.pendingBlockChanges.getInt(position); // 0 if not found + if (ackedSequence > 0) { + this.pendingBlockChanges.values().removeIf(sequence -> sequence <= ackedSequence); // Remove all previous pending changes + } + return ackedSequence; + } + + public void clearPendingBlockChanges() { + this.pendingBlockChanges.clear(); + } + }