From b7a8432cfb7106475f0511b58e89690c526868b9 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Thu, 15 Aug 2024 10:32:54 +0100 Subject: [PATCH] Fix turtles capturing their own drops when broken There's a whole load of gnarly issues that occur when a turtle is broken mid-dig/attack (normally due to an explosion). We fixed most of these in 24af36743d08fcdb58439c52bf587b33ed828263, but not perfectly. Part of the fix here was to not capture drops if the turtle BE has been removed. However, on removal, turtles drop their items *before* removing the BE. This meant that the drop consumer still triggered, and attempted to insert items back into the turtle. This bug only triggers if the turtle contains a stack larger than 10 (ish, I think) items, which is possibly why I'd never reproduced before. We now drop items after removing the BE, which resolves the issue. Fixes #1936. --- .../shared/turtle/blocks/TurtleBlock.java | 10 +++++++--- .../dan200/computercraft/gametest/Turtle_Test.kt | 9 +++++---- .../structures/turtle_test.breaks_exploding_block.snbt | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/projects/common/src/main/java/dan200/computercraft/shared/turtle/blocks/TurtleBlock.java b/projects/common/src/main/java/dan200/computercraft/shared/turtle/blocks/TurtleBlock.java index 4694a8183..2456a76ae 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/turtle/blocks/TurtleBlock.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/turtle/blocks/TurtleBlock.java @@ -125,11 +125,15 @@ public class TurtleBlock extends AbstractComputerBlock implem public final void onRemove(BlockState state, Level level, BlockPos pos, BlockState newState, boolean isMoving) { if (state.is(newState.getBlock())) return; - if (!level.isClientSide && level.getBlockEntity(pos) instanceof TurtleBlockEntity turtle && !turtle.hasMoved()) { - Containers.dropContents(level, pos, turtle); - } + // Most blocks drop items and then remove the BE. However, if a turtle is consuming drops right now, that can + // lead to loops where it tries to insert an item back into the inventory. To prevent this, take a reference to + // the turtle BE now, remove it, and then drop the items. + var turtle = !level.isClientSide && level.getBlockEntity(pos) instanceof TurtleBlockEntity t && !t.hasMoved() + ? t : null; super.onRemove(state, level, pos, newState, isMoving); + + if (turtle != null) Containers.dropContents(level, pos, turtle); } @Override diff --git a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt index 227ece5d0..e1619029f 100644 --- a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt +++ b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Turtle_Test.kt @@ -696,15 +696,16 @@ class Turtle_Test { /** * Tests a turtle can break a block that explodes, causing the turtle itself to explode. * - * This attempts to test [#585](https://github.com/cc-tweaked/CC-Tweaked/issues/585) and other similar issues. It's - * not clear if this is a good test case, as that bug does not seem reliably reproducible, but it's at least a good - * sanity check. + * @see [#585](https://github.com/cc-tweaked/CC-Tweaked/issues/585). */ @GameTest fun Breaks_exploding_block(context: GameTestHelper) = context.sequence { thenOnComputer { turtle.dig(Optional.empty()) } thenIdle(2) - thenExecute { context.assertItemEntityPresent(ModRegistry.Items.TURTLE_NORMAL.get(), BlockPos(2, 2, 2), 1.0) } + thenExecute { + context.assertItemEntityCountIs(ModRegistry.Items.TURTLE_NORMAL.get(), BlockPos(2, 2, 2), 1.0, 1) + context.assertItemEntityCountIs(Items.BONE_BLOCK, BlockPos(2, 2, 2), 1.0, 65) + } } /** diff --git a/projects/common/src/testMod/resources/data/cctest/structures/turtle_test.breaks_exploding_block.snbt b/projects/common/src/testMod/resources/data/cctest/structures/turtle_test.breaks_exploding_block.snbt index 2c20fc77a..3e315e552 100644 --- a/projects/common/src/testMod/resources/data/cctest/structures/turtle_test.breaks_exploding_block.snbt +++ b/projects/common/src/testMod/resources/data/cctest/structures/turtle_test.breaks_exploding_block.snbt @@ -39,7 +39,7 @@ {pos: [1, 1, 4], state: "minecraft:barrier"}, {pos: [2, 1, 0], state: "minecraft:barrier"}, {pos: [2, 1, 1], state: "minecraft:air"}, - {pos: [2, 1, 2], state: "computercraft:turtle_normal{facing:south,waterlogged:false}", nbt: {ComputerId: 1, Fuel: 0, Items: [], Label: "turtle_test.breaks_exploding_block", LeftUpgrade: "minecraft:diamond_pickaxe", LeftUpgradeNbt: {Tag: {Damage: 0}}, On: 1b, Owner: {LowerId: -5670393268852517359L, Name: "Player172", UpperId: 3578583684139923613L}, Slot: 0, id: "computercraft:turtle_normal"}}, + {pos: [2, 1, 2], state: "computercraft:turtle_normal{facing:south,waterlogged:false}", nbt: {ComputerId: 1, Fuel: 0, Items: [], Label: "turtle_test.breaks_exploding_block", LeftUpgrade: "minecraft:diamond_pickaxe", LeftUpgradeNbt: {Tag: {Damage: 0}}, On: 1b, Owner: {LowerId: -5670393268852517359L, Name: "Player172", UpperId: 3578583684139923613L}, Slot: 0, Items: [{Count: 64b, Slot: 0b, id: "minecraft:bone_block"}], id: "computercraft:turtle_normal"}}, {pos: [2, 1, 3], state: "minecraft:bone_block{axis:y}"}, {pos: [2, 1, 4], state: "minecraft:barrier"}, {pos: [3, 1, 0], state: "minecraft:barrier"},