Fix breaking of custom blocks or blocks with custom items (#5817)

* Tick BlockBreakHandler to properly send updates when breaking custom blocks/blocks with custom items

* Call tick when receiving input packet, store Direction instead of face ID

* Whoops, only call tick when no block actions are performed

core/src/main/java/org/geysermc/geyser/session/cache/BlockBreakHandler.java

@@ -106,6 +106,12 @@ public class BlockBreakHandler {
      */
     protected long blockStartBreakTime = 0;
 
+    /**
+     * The last known face of the block the client was breaking.
+     * Only set when keeping track of custom blocks / custom items breaking blocks.
+     */
+    protected Direction lastBlockBreakFace = null;
+
     /**
      * The last block position that was instantly broken.
      * Used to ignore subsequent block actions from the Bedrock client.
@@ -142,7 +148,7 @@ public class BlockBreakHandler {
     }
 
     /**
-     * Main entrypoint that handles block breaking actions, if present
+     * Main entrypoint that handles block breaking actions, if present. Ticks the handler if no breaking actions were performed.
      * @param packet the player auth input packet
      */
     public void handlePlayerAuthInputPacket(PlayerAuthInputPacket packet) {
@@ -150,6 +156,19 @@ public class BlockBreakHandler {
             handleBlockBreakActions(packet);
             restoredBlocks.clear();
             this.itemFramePos = null;
+        } else {
+            tick();
+        }
+    }
+
+    protected void tick() {
+        // We need to manually check if a block should be destroyed, and send the client progress updates, when mining a custom block, or with a custom item
+        // This is because, in CustomItemRegistryPopulator#computeToolProperties, we set a block break speed of 0,
+        // meaning the client will only ever send START_BREAK for breaking blocks, and nothing else
+
+        // Check lastBlockBreakFace, currentBlockPos and currentBlockState, just in case
+        if (blockStartBreakTime != 0 && lastBlockBreakFace != null && currentBlockPos != null && currentBlockState != null) {
+            handleContinueDestroy(currentBlockPos, currentBlockState, lastBlockBreakFace, session.getClientTicks());
         }
     }
 
@@ -157,12 +176,12 @@ public class BlockBreakHandler {
         for (int i = 0; i < packet.getPlayerActions().size(); i++) {
             PlayerBlockActionData actionData = packet.getPlayerActions().get(i);
             Vector3i position = actionData.getBlockPosition();
-            int blockFace = actionData.getFace();
+            Direction blockFace = Direction.VALUES[actionData.getFace()];
 
             switch (actionData.getAction()) {
                 case DROP_ITEM -> {
                     ServerboundPlayerActionPacket dropItemPacket = new ServerboundPlayerActionPacket(PlayerAction.DROP_ITEM,
-                        position, Direction.VALUES[blockFace], 0);
+                        position, blockFace, 0);
                     session.sendDownstreamGamePacket(dropItemPacket);
                 }
                 // Must do this ugly as it can also be called from the block_continue_destroy case :(
@@ -253,7 +272,7 @@ public class BlockBreakHandler {
      * Called from either a START_BREAK or BLOCK_CONTINUE_DESTROY case, the latter
      * if the client switches to a new block. This method then runs pre-break checks.
      */
-    private void preStartBreakHandle(Vector3i position, int blockFace, long tick) {
+    private void preStartBreakHandle(Vector3i position, Direction blockFace, long tick) {
         // New block being broken -> ignore previous insta-mine pos since that's no longer relevant
         lastInstaMinedPosition = null;
 
@@ -271,16 +290,15 @@ public class BlockBreakHandler {
         handleStartBreak(position, state, blockFace, tick);
     }
 
-    protected void handleStartBreak(@NonNull Vector3i position, @NonNull BlockState state, int blockFace, long tick) {
+    protected void handleStartBreak(@NonNull Vector3i position, @NonNull BlockState state, Direction blockFace, long tick) {
         GeyserItemStack item = session.getPlayerInventory().getItemInHand();
-        Direction direction = Direction.VALUES[blockFace];
 
         // Account for fire - the client likes to hit the block behind.
         Vector3i fireBlockPos = BlockUtils.getBlockPosition(position, blockFace);
         Block possibleFireBlock = session.getGeyser().getWorldManager().blockAt(session, fireBlockPos).block();
         if (possibleFireBlock == Blocks.FIRE || possibleFireBlock == Blocks.SOUL_FIRE) {
             ServerboundPlayerActionPacket startBreakingPacket = new ServerboundPlayerActionPacket(PlayerAction.START_DIGGING, fireBlockPos,
-                direction, session.getWorldCache().nextPredictionSequence());
+                blockFace, session.getWorldCache().nextPredictionSequence());
             session.sendDownstreamGamePacket(startBreakingPacket);
         }
 
@@ -291,7 +309,7 @@ public class BlockBreakHandler {
         if (session.isInstabuild() || breakProgress >= 1.0F) {
             // Avoid sending STOP_BREAK for instantly broken blocks
             lastInstaMinedPosition = position;
-            destroyBlock(state, position, direction, true);
+            destroyBlock(state, position, blockFace, true);
         } else {
             // If the block is custom or the breaking item is custom, we must keep track of break time ourselves
             ItemMapping mapping = item.getMapping(session);
@@ -303,6 +321,7 @@ public class BlockBreakHandler {
             if (BlockRegistries.NON_VANILLA_BLOCK_IDS.get().get(state.javaId()) || blockStateOverride != null ||
                     customItem != null || (skull != null && skull.getBlockDefinition() != null)) {
                 this.blockStartBreakTime = tick;
+                this.lastBlockBreakFace = blockFace;
             }
 
             LevelEventPacket startBreak = new LevelEventPacket();
@@ -311,27 +330,28 @@ public class BlockBreakHandler {
             startBreak.setData((int) (65535 / BlockUtils.reciprocal(breakProgress)));
             session.sendUpstreamPacket(startBreak);
 
-            BlockUtils.spawnBlockBreakParticles(session, direction, position, state);
+            BlockUtils.spawnBlockBreakParticles(session, blockFace, position, state);
 
             this.currentBlockPos = position;
             this.currentBlockState = state;
 
-            session.sendDownstreamGamePacket(new ServerboundPlayerActionPacket(PlayerAction.START_DIGGING, position, direction, session.getWorldCache().nextPredictionSequence()));
+            session.sendDownstreamGamePacket(new ServerboundPlayerActionPacket(PlayerAction.START_DIGGING, position, blockFace, session.getWorldCache().nextPredictionSequence()));
         }
     }
 
-    protected void handleContinueDestroy(Vector3i position, BlockState state, int blockFace, long tick) {
+    protected void handleContinueDestroy(Vector3i position, BlockState state, Direction blockFace, long tick) {
-        Direction direction = Direction.VALUES[blockFace];
+        BlockUtils.spawnBlockBreakParticles(session, blockFace, position, state);
-        BlockUtils.spawnBlockBreakParticles(session, direction, position, state);
         double totalBreakTime = BlockUtils.reciprocal(calculateBreakProgress(state, position, session.getPlayerInventory().getItemInHand()));
 
         if (blockStartBreakTime != 0) {
             long ticksSinceStart = tick - blockStartBreakTime;
             // We need to add a slight delay to the break time, otherwise the client breaks blocks too fast
             if (ticksSinceStart >= (totalBreakTime += 2)) {
-                destroyBlock(state, position, direction, false);
+                destroyBlock(state, position, blockFace, false);
                 return;
             }
+            // Update in case it has changed
+            lastBlockBreakFace = blockFace;
         }
 
         // Update the break time in the event that player conditions changed (jumping, effects applied)
@@ -342,8 +362,8 @@ public class BlockBreakHandler {
         session.sendUpstreamPacket(updateBreak);
     }
 
-    protected void handlePredictDestroy(Vector3i position, BlockState state, int blockFace, long tick) {
+    protected void handlePredictDestroy(Vector3i position, BlockState state, Direction blockFace, long tick) {
-        destroyBlock(state, position, Direction.VALUES[blockFace], false);
+        destroyBlock(state, position, blockFace, false);
     }
 
     protected void handleAbortBreaking(Vector3i position) {

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockInventoryTransactionTranslator.java

@@ -171,7 +171,7 @@ public class BedrockInventoryTransactionTranslator extends PacketTranslator<Inve
                 switch (packet.getActionType()) {
                     case 0 -> {
                         final Vector3i packetBlockPosition = packet.getBlockPosition();
-                        Vector3i blockPos = BlockUtils.getBlockPosition(packetBlockPosition, packet.getBlockFace());
+                        Vector3i blockPos = BlockUtils.getBlockPosition(packetBlockPosition, Direction.VALUES[packet.getBlockFace()]);
 
                         if (session.getGeyser().getConfig().isDisableBedrockScaffolding()) {
                             float yaw = session.getPlayerEntity().getYaw();

core/src/main/java/org/geysermc/geyser/util/BlockUtils.java

@@ -154,15 +154,14 @@ public final class BlockUtils {
      * @param face the face of the block - see {@link org.geysermc.mcprotocollib.protocol.data.game.entity.object.Direction}
      * @return the block position with the block face accounted for
      */
-    public static Vector3i getBlockPosition(Vector3i blockPos, int face) {
+    public static Vector3i getBlockPosition(Vector3i blockPos, Direction face) {
         return switch (face) {
-            case 0 -> blockPos.sub(0, 1, 0);
+            case DOWN -> blockPos.sub(0, 1, 0);
-            case 1 -> blockPos.add(0, 1, 0);
+            case UP -> blockPos.add(0, 1, 0);
-            case 2 -> blockPos.sub(0, 0, 1);
+            case NORTH -> blockPos.sub(0, 0, 1);
-            case 3 -> blockPos.add(0, 0, 1);
+            case SOUTH -> blockPos.add(0, 0, 1);
-            case 4 -> blockPos.sub(1, 0, 0);
+            case WEST -> blockPos.sub(1, 0, 0);
-            case 5 -> blockPos.add(1, 0, 0);
+            case EAST -> blockPos.add(1, 0, 0);
-            default -> blockPos;
         };
     }