Propagate Block#playerWillDestroy result to Block#playerDestroy

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

@@ -1,16 +1,25 @@
 --- a/net/minecraft/client/multiplayer/MultiPlayerGameMode.java
 +++ b/net/minecraft/client/multiplayer/MultiPlayerGameMode.java
-@@ -120,9 +_,8 @@
+@@ -120,14 +_,14 @@
                  } else if (blockstate.isAir()) {
                      return false;
                  } else {
 -                    block.playerWillDestroy(level, p_105268_, blockstate, this.minecraft.player);
                      FluidState fluidstate = level.getFluidState(p_105268_);
 -                    boolean flag = level.setBlock(p_105268_, fluidstate.createLegacyBlock(), 11);
-+                    boolean flag = blockstate.onDestroyedByPlayer(level, p_105268_, minecraft.player, false, fluidstate);
-                     if (flag) {
-                         block.destroy(level, p_105268_, blockstate);
+-                    if (flag) {
+-                        block.destroy(level, p_105268_, blockstate);
++                    BlockState playerDestroyedBlock = blockstate.onDestroyedByPlayer(level, p_105268_, minecraft.player, false, fluidstate);
++                    if (playerDestroyedBlock != null) {
++                        block.destroy(level, p_105268_, playerDestroyedBlock);
++                        return true;
                      }
+
+-                    return flag;
++                    return false;
+                 }
+             }
+         }
 @@ -143,6 +_,7 @@
                  BlockState blockstate = this.minecraft.level.getBlockState(p_105270_);
                  this.minecraft.getTutorial().onDestroyBlock(this.minecraft.level, p_105270_, blockstate, 1.0F);

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

