.item() and getItem() now creates a defensive copy

CraftYourTown · Nov 16, 2024 · 3c1d8fd · 3c1d8fd
1 parent 415f690
commit 3c1d8fd
Show file tree

Hide file tree

Showing 10 changed files with 16 additions and 36 deletions.
diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java
@@ -219,7 +219,7 @@ protected SlimefunItem(ItemGroup itemGroup, ItemStack item, String id, RecipeTyp
      * @return The {@link ItemStack} that this {@link SlimefunItem} represents
      */
     public @Nonnull ItemStack getItem() {
-        return itemStackTemplate;
+        return itemStackTemplate.clone();
     }
 
     /**
@@ -500,12 +500,6 @@ public void register(@Nonnull SlimefunAddon addon) {
                 this.itemHandlers.clear();
             }
 
-            // TODO: ItemStack Lock - find a way to lock the item or maybe clone the item everytime it is obtained
-            // Lock the SlimefunItemStack from any accidental manipulations
-            //if (itemStackTemplate instanceof SlimefunItemStack stack && isItemStackImmutable()) {
-            //    stack.lock();
-            //}
-
             postRegister();
 
             // handle runtime-registrations / auto-loading

diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItemStack.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItemStack.java
@@ -61,7 +61,6 @@ public class SlimefunItemStack {
     private String id;
     private ItemMetaSnapshot itemMetaSnapshot;
 
-    private boolean locked = false;
     private String texture = null;
 
     public SlimefunItemStack(@Nonnull String id, @Nonnull ItemStack item) {
@@ -255,32 +254,19 @@ public SlimefunItemStack(@Nonnull String id, @Nonnull String texture, @Nonnull C
     }
 
     public boolean setItemMeta(ItemMeta meta) {
-        validate();
         itemMetaSnapshot = new ItemMetaSnapshot(meta);
 
         return delegate.setItemMeta(meta);
     }
 
     public void setType(Material type) {
-        validate();
         delegate.setType(type);
     }
 
     public void setAmount(int amount) {
-        validate();
         delegate.setAmount(amount);
     }
 
-    private void validate() {
-        if (locked) {
-            throw new WrongItemStackException(id + " is not mutable.");
-        }
-    }
-
-    public void lock() {
-        locked = true;
-    }
-
     public @Nonnull Optional<String> getSkullTexture() {
         return Optional.ofNullable(texture);
     }

diff --git a/...in/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java b/...in/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java
@@ -69,7 +69,7 @@ public void onExecute(CommandSender sender, String[] args) {
                     }
 
                     Slimefun.runSync(() -> {
-                        ItemStack item = SlimefunItems.RESTORED_BACKPACK.cloneItem();
+                        ItemStack item = SlimefunItems.RESTORED_BACKPACK.item();
                         Slimefun.getBackpackListener().setBackpackId(backpackOwner, item, 2, id);
                         player.getInventory().addItem(item);
                         Slimefun.getLocalization().sendMessage(sender, "commands.backpack.restored-backpack-given");

diff --git a/...n/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/DebugFishCommand.java b/...n/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/DebugFishCommand.java
@@ -20,7 +20,7 @@ class DebugFishCommand extends SubCommand {
     @Override
     public void onExecute(CommandSender sender, String[] args) {
         if (sender instanceof Player player && sender.hasPermission("slimefun.debugging")) {
-            player.getInventory().addItem(SlimefunItems.DEBUG_FISH.cloneItem());
+            player.getInventory().addItem(SlimefunItems.DEBUG_FISH.item());
         } else {
             Slimefun.getLocalization().sendMessage(sender, "messages.no-permission", true);
         }

diff --git a/...n/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/OreWasher.java b/...n/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/OreWasher.java
@@ -185,7 +185,7 @@ private void removeItem(Player p, Block b, Inventory inputInv, @Nullable Invento
      */
     public @Nonnull ItemStack getRandomDust() {
         int index = ThreadLocalRandom.current().nextInt(dusts.length);
-        return dusts[index].cloneItem();
+        return dusts[index].item();
     }
 
 }
diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/core/commands/TestChargeCommand.java b/src/test/java/io/github/thebusybiscuit/slimefun4/core/commands/TestChargeCommand.java
@@ -46,7 +46,7 @@ void testCommand() {
 
         Player player = server.addPlayer();
         player.setOp(true);
-        player.getInventory().setItemInMainHand(RECHARGEABLE_ITEM.cloneItem());
+        player.getInventory().setItemInMainHand(RECHARGEABLE_ITEM.item());
 
         ItemStack chargedItemStack = player.getInventory().getItemInMainHand();
         Rechargeable chargedItem = (Rechargeable) SlimefunItem.getByItem(chargedItemStack);

diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/TestSlimefunItem.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/TestSlimefunItem.java
@@ -12,7 +12,6 @@
 
 import io.github.bakedlibs.dough.items.CustomItemStack;
 import io.github.thebusybiscuit.slimefun4.api.exceptions.UnregisteredItemException;
-import io.github.thebusybiscuit.slimefun4.api.exceptions.WrongItemStackException;
 import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItem;
 import io.github.thebusybiscuit.slimefun4.api.items.SlimefunItemStack;
 import io.github.thebusybiscuit.slimefun4.api.recipes.RecipeType;
@@ -122,17 +121,18 @@ void testIsItem() {
         Assertions.assertEquals(sfItem, SlimefunItem.getByItem(new SlimefunItemStack(sfItem.getId(), item)));
     }
 
-    /* TODO: ItemStack Lock - Releated to the fact that we can't lock itemstacks directly
+
     @Test
-    @DisplayName("Test WrongItemStackException")
-    void testWrongItemStackException() {
+    @DisplayName("Test Defensive Item copy")
+    void testDefensiveItemCopy() {
         SlimefunItem item = TestUtilities.mockSlimefunItem(plugin, "WRONG_ITEMSTACK_EXCEPTION", CustomItemStack.create(Material.NETHER_STAR, "&4Do not modify me"));
         item.register(plugin);
         item.load();
 
         ItemStack itemStack = item.getItem();
-        Assertions.assertThrows(WrongItemStackException.class, () -> itemStack.setAmount(40));
-    }*/
+        itemStack.setAmount(40);
+        Assertions.assertNotEquals(itemStack, item.getItem());
+    }
 
     @Test
     @DisplayName("Test UnregisteredItemException")

diff --git a/...io/github/thebusybiscuit/slimefun4/implementation/items/autocrafters/TestAutoCrafter.java b/...io/github/thebusybiscuit/slimefun4/implementation/items/autocrafters/TestAutoCrafter.java
@@ -154,7 +154,7 @@ void ShapelessRecipeWithSlimefunItem() {
         SlimefunItem slimefunItem = TestUtilities.mockSlimefunItem(plugin, itemStack.getItemId(), itemStack.item());
         slimefunItem.register(plugin);
 
-        inv.addItem(itemStack.cloneItem());
+        inv.addItem(itemStack.item());
 
         // Test unusable SlimefunItem
         slimefunItem.setUseableInWorkbench(false);

diff --git a/...est/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBeeListener.java b/...est/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBeeListener.java
@@ -54,7 +54,7 @@ void testBeeDamage(boolean hasArmor) throws InterruptedException {
             MockBeeProtectionSuit armor = new MockBeeProtectionSuit(itemGroup, chestplate);
             armor.register(plugin);
 
-            player.getInventory().setChestplate(chestplate.cloneItem());
+            player.getInventory().setChestplate(chestplate.item());
             // Force update the cached armor
             profile.getArmor()[1].update(chestplate.item(), armor);
         }

diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TestArmorTask.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TestArmorTask.java
@@ -62,7 +62,7 @@ void testSlimefunArmor() throws InterruptedException {
         SlimefunArmorPiece armor = new SlimefunArmorPiece(TestUtilities.getItemGroup(plugin, "armor_test"), helmet, RecipeType.NULL, new ItemStack[9], effects);
         armor.register(plugin);
 
-        player.getInventory().setHelmet(helmet.cloneItem());
+        player.getInventory().setHelmet(helmet.item());
         player.getInventory().setChestplate(new ItemStack(Material.DIAMOND_CHESTPLATE));
         new ArmorTask(false).run();
 
@@ -84,15 +84,15 @@ void testRadiactivity(boolean hazmat, boolean radioactiveFire) throws Interrupte
         SlimefunItemStack item = new SlimefunItemStack("MOCK_URANIUM_" + String.valueOf(hazmat).toUpperCase(Locale.ROOT) + "_" + String.valueOf(radioactiveFire).toUpperCase(Locale.ROOT), Material.EMERALD, "&aHi, I am deadly");
         new RadioactiveItem(itemGroup, Radioactivity.VERY_DEADLY, item, RecipeType.NULL, new ItemStack[9]).register(plugin);
 
-        player.getInventory().setItemInMainHand(item.cloneItem());
+        player.getInventory().setItemInMainHand(item.item());
         player.getInventory().setItemInOffHand(new ItemStack(Material.EMERALD_ORE));
 
         if (hazmat) {
             SlimefunItemStack chestplate = new SlimefunItemStack("MOCK_HAZMAT_SUIT_" + String.valueOf(radioactiveFire).toUpperCase(Locale.ROOT), Material.LEATHER_CHESTPLATE, "&4Hazmat Prototype");
             MockHazmatSuit armor = new MockHazmatSuit(itemGroup, chestplate);
             armor.register(plugin);
 
-            player.getInventory().setChestplate(chestplate.cloneItem());
+            player.getInventory().setChestplate(chestplate.item());
         }
 
         ArmorTask task = new ArmorTask(radioactiveFire);