Finish documenting all edge cases that can false AirLiquidPlace and f…

…ixing them

src/main/java/ac/grim/grimac/checks/impl/scaffolding/AirLiquidPlace.java

@@ -12,7 +12,6 @@
 import com.github.retrooper.packetevents.protocol.world.states.type.StateType;
 import com.github.retrooper.packetevents.util.Vector3i;
 
-import java.util.Iterator;
 
 
 @CheckData(name = "AirLiquidPlace")
@@ -29,13 +28,32 @@ public AirLiquidPlace(GrimPlayer player) {
 
     We will often see:
         Async world updated: short_grass -> air at X: -32, Y: 69, Z: -240, tick 0, cause/source: DiggingAction.START_DIGGING
-        AirLiquidPlace Check: Block state at X: -32, Y: 69, Z: -240 is air (valid: false), tick +0, cause/source: PacketType.Play.Client.PLAYER_BLOCK_PLACEMENT <---- previously falsed here
+        AirLiquidPlace Check: Block state at X: -32, Y: 69, Z: -240 is air (valid: false), tick +0-1, cause/source: PacketType.Play.Client.PLAYER_BLOCK_PLACEMENT <---- previously falsed here
         Async world updated: air -> short_grass at X: -32, Y: 69, Z: -240, tick +3-4, cause: realtime task in applyBlockChanges(List<Vector3i> toApplyBlocks) source: PacketType.Play.Client.PONG
         Async world updated: short_grass -> air at X: -32, Y: 69, Z: -240, tick +0-1, cause: handleNettySyncTransaction(LatencyUtils.java:56) source: PacketType.Play.Client.PONG
 
-        Which previously would've caused a false.
+    In addition, it is possible for:
+        Async world updated: short_grass -> air at X: -49, Y: 69, Z: -190, tick 0, cause: realtime task in applyBlockChanges(List<Vector3i> toApplyBlocks) source: PacketType.Play.Client.PONG
+        AirLiquidPlace Check: Block state at X: -49, Y: 69, Z: -190 is air (valid=false), tick 0 <--- false due to change from applyBlockChanges()
+        Async world updated: grass_block[snowy=false] -> grass_block[snowy=false] at X: -49, Y: 69, Z: -189, tick 0, cause: handleNettySyncTransaction(LatencyUtils.java:56) source: PacketType.Play.Client.PONG
+
+    And in even more rare cases:
+        Async world updated: air -> air at X: -51, Y: 71, Z: -179, tick 0, cause: handleNettySyncTransaction(LatencyUtils.java:56) source: PacketType.Play.Client.PONG
+        AirLiquidPlace Check: Block state at X: -49, Y: 70, Z: -180 is short_grass (valid=true), tick 0
+        Async world updated: short_grass -> air at X: -49, Y: 70, Z: -180, tick 1, cause/source: DiggingAction.START_DIGGING <--- double dig here (see my BadPacketsX patch) this is legit behaviour. Can only be up to 2 in 1 tick though.
+        Async world updated: air -> short_grass at X: -49, Y: 70, Z: -180, tick 1, cause/source: DiggingAction.START_DIGGING
+        Async world updated: short_grass -> air at X: -51, Y: 70, Z: -179, tick 1, cause: realtime task in applyBlockChanges(List<Vector3i> toApplyBlocks) source: PacketType.Play.Client.PONG
+        AirLiquidPlace Check: Block state at X: -49, Y: 70, Z: -179 is air (valid=false), tick 2 <--- falses here due to double dig if we only check the latest changed blockstate. We have to check all changes at the location in same tick.
+        AirLiquidPlace Check: Block state at X: -49, Y: 70, Z: -179 is air (valid=true), tick 2
+
+        All of which previously would've caused a false.
         To solve this we store recently changed blocks caused by DiggingAction.START_DIGGING (instant breaking) and check against the old block.
         Lots of other checks have similar issues, and with the new player.blockHistory we can patch those.
+
+    So that's it right? It's unfalsable?
+        Very close but not quite. Vanilla's client game desyncs, especially on a laggy connection where a player is breaking and placing grass 20 cps/sec in the same tick
+        it is possible for short grass to be interacted with even if server-side the block is air much later, and it won't be accounted for because the modification isn't recent.
+        This is incredibly rare, unreliable and is only triggerable if you intentionally want to false the check. Enough so that I consider a violation lvl of 2-3 to be perfect.
     */
 
     @Override
@@ -46,18 +64,16 @@ public void onBlockPlace(final BlockPlace place) {
         StateType placeAgainst = player.compensatedWorld.getStateTypeAt(blockPos.getX(), blockPos.getY(), blockPos.getZ());
 
         int currentTick = GrimAPI.INSTANCE.getTickManager().currentTick;
-
-
-        Iterable<BlockModification> blockModifications = player.blockHistory.getRecentModifications((blockModification) -> currentTick - blockModification.getTick() == 0
+        // this is actual more lenient then we need to be, We can check up to 1 ticks for all changes at location sand up to 0 ticks for first change
+        // But for such tiny differences in legitness its not worth it.
+        Iterable<BlockModification> blockModifications = player.blockHistory.getRecentModifications((blockModification) -> currentTick - blockModification.getTick() < 2
                 && blockPos.equals(blockModification.getLocation())
-                && blockModification.getCause() == BlockModification.Cause.START_DIGGING);
-
+                && (blockModification.getCause() == BlockModification.Cause.START_DIGGING || blockModification.getCause() == BlockModification.Cause.HANDLE_NETTY_SYNC_TRANSACTION));
 
         // Check if old block from instant breaking in same tick as the current placement was valid
         // There should only be one block here for legit clients
-        Iterator<BlockModification> iterator =  blockModifications.iterator();
-        if (iterator.hasNext()) {
-            StateType stateType = iterator.next().getOldBlockContents().getType();
+        for (BlockModification blockModification : blockModifications) {
+            StateType stateType = blockModification.getOldBlockContents().getType();
             if (!(stateType.isAir() || Materials.isNoPlaceLiquid(stateType))) {
                 return;
             }

src/main/java/ac/grim/grimac/utils/latency/CompensatedWorld.java

@@ -2,6 +2,7 @@
 
 import ac.grim.grimac.GrimAPI;
 import ac.grim.grimac.player.GrimPlayer;
+import ac.grim.grimac.utils.change.BlockModification;
 import ac.grim.grimac.utils.chunks.Column;
 import ac.grim.grimac.utils.collisions.CollisionData;
 import ac.grim.grimac.utils.collisions.datatypes.SimpleCollisionBox;
@@ -139,8 +140,16 @@ private void applyBlockChanges(List<Vector3i> toApplyBlocks) {
     private void handleAck(Vector3i vector3i, int originalBlockId, Vector3d playerPosition) {
         // If we need to change the world block state
         if (getWrappedBlockStateAt(vector3i).getGlobalId() != originalBlockId) {
+            player.blockHistory.add(
+                    new BlockModification(
+                            player.compensatedWorld.getWrappedBlockStateAt(vector3i),
+                            WrappedBlockState.getByGlobalId(originalBlockId),
+                            vector3i,
+                            GrimAPI.INSTANCE.getTickManager().currentTick,
+                            BlockModification.Cause.HANDLE_NETTY_SYNC_TRANSACTION
+                    )
+            );
             updateBlock(vector3i.getX(), vector3i.getY(), vector3i.getZ(), originalBlockId);
-
             WrappedBlockState state = WrappedBlockState.getByGlobalId(blockVersion, originalBlockId);
 
             // The player will teleport themselves if they get stuck in the reverted block