[1.20.5] Record a BlockSnapshot for client break predictions and clea…

…nup break pipeline (#809)

patches/net/minecraft/client/multiplayer/ClientLevel.java.patch

@@ -17,6 +17,22 @@
 
      public void handleBlockChangedAck(int p_233652_) {
          this.blockStatePredictionHandler.endPredictionsUpTo(p_233652_, this);
+@@ -157,10 +_,15 @@
+     @Override
+     public boolean setBlock(BlockPos p_233643_, BlockState p_233644_, int p_233645_, int p_233646_) {
+         if (this.blockStatePredictionHandler.isPredicting()) {
++            // Neo: Record and store a snapshot in the prediction so that BE data can be restored if the break is denied.
++            // Fixes MC-36093 and permits correct server-side only cancellation of block changes.
++            var snapshot = net.neoforged.neoforge.common.util.BlockSnapshot.create(this.dimension(), this, p_233643_, p_233645_);
++
+             BlockState blockstate = this.getBlockState(p_233643_);
+             boolean flag = super.setBlock(p_233643_, p_233644_, p_233645_, p_233646_);
+             if (flag) {
+                 this.blockStatePredictionHandler.retainKnownServerState(p_233643_, blockstate, this.minecraft.player);
++                this.blockStatePredictionHandler.retainSnapshot(p_233643_, snapshot);
+             }
+
+             return flag;
 @@ -192,6 +_,7 @@
          this.serverSimulationDistance = p_205510_;
          this.updateSkyBrightness();

patches/net/minecraft/client/multiplayer/MultiPlayerGameMode.java.patch

@@ -1,13 +1,5 @@
 --- a/net/minecraft/client/multiplayer/MultiPlayerGameMode.java
 +++ b/net/minecraft/client/multiplayer/MultiPlayerGameMode.java
-@@ -106,6 +_,7 @@
-     }
-
-     public boolean destroyBlock(BlockPos p_105268_) {
-+        if (minecraft.player.getMainHandItem().onBlockStartBreak(p_105268_, minecraft.player)) return false;
-         if (this.minecraft.player.blockActionRestricted(this.minecraft.level, p_105268_, this.localPlayerMode)) {
-             return false;
-         } else {
 @@ -120,9 +_,8 @@
                  } else if (blockstate.isAir()) {
                      return false;

patches/net/minecraft/client/multiplayer/prediction/BlockStatePredictionHandler.java.patch

@@ -0,0 +1,39 @@
+--- a/net/minecraft/client/multiplayer/prediction/BlockStatePredictionHandler.java
++++ b/net/minecraft/client/multiplayer/prediction/BlockStatePredictionHandler.java
+@@ -49,6 +_,15 @@
+                 p_233858_.syncBlockState(
+                     blockpos, blockstatepredictionhandler$serververifiedstate.blockState, blockstatepredictionhandler$serververifiedstate.playerPos
+                 );
++                // Neo: Restore the BlockEntity if one was present before the break was cancelled.
++                // Fixes MC-36093 and permits correct server-side only cancellation of block changes.
++                var verifiedState = blockstatepredictionhandler$serververifiedstate;
++                if (verifiedState.snapshot != null && verifiedState.blockState == verifiedState.snapshot.getState()) {
++                    if (verifiedState.snapshot.restoreBlockEntity(p_233858_, blockpos)) {
++                        // Attempt a re-render if BE data was loaded, since some blocks may depend on it.
++                        p_233858_.sendBlockUpdated(blockpos, verifiedState.blockState, verifiedState.blockState, 3);
++                    }
++                }
+             }
+         }
+     }
+@@ -72,8 +_,20 @@
+         return this.isPredicting;
+     }
+
++    /**
++     * Sets the stored BlockSnapshot on the ServerVerifiedState for the given position.
++     * This method is only called after {@link #retainKnownServerState}, so we are certain a map entry exists.
++     */
++    public void retainSnapshot(BlockPos pos, net.neoforged.neoforge.common.util.BlockSnapshot snapshot) {
++        this.serverVerifiedStates.get(pos.asLong()).snapshot = snapshot;
++    }
++
+     @OnlyIn(Dist.CLIENT)
+     static class ServerVerifiedState {
++        /**
++         * Neo: Used to hold all data necessary for clientside restoration during break denial.
++         */
++        net.neoforged.neoforge.common.util.BlockSnapshot snapshot;
+         final Vec3 playerPos;
+         int sequence;
+         BlockState blockState;

patches/net/minecraft/server/level/ServerPlayerGameMode.java.patch

@@ -35,17 +35,12 @@
      public boolean destroyBlock(BlockPos p_9281_) {
          BlockState blockstate1 = this.level.getBlockState(p_9281_);
 -        if (!this.player.getMainHandItem().getItem().canAttackBlock(blockstate1, this.level, p_9281_, this.player)) {
-+        int exp = net.neoforged.neoforge.common.CommonHooks.onBlockBreakEvent(level, gameModeForPlayer, player, p_9281_);
++        int exp = net.neoforged.neoforge.common.CommonHooks.fireBlockBreak(level, gameModeForPlayer, player, p_9281_, blockstate1);
 +        if (exp == -1) {
              return false;
          } else {
              BlockEntity blockentity = this.level.getBlockEntity(p_9281_);
-@@ -232,30 +_,44 @@
-             if (block instanceof GameMasterBlock && !this.player.canUseGameMasterBlocks()) {
-                 this.level.sendBlockUpdated(p_9281_, blockstate1, blockstate1, 3);
-                 return false;
-+            } else if (player.getMainHandItem().onBlockStartBreak(p_9281_, player)) {
-+                return false;
+@@ -235,27 +_,46 @@
              } else if (this.player.blockActionRestricted(this.level, p_9281_, this.gameModeForPlayer)) {
                  return false;
              } else {
@@ -57,7 +52,7 @@
 +                BlockState blockstate = blockstate1;
 
                  if (this.isCreative()) {
-+                    removeBlock(p_9281_, false);
++                    removeBlock(p_9281_, blockstate, false);
                      return true;
                  } else {
                      ItemStack itemstack = this.player.getMainHandItem();
@@ -67,7 +62,7 @@
                      itemstack.mineBlock(this.level, blockstate, p_9281_, this.player);
 +                    if (itemstack.isEmpty() && !itemstack1.isEmpty())
 +                        net.neoforged.neoforge.event.EventHooks.onPlayerDestroyItem(this.player, itemstack1, InteractionHand.MAIN_HAND);
-+                    boolean flag = removeBlock(p_9281_, flag1);
++                    boolean flag = removeBlock(p_9281_, blockstate, flag1);
 +
                      if (flag1 && flag) {
                          block.playerDestroy(this.level, this.player, p_9281_, blockstate, blockentity, itemstack1);
@@ -82,11 +77,18 @@
          }
 +    }
 +
-+    private boolean removeBlock(BlockPos p_180235_1_, boolean canHarvest) {
-+        BlockState state = this.level.getBlockState(p_180235_1_);
-+        boolean removed = state.onDestroyedByPlayer(this.level, p_180235_1_, this.player, canHarvest, this.level.getFluidState(p_180235_1_));
++    /**
++     * Patched-in method that handles actual removal of blocks for {@link #destroyBlock(BlockPos)}.
++     *
++     * @param pos The block pos of the destroyed block
++     * @param state The state of the destroyed block
++     * @param canHarvest If the player breaking the block can harvest the drops of the block
++     * @return If the block was removed, as reported by {@link BlockState#onDestroyedByPlayer}.
++     */
++    private boolean removeBlock(BlockPos pos, BlockState state, boolean canHarvest) {
++        boolean removed = state.onDestroyedByPlayer(this.level, pos, this.player, canHarvest, this.level.getFluidState(pos));
 +        if (removed)
-+            state.getBlock().destroy(this.level, p_180235_1_, state);
++            state.getBlock().destroy(this.level, pos, state);
 +        return removed;
      }
 

src/main/java/net/neoforged/neoforge/common/CommonHooks.java

@@ -58,7 +58,6 @@
 import net.minecraft.network.chat.MutableComponent;
 import net.minecraft.network.chat.TextColor;
 import net.minecraft.network.chat.contents.PlainTextContents;
-import net.minecraft.network.protocol.Packet;
 import net.minecraft.network.protocol.game.ClientboundBlockUpdatePacket;
 import net.minecraft.network.protocol.game.ServerboundPlayerActionPacket;
 import net.minecraft.network.syncher.EntityDataSerializer;
@@ -124,7 +123,7 @@
 import net.minecraft.world.level.biome.MobSpawnSettings;
 import net.minecraft.world.level.block.Block;
 import net.minecraft.world.level.block.Blocks;
-import net.minecraft.world.level.block.entity.BlockEntity;
+import net.minecraft.world.level.block.GameMasterBlock;
 import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.block.state.pattern.BlockInWorld;
 import net.minecraft.world.level.chunk.ChunkAccess;
@@ -447,51 +446,44 @@ public static void dropXpForBlock(BlockState state, ServerLevel level, BlockPos
             state.getBlock().popExperience(level, pos, exp);
     }
 
-    public static int onBlockBreakEvent(Level level, GameType gameType, ServerPlayer entityPlayer, BlockPos pos) {
-        // Logic from tryHarvestBlock for pre-canceling the event
+    /**
+     * Fires {@link BlockEvent.BreakEvent}, pre-emptively canceling the event based on the conditions that will cause the block to not be broken anyway.
+     * <p>
+     * Note that undoing the pre-cancel will not permit breaking the block, since the vanilla conditions will always be checked.
+     * 
+     * @param level    The level
+     * @param gameType The game type of the breaking player
+     * @param player   The breaking player
+     * @param pos      The position of the block being broken
+     * @param state    The state of the block being broken
+     * @return The experience dropped by the broken block, or -1 if the break event was canceled.
+     */
+    public static int fireBlockBreak(Level level, GameType gameType, ServerPlayer player, BlockPos pos, BlockState state) {
         boolean preCancelEvent = false;
-        ItemStack itemstack = entityPlayer.getMainHandItem();
-        if (!itemstack.isEmpty() && !itemstack.getItem().canAttackBlock(level.getBlockState(pos), level, pos, entityPlayer)) {
+
+        ItemStack itemstack = player.getMainHandItem();
+        if (!itemstack.isEmpty() && !itemstack.getItem().canAttackBlock(state, level, pos, player)) {
             preCancelEvent = true;
         }
 
-        if (gameType.isBlockPlacingRestricted()) {
-            if (gameType == GameType.SPECTATOR)
-                preCancelEvent = true;
-
-            if (!entityPlayer.mayBuild()) {
-                AdventureModePredicate adventureModePredicate = itemstack.get(DataComponents.CAN_BREAK);
-                if (itemstack.isEmpty() || adventureModePredicate == null || !adventureModePredicate.test(new BlockInWorld(level, pos, false))) {
-                    preCancelEvent = true;
-                }
-            }
+        if (player.blockActionRestricted(level, pos, gameType)) {
+            preCancelEvent = true;
         }
 
-        // Tell client the block is gone immediately then process events
-        if (level.getBlockEntity(pos) == null) {
-            entityPlayer.connection.send(new ClientboundBlockUpdatePacket(pos, level.getFluidState(pos).createLegacyBlock()));
+        if (state.getBlock() instanceof GameMasterBlock && !player.canUseGameMasterBlocks()) {
+            preCancelEvent = true;
         }
 
         // Post the block break event
-        BlockState state = level.getBlockState(pos);
-        BlockEvent.BreakEvent event = new BlockEvent.BreakEvent(level, pos, state, entityPlayer);
+        BlockEvent.BreakEvent event = new BlockEvent.BreakEvent(level, pos, state, player);
         event.setCanceled(preCancelEvent);
         NeoForge.EVENT_BUS.post(event);
 
-        // Handle if the event is canceled
+        // If the event is canceled, let the client know the block still exists
         if (event.isCanceled()) {
-            // Let the client know the block still exists
-            entityPlayer.connection.send(new ClientboundBlockUpdatePacket(level, pos));
-
-            // Update any tile entity data for this block
-            BlockEntity blockEntity = level.getBlockEntity(pos);
-            if (blockEntity != null) {
-                Packet<?> pkt = blockEntity.getUpdatePacket();
-                if (pkt != null) {
-                    entityPlayer.connection.send(pkt);
-                }
-            }
+            player.connection.send(new ClientboundBlockUpdatePacket(pos, state));
         }
+
         return event.isCanceled() ? -1 : event.getExpToDrop();
     }
 
@@ -549,7 +541,7 @@ public static InteractionResult onPlaceItemIntoWorld(UseOnContext context) {
                 // revert back all captured blocks
                 for (BlockSnapshot blocksnapshot : Lists.reverse(blockSnapshots)) {
                     level.restoringBlockSnapshots = true;
-                    blocksnapshot.restore(true, false);
+                    blocksnapshot.restore(blocksnapshot.getFlags() | Block.UPDATE_CLIENTS);
                     level.restoringBlockSnapshots = false;
                 }
             } else {
@@ -558,8 +550,8 @@ public static InteractionResult onPlaceItemIntoWorld(UseOnContext context) {
                 itemstack.applyComponents(newComponents);
 
                 for (BlockSnapshot snap : blockSnapshots) {
-                    int updateFlag = snap.getFlag();
-                    BlockState oldBlock = snap.getReplacedBlock();
+                    int updateFlag = snap.getFlags();
+                    BlockState oldBlock = snap.getState();
                     BlockState newBlock = level.getBlockState(snap.getPos());
                     newBlock.onPlace(level, snap.getPos(), oldBlock, false);
 

src/main/java/net/neoforged/neoforge/common/extensions/IItemExtension.java

@@ -138,21 +138,6 @@ default float getXpRepairRatio(ItemStack stack) {
         return 2f;
     }
 
-    /**
-     * Called before a block is broken. Return true to prevent default block
-     * harvesting.
-     *
-     * Note: In SMP, this is called on both client and server sides!
-     *
-     * @param itemstack The current ItemStack
-     * @param pos       Block's position in world
-     * @param player    The Player that is wielding the item
-     * @return True to prevent harvesting, false to continue as normal
-     */
-    default boolean onBlockStartBreak(ItemStack itemstack, BlockPos pos, Player player) {
-        return false;
-    }
-
     /**
      * Called when an entity stops using an item for any reason, notably when selecting another item without releasing or finishing.
      * This method is called in addition to any other hooks called when an item is finished using; when another hook is also called it will be called before this method.

src/main/java/net/neoforged/neoforge/common/extensions/IItemStackExtension.java

@@ -125,20 +125,6 @@ default boolean canPerformAction(ToolAction toolAction) {
         return self().getItem().canPerformAction(self(), toolAction);
     }
 
-    /**
-     * Called before a block is broken. Return true to prevent default block
-     * harvesting.
-     *
-     * Note: In SMP, this is called on both client and server sides!
-     *
-     * @param pos    Block's position in world
-     * @param player The Player that is wielding the item
-     * @return True to prevent harvesting, false to continue as normal
-     */
-    default boolean onBlockStartBreak(BlockPos pos, Player player) {
-        return !self().isEmpty() && self().getItem().onBlockStartBreak(self(), pos, player);
-    }
-
     /**
      * Called when the player is mining a block and the item in his hand changes.
      * Allows to not reset blockbreaking if only NBT or similar changes.