From d138d9c4a514ab0d95dae9726ab301bf2c3c1cec Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 2 Jul 2023 11:02:10 +0100 Subject: [PATCH] Preserve item NBT for turtle tools This is a pre-requisite for #1501, and some other refactorings I want to do. Also fix items in the turtle upgrade slots vanishing. We now explicitly invalidate the cache when setting the item. --- .../turtle/inventory/UpgradeContainer.java | 23 +++++++++++-------- .../shared/turtle/upgrades/TurtleTool.java | 21 +++++++++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/projects/common/src/main/java/dan200/computercraft/shared/turtle/inventory/UpgradeContainer.java b/projects/common/src/main/java/dan200/computercraft/shared/turtle/inventory/UpgradeContainer.java index 0dd06e896..a0f0b6987 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/turtle/inventory/UpgradeContainer.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/turtle/inventory/UpgradeContainer.java @@ -14,6 +14,7 @@ import net.minecraft.world.Container; import net.minecraft.world.entity.player.Player; import net.minecraft.world.item.ItemStack; +import javax.annotation.Nullable; import java.util.Arrays; import java.util.List; @@ -46,24 +47,28 @@ class UpgradeContainer implements Container { @Override public ItemStack getItem(int slot) { var side = getSide(slot); - var upgrade = turtle.getUpgrade(side); + var upgrade = turtle.getUpgradeWithData(side); if (upgrade == null) return ItemStack.EMPTY; - // We don't want to return getCraftingItem directly here, as consumers may mutate the stack (they shouldn't!, - // but if they do it's a pain to track down). To avoid recreating the stack each tick, we maintain a simple - // cache. We use an inlined getUpgradeData here to avoid the additional defensive copy. - var upgradeData = UpgradeData.of(upgrade, turtle.getUpgradeNBTData(side)); - if (upgradeData.equals(lastUpgrade.get(slot))) return lastStack.get(slot); + // We don't want to return getUpgradeItem directly here, as we'd end up recreating the stack each tick. To + // avoid that, we maintain a simple cache. + if (upgrade.equals(lastUpgrade.get(slot))) return lastStack.get(slot); - var stack = upgradeData.getUpgradeItem(); - lastUpgrade.set(slot, upgradeData.copy()); + return setUpgradeStack(slot, upgrade); + } + + private ItemStack setUpgradeStack(int slot, @Nullable UpgradeData upgrade) { + var stack = upgrade == null ? ItemStack.EMPTY : upgrade.getUpgradeItem(); + lastUpgrade.set(slot, upgrade); lastStack.set(slot, stack); return stack; } @Override public void setItem(int slot, ItemStack itemStack) { - turtle.setUpgradeWithData(getSide(slot), TurtleUpgrades.instance().get(itemStack)); + var upgrade = TurtleUpgrades.instance().get(itemStack); + turtle.setUpgradeWithData(getSide(slot), upgrade); + setUpgradeStack(slot, upgrade); } @Override diff --git a/projects/common/src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java b/projects/common/src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java index 9f8b80150..7f75e7456 100644 --- a/projects/common/src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java +++ b/projects/common/src/main/java/dan200/computercraft/shared/turtle/upgrades/TurtleTool.java @@ -14,6 +14,7 @@ import dan200.computercraft.shared.util.DropConsumer; import dan200.computercraft.shared.util.WorldUtil; import net.minecraft.core.BlockPos; import net.minecraft.core.Direction; +import net.minecraft.nbt.CompoundTag; import net.minecraft.resources.ResourceLocation; import net.minecraft.server.level.ServerLevel; import net.minecraft.tags.TagKey; @@ -59,7 +60,7 @@ public class TurtleTool extends AbstractTurtleUpgrade { // Check we've not got anything vaguely interesting on the item. We allow other mods to add their // own NBT, with the understanding such details will be lost to the mist of time. - if (stack.isDamaged() || stack.isEnchanted() || stack.hasCustomHoverName()) return false; + if (stack.isDamaged() || stack.isEnchanted()) return false; if (tag.contains("AttributeModifiers", TAG_LIST) && !tag.getList("AttributeModifiers", TAG_COMPOUND).isEmpty()) { return false; } @@ -67,6 +68,21 @@ public class TurtleTool extends AbstractTurtleUpgrade { return true; } + @Override + public CompoundTag getUpgradeData(ItemStack stack) { + // Just use the current item's tag. + var itemTag = stack.getTag(); + return itemTag == null ? new CompoundTag() : itemTag; + } + + @Override + public ItemStack getUpgradeItem(CompoundTag upgradeData) { + // Copy upgrade data back to the item. + var item = super.getUpgradeItem(upgradeData); + if (!upgradeData.isEmpty()) item.setTag(upgradeData); + return item; + } + @Override public TurtleCommandResult useTool(ITurtleAccess turtle, TurtleSide side, TurtleVerb verb, Direction direction) { return switch (verb) { @@ -177,7 +193,8 @@ public class TurtleTool extends AbstractTurtleUpgrade { } private static boolean isTriviallyBreakable(BlockGetter reader, BlockPos pos, BlockState state) { - return state.is(ComputerCraftTags.Blocks.TURTLE_ALWAYS_BREAKABLE) + return + state.is(ComputerCraftTags.Blocks.TURTLE_ALWAYS_BREAKABLE) // Allow breaking any "instabreak" block. || state.getDestroySpeed(reader, pos) == 0; }