diff --git a/src/main/java/ac/grim/grimac/checks/impl/scaffolding/AirLiquidPlace.java b/src/main/java/ac/grim/grimac/checks/impl/scaffolding/AirLiquidPlace.java index 7f31823f23..a4732b6174 100644 --- a/src/main/java/ac/grim/grimac/checks/impl/scaffolding/AirLiquidPlace.java +++ b/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 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 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 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 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 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 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; } diff --git a/src/main/java/ac/grim/grimac/utils/latency/CompensatedWorld.java b/src/main/java/ac/grim/grimac/utils/latency/CompensatedWorld.java index 672a63a2be..a98521a7bc 100644 --- a/src/main/java/ac/grim/grimac/utils/latency/CompensatedWorld.java +++ b/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 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