@@ -40,7 +40,7 @@
              return false;
          } else {
              BlockEntity blockentity = this.level.getBlockEntity(p_9281_);
-@@ -235,27 +_,46 @@
+@@ -235,27 +_,47 @@
              } else if (this.player.blockActionRestricted(this.level, p_9281_, this.gameModeForPlayer)) {
                  return false;
              } else {
@@ -60,16 +60,18 @@
 -                    boolean flag = this.player.hasCorrectToolForDrops(blockstate);
 +                    boolean flag1 = blockstate.canHarvestBlock(this.level, p_9281_, this.player); // previously player.hasCorrectToolForDrops(blockstate)
                      itemstack.mineBlock(this.level, blockstate, p_9281_, this.player);
+-                    if (flag1 && flag) {
+-                        block.playerDestroy(this.level, this.player, p_9281_, blockstate, blockentity, itemstack1);
 +                    if (itemstack.isEmpty() && !itemstack1.isEmpty())
 +                        net.neoforged.neoforge.event.EventHooks.onPlayerDestroyItem(this.player, itemstack1, InteractionHand.MAIN_HAND);
-+                    boolean flag = removeBlock(p_9281_, blockstate, flag1);
++                    BlockState blockState = removeBlock(p_9281_, blockstate, flag1);
 +
-                     if (flag1 && flag) {
-                         block.playerDestroy(this.level, this.player, p_9281_, blockstate, blockentity, itemstack1);
++                    if (flag1 && blockState != null) {
++                        block.playerDestroy(this.level, this.player, p_9281_, blockState, blockentity, itemstack1);
                      }
 
-+                    if (flag && exp > 0)
-+                        blockstate.getBlock().popExperience(level, p_9281_, exp);
++                    if (blockState != null && exp > 0)
++                        blockState.getBlock().popExperience(level, p_9281_, exp);
 +
                      return true;
                  }
@@ -83,13 +85,14 @@
 +     * @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}.
++     * @return The new blockstate to pass to {@link Block#destroy} and {@link Block#playerDestroy} if this block was successfully replaced in world as reported by {@link BlockState#onDestroyedByPlayer}. Null if block was not replaced.
 +     */
-+    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, pos, state);
-+        return removed;
++    private @Nullable BlockState removeBlock(BlockPos pos, BlockState state, boolean canHarvest) {
++        BlockState blockState = state.onDestroyedByPlayer(this.level, pos, this.player, canHarvest, this.level.getFluidState(pos));
++        if (blockState != null) {
++            blockState.getBlock().destroy(this.level, pos, blockState);
++        }
++        return blockState;
      }
 
      public InteractionResult useItem(ServerPlayer p_9262_, Level p_9263_, ItemStack p_9264_, InteractionHand p_9265_) {

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

@@ -215,11 +215,11 @@ default public boolean canHarvestBlock(BlockState state, BlockGetter level, Bloc
      * @param willHarvest True if Block.harvestBlock will be called after this, if the return in true.
      *                    Can be useful to delay the destruction of tile entities till after harvestBlock
      * @param fluid       The current fluid state at current position
-     * @return True if the block is actually destroyed.
+     * @return The new blockstate to pass to {@link Block#destroy} and {@link Block#playerDestroy} if this block was successfully replaced in world by {@link Level#setBlock}. Null if block was not replaced.
      */
-    default boolean onDestroyedByPlayer(BlockState state, Level level, BlockPos pos, Player player, boolean willHarvest, FluidState fluid) {
-        self().playerWillDestroy(level, pos, state, player);
-        return level.setBlock(pos, fluid.createLegacyBlock(), level.isClientSide ? 11 : 3);
+    default @Nullable BlockState onDestroyedByPlayer(BlockState state, Level level, BlockPos pos, Player player, boolean willHarvest, FluidState fluid) {
+        BlockState newState = self().playerWillDestroy(level, pos, state, player);
+        return level.setBlock(pos, fluid.createLegacyBlock(), level.isClientSide ? 11 : 3) ? newState : null;
     }
 
     /**

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

@@ -28,6 +28,7 @@
 import net.minecraft.world.level.LevelAccessor;
 import net.minecraft.world.level.LevelReader;
 import net.minecraft.world.level.SignalGetter;
+import net.minecraft.world.level.block.Block;
 import net.minecraft.world.level.block.Rotation;
 import net.minecraft.world.level.block.SoundType;
 import net.minecraft.world.level.block.state.BlockBehaviour;
@@ -129,9 +130,9 @@ default boolean canHarvestBlock(BlockGetter level, BlockPos pos, Player player)
      * @param willHarvest True if Block.harvestBlock will be called after this, if the return in true.
      *                    Can be useful to delay the destruction of tile entities till after harvestBlock
      * @param fluid       The current fluid and block state for the position in the level.
-     * @return True if the block is actually destroyed.
+     * @return The new blockstate to pass to {@link Block#destroy} and {@link Block#playerDestroy} if this block was successfully replaced in world. Null if block was not replaced.
      */
-    default boolean onDestroyedByPlayer(Level level, BlockPos pos, Player player, boolean willHarvest, FluidState fluid) {
+    default @Nullable BlockState onDestroyedByPlayer(Level level, BlockPos pos, Player player, boolean willHarvest, FluidState fluid) {
         return self().getBlock().onDestroyedByPlayer(self(), level, pos, player, willHarvest, fluid);
     }
 

tests/src/main/java/net/neoforged/neoforge/debug/block/BlockTests.java

@@ -17,7 +17,9 @@
 import net.minecraft.util.RandomSource;
 import net.minecraft.world.InteractionHand;
 import net.minecraft.world.ItemInteractionResult;
+import net.minecraft.world.entity.Entity;
 import net.minecraft.world.entity.EntityType;
+import net.minecraft.world.entity.item.ItemEntity;
 import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.Items;
@@ -47,6 +49,32 @@
 public class BlockTests {
     public static final String GROUP = "level.block";
 
+    @GameTest
+    @EmptyTemplate
+    @TestHolder(description = "Tests if player breaking decorated pots with swords drops Bricks")
+    static void decoratedPotBreaking(final DynamicTest test) {
+        test.onGameTest(helper -> helper.startSequence(() -> helper.makeTickingMockServerPlayerInCorner(GameType.SURVIVAL))
+
+                // Mine pot with sword
+                .thenExecute(() -> helper.setBlock(1, 1, 1, Blocks.DECORATED_POT.defaultBlockState()))
+                .thenExecute(player -> player.setItemInHand(InteractionHand.MAIN_HAND, Items.DIAMOND_SWORD.getDefaultInstance()))
+                .thenExecute(player -> player.gameMode.destroyBlock(helper.absolutePos(new BlockPos(1, 1, 1))))
+                .thenExecute(player -> helper.assertTrue(
+                        helper.getLevel().getEntitiesOfClass(ItemEntity.class, player.getBoundingBox().expandTowards(2, 2, 2)).stream().anyMatch(itemEntity -> itemEntity.getItem().is(Items.BRICK)),
+                        "Decorated Pot should had dropped Bricks"))
+                .thenExecute(player -> helper.getLevel().getEntitiesOfClass(ItemEntity.class, player.getBoundingBox().expandTowards(2, 2, 2)).forEach(itemEntity -> itemEntity.remove(Entity.RemovalReason.DISCARDED)))
+
+                .thenExecute(() -> helper.setBlock(1, 1, 1, Blocks.DECORATED_POT.defaultBlockState()))
+                .thenExecute(player -> player.setItemInHand(InteractionHand.MAIN_HAND, Items.DANDELION.getDefaultInstance()))
+                .thenExecute(player -> player.gameMode.destroyBlock(helper.absolutePos(new BlockPos(1, 1, 1))))
+                .thenExecute(player -> helper.assertTrue(
+                        helper.getLevel().getEntitiesOfClass(ItemEntity.class, player.getBoundingBox().expandTowards(2, 2, 2)).stream().anyMatch(itemEntity -> itemEntity.getItem().is(Items.DECORATED_POT)),
+                        "Decorated Pot should had dropped the Decorated Pot"))
+                .thenExecute(player -> helper.getLevel().getEntitiesOfClass(ItemEntity.class, player.getBoundingBox().expandTowards(2, 2, 2)).forEach(itemEntity -> itemEntity.remove(Entity.RemovalReason.DISCARDED)))
+
+                .thenSucceed());
+    }
+
     @GameTest
     @EmptyTemplate
     @TestHolder(description = "Tests if custom fence gates without wood types work, allowing for the use of the vanilla block for non-wooden gates")