Simplify cable/modem block breaking code

Instead of taking control of the breaking logic in all cases, we now
only do so when we have both a cable and modem. This allows us to fall
back to default vanilla behaviour and so correctly drop the modem/cable
item.

projects/common/src/main/java/dan200/computercraft/shared/peripheral/modem/wired/CableBlock.java

@@ -97,45 +97,46 @@ public class CableBlock extends Block implements SimpleWaterloggedBlock, EntityB
     @ForgeOverride
     public boolean onDestroyedByPlayer(BlockState state, Level world, BlockPos pos, Player player, boolean willHarvest, FluidState fluid) {
         playerWillDestroy(world, pos, state, player);
-        return onDestroyedByPlayer(state, world, pos, player, fluid);
+        if (onCustomDestroyBlock(state, world, pos, player)) {
-    }
+            return false;
-
-    public boolean onDestroyedByPlayer(BlockState state, Level world, BlockPos pos, Player player, FluidState fluid) {
-        if (state.getValue(CABLE) && state.getValue(MODEM).getFacing() != null) {
-            var hit = world.clip(new ClipContext(
-                WorldUtil.getRayStart(player), WorldUtil.getRayEnd(player),
-                ClipContext.Block.COLLIDER, ClipContext.Fluid.NONE, player
-            ));
-            if (hit.getType() == HitResult.Type.BLOCK) {
-                var tile = world.getBlockEntity(pos);
-                if (tile instanceof CableBlockEntity cable && tile.hasLevel()) {
-                    ItemStack item;
-                    BlockState newState;
-
-                    if (WorldUtil.isVecInside(CableShapes.getModemShape(state), hit.getLocation().subtract(pos.getX(), pos.getY(), pos.getZ()))) {
-                        newState = state.setValue(MODEM, CableModemVariant.None);
-                        item = new ItemStack(ModRegistry.Items.WIRED_MODEM.get());
-                    } else {
-                        newState = state.setValue(CABLE, false);
-                        item = new ItemStack(ModRegistry.Items.CABLE.get());
-                    }
-
-                    world.setBlock(pos, correctConnections(world, pos, newState), 3);
-
-                    cable.modemChanged();
-                    cable.connectionsChanged();
-                    if (!world.isClientSide && !player.getAbilities().instabuild) {
-                        Block.popResource(world, pos, item);
-                    }
-
-                    return false;
-                }
-            }
         }
 
         return world.setBlock(pos, fluid.createLegacyBlock(), world.isClientSide ? UPDATE_ALL_IMMEDIATE : UPDATE_ALL);
     }
 
+    public boolean onCustomDestroyBlock(BlockState state, Level world, BlockPos pos, Player player) {
+        if (!state.getValue(CABLE) || state.getValue(MODEM).getFacing() == null) return false;
+
+        var hit = world.clip(new ClipContext(
+            WorldUtil.getRayStart(player), WorldUtil.getRayEnd(player),
+            ClipContext.Block.COLLIDER, ClipContext.Fluid.NONE, player
+        ));
+        if (hit.getType() != HitResult.Type.BLOCK) return false;
+
+        var tile = world.getBlockEntity(pos);
+        if (!(tile instanceof CableBlockEntity cable) || !tile.hasLevel()) return false;
+
+        ItemStack item;
+        BlockState newState;
+        if (WorldUtil.isVecInside(CableShapes.getModemShape(state), hit.getLocation().subtract(pos.getX(), pos.getY(), pos.getZ()))) {
+            newState = state.setValue(MODEM, CableModemVariant.None);
+            item = new ItemStack(ModRegistry.Items.WIRED_MODEM.get());
+        } else {
+            newState = state.setValue(CABLE, false);
+            item = new ItemStack(ModRegistry.Items.CABLE.get());
+        }
+
+        world.setBlock(pos, correctConnections(world, pos, newState), 3);
+
+        cable.modemChanged();
+        cable.connectionsChanged();
+        if (!world.isClientSide && !player.getAbilities().instabuild) {
+            Block.popResource(world, pos, item);
+        }
+
+        return true;
+    }
+
     @Override
     @Deprecated
     public ItemStack getCloneItemStack(BlockGetter world, BlockPos pos, BlockState state) {

projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt

@@ -15,6 +15,8 @@ import dan200.computercraft.mixin.gametest.GameTestHelperAccessor
 import dan200.computercraft.mixin.gametest.GameTestInfoAccessor
 import dan200.computercraft.shared.ModRegistry
 import dan200.computercraft.shared.media.items.PrintoutItem
+import dan200.computercraft.shared.peripheral.modem.wired.CableBlock
+import dan200.computercraft.shared.peripheral.modem.wired.CableModemVariant
 import dan200.computercraft.shared.peripheral.monitor.MonitorBlock
 import dan200.computercraft.shared.peripheral.monitor.MonitorEdgeState
 import dan200.computercraft.shared.turtle.apis.TurtleAPI
@@ -89,7 +91,7 @@ class Turtle_Test {
                 .assertArrayEquals(true, message = "Placed oak fence")
         }
         thenExecute {
-            helper.assertBlockIs(BlockPos(2, 2, 2), { it.block == Blocks.OAK_FENCE && it.getValue(FenceBlock.WATERLOGGED) })
+            helper.assertBlockIs(BlockPos(2, 2, 2)) { it.block == Blocks.OAK_FENCE && it.getValue(FenceBlock.WATERLOGGED) }
         }
     }
 
@@ -123,6 +125,33 @@ class Turtle_Test {
         thenExecute { helper.assertBlockPresent(Blocks.FARMLAND, BlockPos(1, 2, 1)) }
     }
 
+    /**
+     * Checks turtles break cables in two parts.
+     */
+    @GameTest
+    fun Break_cable(helper: GameTestHelper) = helper.sequence {
+        thenOnComputer { turtle.dig(Optional.empty()).await() }
+        thenExecute {
+            helper.assertBlockIs(BlockPos(2, 2, 3)) {
+                it.block == ModRegistry.Blocks.CABLE.get() && !it.getValue(CableBlock.CABLE) && it.getValue(CableBlock.MODEM) == CableModemVariant.DownOff
+            }
+
+            helper.assertContainerExactly(BlockPos(2, 2, 2), listOf(ItemStack(ModRegistry.Items.CABLE.get())))
+        }
+        thenOnComputer { turtle.dig(Optional.empty()).await().assertArrayEquals(true) }
+        thenExecute {
+            helper.assertBlockPresent(Blocks.AIR, BlockPos(2, 2, 3))
+
+            helper.assertContainerExactly(
+                BlockPos(2, 2, 2),
+                listOf(
+                    ItemStack(ModRegistry.Items.CABLE.get()),
+                    ItemStack(ModRegistry.Items.WIRED_MODEM.get()),
+                ),
+            )
+        }
+    }
+
     /**
      * Checks turtles can place monitors
      *
@@ -366,7 +395,7 @@ class Turtle_Test {
         thenOnComputer { turtle.forward().await().assertArrayEquals(true, message = "Turtle moved forward") }
         thenExecute {
             // Assert we're no longer waterlogged and we've left a source block.
-            helper.assertBlockIs(BlockPos(2, 2, 2), { it.block == Blocks.WATER && it.fluidState.isSource })
+            helper.assertBlockIs(BlockPos(2, 2, 2)) { it.block == Blocks.WATER && it.fluidState.isSource }
             helper.assertBlockHas(BlockPos(2, 2, 3), WaterloggableHelpers.WATERLOGGED, false)
         }
     }

projects/common/src/testMod/kotlin/dan200/computercraft/gametest/api/TestExtensions.kt

@@ -143,7 +143,12 @@ private fun GameTestHelper.fail(message: String?, detail: String, pos: BlockPos)
 /**
  * A version of [GameTestHelper.assertBlockState] which also includes the current block state.
  */
-fun GameTestHelper.assertBlockIs(pos: BlockPos, predicate: (BlockState) -> Boolean, message: String = "") {
+fun GameTestHelper.assertBlockIs(pos: BlockPos, predicate: (BlockState) -> Boolean) = assertBlockIs(pos, predicate, "")
+
+/**
+ * A version of [GameTestHelper.assertBlockState] which also includes the current block state.
+ */
+fun GameTestHelper.assertBlockIs(pos: BlockPos, predicate: (BlockState) -> Boolean, message: String) {
     val state = getBlockState(pos)
     if (!predicate(state)) fail(message, state.toString(), pos)
 }

projects/common/src/testMod/resources/data/cctest/structures/turtle_test.break_cable.snbt

@@ -0,0 +1,138 @@
+{
+    DataVersion: 3218,
+    size: [5, 5, 5],
+    data: [
+        {pos: [0, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [0, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [1, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [2, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [3, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 0], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 1], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 2], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 3], state: "minecraft:polished_andesite"},
+        {pos: [4, 0, 4], state: "minecraft:polished_andesite"},
+        {pos: [0, 1, 0], state: "minecraft:air"},
+        {pos: [0, 1, 1], state: "minecraft:air"},
+        {pos: [0, 1, 2], state: "minecraft:air"},
+        {pos: [0, 1, 3], state: "minecraft:air"},
+        {pos: [0, 1, 4], state: "minecraft:air"},
+        {pos: [1, 1, 0], state: "minecraft:air"},
+        {pos: [1, 1, 1], state: "minecraft:air"},
+        {pos: [1, 1, 2], state: "minecraft:air"},
+        {pos: [1, 1, 3], state: "minecraft:air"},
+        {pos: [1, 1, 4], state: "minecraft:air"},
+        {pos: [2, 1, 0], state: "minecraft:air"},
+        {pos: [2, 1, 1], state: "minecraft:air"},
+        {pos: [2, 1, 2], state: "computercraft:turtle_normal{facing:south,waterlogged:false}", nbt: {ComputerId: 1, Fuel: 80, Items: [], Label: "turtle_test.break_cable", LeftUpgrade: "minecraft:diamond_pickaxe", On: 1b, Owner: {LowerId: -6876936588741668278L, Name: "Dev", UpperId: 4039158846114182220L}, Slot: 0, id: "computercraft:turtle_normal"}},
+        {pos: [2, 1, 3], state: "computercraft:cable{cable:true,down:true,east:false,modem:down_off,north:false,south:false,up:false,waterlogged:false,west:false}", nbt: {PeirpheralAccess: 0b, id: "computercraft:cable"}},
+        {pos: [2, 1, 4], state: "minecraft:air"},
+        {pos: [3, 1, 0], state: "minecraft:air"},
+        {pos: [3, 1, 1], state: "minecraft:air"},
+        {pos: [3, 1, 2], state: "minecraft:air"},
+        {pos: [3, 1, 3], state: "minecraft:air"},
+        {pos: [3, 1, 4], state: "minecraft:air"},
+        {pos: [4, 1, 0], state: "minecraft:air"},
+        {pos: [4, 1, 1], state: "minecraft:air"},
+        {pos: [4, 1, 2], state: "minecraft:air"},
+        {pos: [4, 1, 3], state: "minecraft:air"},
+        {pos: [4, 1, 4], state: "minecraft:air"},
+        {pos: [0, 2, 0], state: "minecraft:air"},
+        {pos: [0, 2, 1], state: "minecraft:air"},
+        {pos: [0, 2, 2], state: "minecraft:air"},
+        {pos: [0, 2, 3], state: "minecraft:air"},
+        {pos: [0, 2, 4], state: "minecraft:air"},
+        {pos: [1, 2, 0], state: "minecraft:air"},
+        {pos: [1, 2, 1], state: "minecraft:air"},
+        {pos: [1, 2, 2], state: "minecraft:air"},
+        {pos: [1, 2, 3], state: "minecraft:air"},
+        {pos: [1, 2, 4], state: "minecraft:air"},
+        {pos: [2, 2, 0], state: "minecraft:air"},
+        {pos: [2, 2, 1], state: "minecraft:air"},
+        {pos: [2, 2, 2], state: "minecraft:air"},
+        {pos: [2, 2, 3], state: "minecraft:air"},
+        {pos: [2, 2, 4], state: "minecraft:air"},
+        {pos: [3, 2, 0], state: "minecraft:air"},
+        {pos: [3, 2, 1], state: "minecraft:air"},
+        {pos: [3, 2, 2], state: "minecraft:air"},
+        {pos: [3, 2, 3], state: "minecraft:air"},
+        {pos: [3, 2, 4], state: "minecraft:air"},
+        {pos: [4, 2, 0], state: "minecraft:air"},
+        {pos: [4, 2, 1], state: "minecraft:air"},
+        {pos: [4, 2, 2], state: "minecraft:air"},
+        {pos: [4, 2, 3], state: "minecraft:air"},
+        {pos: [4, 2, 4], state: "minecraft:air"},
+        {pos: [0, 3, 0], state: "minecraft:air"},
+        {pos: [0, 3, 1], state: "minecraft:air"},
+        {pos: [0, 3, 2], state: "minecraft:air"},
+        {pos: [0, 3, 3], state: "minecraft:air"},
+        {pos: [0, 3, 4], state: "minecraft:air"},
+        {pos: [1, 3, 0], state: "minecraft:air"},
+        {pos: [1, 3, 1], state: "minecraft:air"},
+        {pos: [1, 3, 2], state: "minecraft:air"},
+        {pos: [1, 3, 3], state: "minecraft:air"},
+        {pos: [1, 3, 4], state: "minecraft:air"},
+        {pos: [2, 3, 0], state: "minecraft:air"},
+        {pos: [2, 3, 1], state: "minecraft:air"},
+        {pos: [2, 3, 2], state: "minecraft:air"},
+        {pos: [2, 3, 3], state: "minecraft:air"},
+        {pos: [2, 3, 4], state: "minecraft:air"},
+        {pos: [3, 3, 0], state: "minecraft:air"},
+        {pos: [3, 3, 1], state: "minecraft:air"},
+        {pos: [3, 3, 2], state: "minecraft:air"},
+        {pos: [3, 3, 3], state: "minecraft:air"},
+        {pos: [3, 3, 4], state: "minecraft:air"},
+        {pos: [4, 3, 0], state: "minecraft:air"},
+        {pos: [4, 3, 1], state: "minecraft:air"},
+        {pos: [4, 3, 2], state: "minecraft:air"},
+        {pos: [4, 3, 3], state: "minecraft:air"},
+        {pos: [4, 3, 4], state: "minecraft:air"},
+        {pos: [0, 4, 0], state: "minecraft:air"},
+        {pos: [0, 4, 1], state: "minecraft:air"},
+        {pos: [0, 4, 2], state: "minecraft:air"},
+        {pos: [0, 4, 3], state: "minecraft:air"},
+        {pos: [0, 4, 4], state: "minecraft:air"},
+        {pos: [1, 4, 0], state: "minecraft:air"},
+        {pos: [1, 4, 1], state: "minecraft:air"},
+        {pos: [1, 4, 2], state: "minecraft:air"},
+        {pos: [1, 4, 3], state: "minecraft:air"},
+        {pos: [1, 4, 4], state: "minecraft:air"},
+        {pos: [2, 4, 0], state: "minecraft:air"},
+        {pos: [2, 4, 1], state: "minecraft:air"},
+        {pos: [2, 4, 2], state: "minecraft:air"},
+        {pos: [2, 4, 3], state: "minecraft:air"},
+        {pos: [2, 4, 4], state: "minecraft:air"},
+        {pos: [3, 4, 0], state: "minecraft:air"},
+        {pos: [3, 4, 1], state: "minecraft:air"},
+        {pos: [3, 4, 2], state: "minecraft:air"},
+        {pos: [3, 4, 3], state: "minecraft:air"},
+        {pos: [3, 4, 4], state: "minecraft:air"},
+        {pos: [4, 4, 0], state: "minecraft:air"},
+        {pos: [4, 4, 1], state: "minecraft:air"},
+        {pos: [4, 4, 2], state: "minecraft:air"},
+        {pos: [4, 4, 3], state: "minecraft:air"},
+        {pos: [4, 4, 4], state: "minecraft:air"}
+    ],
+    entities: [],
+    palette: [
+        "minecraft:polished_andesite",
+        "minecraft:air",
+        "computercraft:turtle_normal{facing:south,waterlogged:false}",
+        "computercraft:cable{cable:true,down:true,east:false,modem:down_off,north:false,south:false,up:false,waterlogged:false,west:false}"
+    ]
+}

projects/common/src/testMod/resources/data/cctest/structures/turtle_test.move_preserves_state.snbt

@@ -131,9 +131,7 @@
     entities: [],
     palette: [
         "minecraft:polished_andesite",
-        "minecraft:dirt",
         "minecraft:air",
-        "minecraft:grass",
         "computercraft:turtle_normal{facing:south,waterlogged:false}"
     ]
 }

projects/fabric/src/main/java/dan200/computercraft/shared/FabricCommonHooks.java

@@ -34,13 +34,8 @@ public class FabricCommonHooks {
     private static final Gson GSON = new GsonBuilder().create();
     private static final Logger LOGGER = LoggerFactory.getLogger(FabricCommonHooks.class);
 
-    public static boolean onBlockDestroy(Level world, Player player, BlockPos pos, BlockState state, @Nullable BlockEntity blockEntity) {
+    public static boolean onBlockDestroy(Level level, Player player, BlockPos pos, BlockState state, @Nullable BlockEntity blockEntity) {
-        if (!(state.getBlock() instanceof CableBlock cable)) return true;
+        return !(state.getBlock() instanceof CableBlock cable) || !cable.onCustomDestroyBlock(state, level, pos, player);
-        if (cable.onDestroyedByPlayer(state, world, pos, player, state.getFluidState())) {
-            state.getBlock().destroy(world, pos, state);
-        }
-
-        return false;
     }
 
     /**