Fix computers/turtles not being dropped on explosion

Computer drops are currently[^1] implemented via a dynamic drop. To support this, we need to inject the dynamic drop into the loot parameters. We currently do this by implementing our own drop logic in playerWillDestroy[^2], manually creating the loot params and adding our additional drop. However, if the item is dropped via some other method (such as via explosions), we'll go through vanilla's drop logic and so never add the dynamic drop! The correct way to do this is to override getDrops to add the dynamic drop instead. I don't know why we didn't always do this -- the code in question was first written for MC 1.14[^3], when things were very different. [^1]: This is no longer the case on 1.21, where we can just copy capabilities. [^2]: We need to override vanilla's drop behaviour to ensure items are dropped in creative mode. [^3]: See 594bc4203c. Which probably means the bug has been around for 5 years :/.
2025-01-09 08:50:29 +00:00 · 2024-08-14 21:12:30 +01:00 · 2024-08-14 21:12:30 +01:00 · bb97c465d9
commit bb97c465d9
parent 9484315d37
4 changed files with 168 additions and 23 deletions
--- a/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlock.java
+++ b/projects/common/src/main/java/dan200/computercraft/shared/computer/blocks/AbstractComputerBlock.java
@ -35,9 +35,9 @@ import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.storage.loot.LootParams;
 import net.minecraft.world.level.storage.loot.parameters.LootContextParams;
 import net.minecraft.world.phys.BlockHitResult;
-import net.minecraft.world.phys.Vec3;

 import javax.annotation.Nullable;
+import java.util.List;

 public abstract class AbstractComputerBlock<T extends AbstractComputerBlockEntity> extends HorizontalDirectionalBlock implements IBundledRedstoneBlock, EntityBlock {
    private static final ResourceLocation DROP = new ResourceLocation(ComputerCraftAPI.MOD_ID, "computer");
@ -110,9 +110,19 @@ public abstract class AbstractComputerBlock<T extends AbstractComputerBlockEntit
        return super.getCloneItemStack(world, pos, state);
    }

+    @Override
+    @Deprecated
+    public List<ItemStack> getDrops(BlockState state, LootParams.Builder params) {
+        if (params.getOptionalParameter(LootContextParams.BLOCK_ENTITY) instanceof AbstractComputerBlockEntity computer) {
+            params = params.withDynamicDrop(DROP, out -> out.accept(getItem(computer)));
+        }
+
+        return super.getDrops(state, params);
+    }
+
    @Override
    public void playerDestroy(Level world, Player player, BlockPos pos, BlockState state, @Nullable BlockEntity tile, ItemStack tool) {
-        // Don't drop blocks here - see onBlockHarvested.
+        // Don't drop blocks here - see playerWillDestroy.
        player.awardStat(Stats.BLOCK_MINED.get(this));
        player.causeFoodExhaustion(0.005F);
    }
@ -120,25 +130,11 @@ public abstract class AbstractComputerBlock<T extends AbstractComputerBlockEntit
    @Override
    public void playerWillDestroy(Level world, BlockPos pos, BlockState state, Player player) {
        super.playerWillDestroy(world, pos, state, player);
-        if (!(world instanceof ServerLevel serverWorld)) return;
+        if (!(world instanceof ServerLevel serverLevel)) return;

        // We drop the item here instead of doing it in the harvest method, as we should
        // drop computers for creative players too.
-
-        var tile = world.getBlockEntity(pos);
-        if (tile instanceof AbstractComputerBlockEntity computer) {
-            var context = new LootParams.Builder(serverWorld)
-                .withParameter(LootContextParams.ORIGIN, Vec3.atCenterOf(pos))
-                .withParameter(LootContextParams.TOOL, player.getMainHandItem())
-                .withParameter(LootContextParams.THIS_ENTITY, player)
-                .withParameter(LootContextParams.BLOCK_ENTITY, tile)
-                .withDynamicDrop(DROP, out -> out.accept(getItem(computer)));
-            for (var item : state.getDrops(context)) {
-                popResource(world, pos, item);
-            }
-
-            state.spawnAfterBreak(serverWorld, pos, player.getMainHandItem(), true);
-        }
+        dropResources(state, serverLevel, pos, world.getBlockEntity(pos));
    }

    @Override
--- a/projects/common/src/testMod/java/dan200/computercraft/mixin/gametest/GameTestHelperAccessor.java
+++ b/projects/common/src/testMod/java/dan200/computercraft/mixin/gametest/GameTestHelperAccessor.java
@ -6,16 +6,11 @@ package dan200.computercraft.mixin.gametest;

 import net.minecraft.gametest.framework.GameTestHelper;
 import net.minecraft.gametest.framework.GameTestInfo;
-import net.minecraft.world.phys.AABB;
 import org.spongepowered.asm.mixin.Mixin;
 import org.spongepowered.asm.mixin.gen.Accessor;
-import org.spongepowered.asm.mixin.gen.Invoker;

@Mixin(GameTestHelper.class)
 public interface GameTestHelperAccessor {
-    @Invoker
-    AABB callGetBounds();
-
    @Accessor
    GameTestInfo getTestInfo();
 }
--- a/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Computer_Test.kt
+++ b/projects/common/src/testMod/kotlin/dan200/computercraft/gametest/Computer_Test.kt
@ -19,9 +19,11 @@ import net.minecraft.gametest.framework.GameTest
 import net.minecraft.gametest.framework.GameTestHelper
 import net.minecraft.world.item.ItemStack
 import net.minecraft.world.item.Items
+import net.minecraft.world.level.Level
 import net.minecraft.world.level.block.Blocks
 import net.minecraft.world.level.block.LeverBlock
 import net.minecraft.world.level.block.RedstoneLampBlock
