Fix missing block connection updates on cancelled place/remove (#3359)

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_13to1_12_2/blockconnections/ConnectionData.java

@@ -64,6 +64,8 @@ public final class ConnectionData {
     }
 
     public static void update(UserConnection user, Position position) throws Exception {
+        Boolean inSync = null;
+
         for (BlockFace face : BlockFace.values()) {
             Position pos = position.getRelative(face);
             int blockState = blockConnectionProvider.getBlockData(user, pos.x(), pos.y(), pos.z());
@@ -73,8 +75,14 @@ public final class ConnectionData {
             }
 
             int newBlockState = handler.connect(user, pos, blockState);
-            if (newBlockState == blockState && blockConnectionProvider.storesBlocks()) {
+            if (newBlockState == blockState) {
-                continue;
+                if (inSync == null) {
+                    inSync = blockConnectionProvider.storesBlocks(user, position);
+                }
+                // Blocks-states are the same, and known to be stored and not de-synced, skip update
+                if (inSync) {
+                    continue;
+                }
             }
 
             updateBlockStorage(user, pos.x(), pos.y(), pos.z(), newBlockState);
@@ -100,8 +108,13 @@ public final class ConnectionData {
         blockConnectionProvider.clearStorage(connection);
     }
 
+    public static void markModified(UserConnection connection, Position pos) {
+        if (!needStoreBlocks()) return;
+        blockConnectionProvider.modifiedBlock(connection, pos);
+    }
+
     public static boolean needStoreBlocks() {
-        return blockConnectionProvider.storesBlocks();
+        return blockConnectionProvider.storesBlocks(null, null);
     }
 
     public static void connectBlocks(UserConnection user, Chunk chunk) {
@@ -726,7 +739,7 @@ public final class ConnectionData {
 
             Position pos = new Position(x, y, z);
             int newBlockState = handler.connect(user, pos, blockState);
-            if (blockState != newBlockState || !blockConnectionProvider.storesBlocks()) {
+            if (blockState != newBlockState || !blockConnectionProvider.storesBlocks(user, null)) {
                 records.add(new BlockChangeRecord1_8(x & 0xF, y, z & 0xF, newBlockState));
                 updateBlockStorage(user, x, y, z, newBlockState);
             }

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_13to1_12_2/blockconnections/providers/BlockConnectionProvider.java

@@ -18,8 +18,10 @@
 package com.viaversion.viaversion.protocols.protocol1_13to1_12_2.blockconnections.providers;
 
 import com.viaversion.viaversion.api.connection.UserConnection;
+import com.viaversion.viaversion.api.minecraft.Position;
 import com.viaversion.viaversion.api.platform.providers.Provider;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.Protocol1_13To1_12_2;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class BlockConnectionProvider implements Provider {
 
@@ -44,6 +46,10 @@ public class BlockConnectionProvider implements Provider {
 
     }
 
+    public void modifiedBlock(UserConnection connection, Position position) {
+
+    }
+
     public void unloadChunk(UserConnection connection, int x, int z) {
 
     }
@@ -52,7 +58,14 @@ public class BlockConnectionProvider implements Provider {
 
     }
 
-    public boolean storesBlocks() {
+    /**
+     * True if blocks are stored, and are known to be accurate around the given position.
+     * If the client has modified the position (ie: placed or broken a block) this should return false.
+     *
+     * @param position The position at which block reliability should be checked, null for general-purpose
+     * @return true if the block & its neighbors are known to be correct
+     */
+    public boolean storesBlocks(UserConnection user, @Nullable Position position) {
         return false;
     }
 

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_13to1_12_2/blockconnections/providers/PacketBlockConnectionProvider.java

@@ -18,7 +18,9 @@
 package com.viaversion.viaversion.protocols.protocol1_13to1_12_2.blockconnections.providers;
 
 import com.viaversion.viaversion.api.connection.UserConnection;
+import com.viaversion.viaversion.api.minecraft.Position;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.storage.BlockConnectionStorage;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class PacketBlockConnectionProvider extends BlockConnectionProvider {
 
@@ -42,6 +44,10 @@ public class PacketBlockConnectionProvider extends BlockConnectionProvider {
         connection.get(BlockConnectionStorage.class).clear();
     }
 
+    public void modifiedBlock(UserConnection connection, Position position) {
+        connection.get(BlockConnectionStorage.class).markModified(position);
+    }
+
     @Override
     public void unloadChunk(UserConnection connection, int x, int z) {
         connection.get(BlockConnectionStorage.class).unloadChunk(x, z);
@@ -53,8 +59,10 @@ public class PacketBlockConnectionProvider extends BlockConnectionProvider {
     }
 
     @Override
-    public boolean storesBlocks() {
+    public boolean storesBlocks(UserConnection connection, @Nullable Position pos) {
-        return true;
+        if (pos == null || connection == null) return true;
+
+        return !connection.get(BlockConnectionStorage.class).recentlyModified(pos);
     }
 
     @Override

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_13to1_12_2/packets/WorldPackets.java

@@ -36,6 +36,7 @@ import com.viaversion.viaversion.api.type.types.Particle;
 import com.viaversion.viaversion.protocols.protocol1_12_1to1_12.ClientboundPackets1_12_1;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.ClientboundPackets1_13;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.Protocol1_13To1_12_2;
+import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.ServerboundPackets1_13;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.blockconnections.ConnectionData;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.blockconnections.ConnectionHandler;
 import com.viaversion.viaversion.protocols.protocol1_13to1_12_2.data.NamedSoundRewriter;
@@ -48,6 +49,7 @@ import com.viaversion.viaversion.protocols.protocol1_9_1_2to1_9_3_4.types.Chunk1
 import com.viaversion.viaversion.protocols.protocol1_9_3to1_9_1_2.storage.ClientWorld;
 import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
 import it.unimi.dsi.fastutil.ints.IntSet;
+
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
@@ -535,6 +537,32 @@ public class WorldPackets {
                 });
             }
         });
+
+        // Incoming Packets
+        protocol.registerServerbound(ServerboundPackets1_13.PLAYER_BLOCK_PLACEMENT, wrapper -> {
+            Position pos = wrapper.passthrough(Type.POSITION);
+            wrapper.passthrough(Type.VAR_INT); // block face
+            wrapper.passthrough(Type.VAR_INT); // hand
+            wrapper.passthrough(Type.FLOAT); // cursor x
+            wrapper.passthrough(Type.FLOAT); // cursor y
+            wrapper.passthrough(Type.FLOAT); // cursor z
+
+            if (Via.getConfig().isServersideBlockConnections() && ConnectionData.needStoreBlocks()) {
+                ConnectionData.markModified(wrapper.user(), pos);
+            }
+        });
+        protocol.registerServerbound(ServerboundPackets1_13.PLAYER_DIGGING, wrapper -> {
+            int status = wrapper.passthrough(Type.VAR_INT); // Status
+            Position pos = wrapper.passthrough(Type.POSITION); // Location
+            wrapper.passthrough(Type.UNSIGNED_BYTE); // block face
+
+            // 0 = Started digging: if in creative this causes the block to break directly
+            // There's no point in storing the finished digging as it may never show-up (creative)
+            if (status == 0 && Via.getConfig().isServersideBlockConnections() && ConnectionData.needStoreBlocks()) {
+                ConnectionData.markModified(wrapper.user(), pos);
+            }
+        });
+
     }
 
     public static int toNewId(int oldId) {

common/src/main/java/com/viaversion/viaversion/protocols/protocol1_13to1_12_2/storage/BlockConnectionStorage.java

@@ -17,18 +17,25 @@
  */
 package com.viaversion.viaversion.protocols.protocol1_13to1_12_2.storage;
 
+import com.google.common.collect.EvictingQueue;
 import com.viaversion.viaversion.api.Via;
 import com.viaversion.viaversion.api.connection.StorableObject;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Queue;
+
+import com.viaversion.viaversion.api.minecraft.Position;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class BlockConnectionStorage implements StorableObject {
     private static Constructor<?> fastUtilLongObjectHashMap;
 
     private final Map<Long, SectionData> blockStorage = createLongObjectMap();
+    @SuppressWarnings("UnstableApiUsage")
+    private final Queue<Position> modified = EvictingQueue.create(5);
 
     // Cache to retrieve section quicker
     private Long lastIndex;
@@ -88,10 +95,27 @@ public class BlockConnectionStorage implements StorableObject {
         }
     }
 
+    public void markModified(Position pos) {
+        // Avoid saving the same pos twice
+        if (!modified.contains(pos)) {
+            this.modified.add(pos);
+        }
+    }
+
+    public boolean recentlyModified(Position pos) {
+        for (Position p : modified) {
+            if (Math.abs(pos.x() - p.x()) + Math.abs(pos.y() - p.y()) + Math.abs(pos.z() - p.z()) <= 2) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public void clear() {
         blockStorage.clear();
         lastSection = null;
         lastIndex = null;
+        modified.clear();
     }
 
     public void unloadChunk(int x, int z) {