+import net.minecraft.world.phys.Vec3
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.lwjgl.glfw.GLFW
@ -115,6 +117,20 @@ class Computer_Test {
        thenOnComputer { callPeripheral("right", "size").assertArrayEquals(54) }
    }

+    /**
+     * Tests a computer item is dropped on explosion.
+     */
+    @GameTest
+    fun Drops_on_explosion(context: GameTestHelper) = context.sequence {
+        thenExecute {
+            val pos = BlockPos(2, 2, 2)
+            val explosionPos = Vec3.atCenterOf(context.absolutePos(pos))
+            context.level.explode(null, explosionPos.x, explosionPos.y, explosionPos.z, 2.0f, Level.ExplosionInteraction.TNT)
+
+            context.assertItemEntityPresent(ModRegistry.Items.COMPUTER_NORMAL.get(), pos, 1.0)
+        }
+    }
+
    /**
     * Check the client can open the computer UI and interact with it.
     */
--- a/projects/common/src/testMod/resources/data/cctest/structures/computer_test.drops_on_explosion.snbt
+++ b/projects/common/src/testMod/resources/data/cctest/structures/computer_test.drops_on_explosion.snbt
@ -0,0 +1,138 @@
+{
+    DataVersion: 3465,
+    size: [5, 5, 5],
+    data: [
+        {pos: [0, 0, 0], state: "minecraft:obsidian"},
+        {pos: [0, 0, 1], state: "minecraft:obsidian"},
+        {pos: [0, 0, 2], state: "minecraft:obsidian"},
+        {pos: [0, 0, 3], state: "minecraft:obsidian"},
+        {pos: [0, 0, 4], state: "minecraft:obsidian"},
+        {pos: [1, 0, 0], state: "minecraft:obsidian"},
+        {pos: [1, 0, 1], state: "minecraft:obsidian"},
+        {pos: [1, 0, 2], state: "minecraft:obsidian"},
+        {pos: [1, 0, 3], state: "minecraft:obsidian"},
+        {pos: [1, 0, 4], state: "minecraft:obsidian"},
+        {pos: [2, 0, 0], state: "minecraft:obsidian"},
+        {pos: [2, 0, 1], state: "minecraft:obsidian"},
+        {pos: [2, 0, 2], state: "minecraft:obsidian"},
+        {pos: [2, 0, 3], state: "minecraft:obsidian"},
+        {pos: [2, 0, 4], state: "minecraft:obsidian"},
+        {pos: [3, 0, 0], state: "minecraft:obsidian"},
+        {pos: [3, 0, 1], state: "minecraft:obsidian"},
+        {pos: [3, 0, 2], state: "minecraft:obsidian"},
+        {pos: [3, 0, 3], state: "minecraft:obsidian"},
+        {pos: [3, 0, 4], state: "minecraft:obsidian"},
+        {pos: [4, 0, 0], state: "minecraft:obsidian"},
+        {pos: [4, 0, 1], state: "minecraft:obsidian"},
+        {pos: [4, 0, 2], state: "minecraft:obsidian"},
+        {pos: [4, 0, 3], state: "minecraft:obsidian"},
+        {pos: [4, 0, 4], state: "minecraft:obsidian"},
+        {pos: [0, 1, 0], state: "minecraft:barrier"},
+        {pos: [0, 1, 1], state: "minecraft:barrier"},
+        {pos: [0, 1, 2], state: "minecraft:barrier"},
+        {pos: [0, 1, 3], state: "minecraft:barrier"},
+        {pos: [0, 1, 4], state: "minecraft:barrier"},
+        {pos: [1, 1, 0], state: "minecraft:barrier"},
+        {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:barrier"},
+        {pos: [2, 1, 0], state: "minecraft:barrier"},
+        {pos: [2, 1, 1], state: "minecraft:air"},
+        {pos: [2, 1, 2], state: "computercraft:computer_normal{facing:east,state:off}", nbt: {On: 0b, id: "computercraft:computer_normal"}},
+        {pos: [2, 1, 3], state: "minecraft:air"},
+        {pos: [2, 1, 4], state: "minecraft:barrier"},
+        {pos: [3, 1, 0], state: "minecraft:barrier"},
+        {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:barrier"},
+        {pos: [4, 1, 0], state: "minecraft:barrier"},
+        {pos: [4, 1, 1], state: "minecraft:barrier"},
+        {pos: [4, 1, 2], state: "minecraft:barrier"},
+        {pos: [4, 1, 3], state: "minecraft:barrier"},
+        {pos: [4, 1, 4], state: "minecraft:barrier"},
+        {pos: [0, 2, 0], state: "minecraft:barrier"},
+        {pos: [0, 2, 1], state: "minecraft:barrier"},
+        {pos: [0, 2, 2], state: "minecraft:air"},
+        {pos: [0, 2, 3], state: "minecraft:barrier"},
+        {pos: [0, 2, 4], state: "minecraft:barrier"},
+        {pos: [1, 2, 0], state: "minecraft:barrier"},
+        {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:barrier"},
+        {pos: [2, 2, 0], state: "minecraft:barrier"},
+        {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:barrier"},
+        {pos: [3, 2, 0], state: "minecraft:barrier"},
+        {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:barrier"},
+        {pos: [4, 2, 0], state: "minecraft:barrier"},
+        {pos: [4, 2, 1], state: "minecraft:barrier"},
+        {pos: [4, 2, 2], state: "minecraft:barrier"},
+        {pos: [4, 2, 3], state: "minecraft:barrier"},
+        {pos: [4, 2, 4], state: "minecraft:barrier"},
+        {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:obsidian",
+        "minecraft:barrier",
+        "minecraft:air",
+        "computercraft:computer_normal{facing:east,state:off}"
+    ]
+}