From 3c1d8fd0d08c9fd25959bd9d7189892159c11f41 Mon Sep 17 00:00:00 2001 From: Intybyte Date: Sat, 16 Nov 2024 14:34:12 +0100 Subject: [PATCH] .item() and getItem() now creates a defensive copy --- .../slimefun4/api/items/SlimefunItem.java | 8 +------- .../slimefun4/api/items/SlimefunItemStack.java | 14 -------------- .../core/commands/subcommands/BackpackCommand.java | 2 +- .../commands/subcommands/DebugFishCommand.java | 2 +- .../items/multiblocks/OreWasher.java | 2 +- .../slimefun4/core/commands/TestChargeCommand.java | 2 +- .../implementation/items/TestSlimefunItem.java | 12 ++++++------ .../items/autocrafters/TestAutoCrafter.java | 2 +- .../implementation/listeners/TestBeeListener.java | 2 +- .../implementation/tasks/TestArmorTask.java | 6 +++--- 10 files changed, 16 insertions(+), 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 index 6be226f7a9..c9ce57cbe1 100644 --- 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 index e2532f2146..025067dcca 100644 --- 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 getSkullTexture() { return Optional.ofNullable(texture); } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java index 778d0f90ce..3e124b8e77 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/BackpackCommand.java +++ b/src/main/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/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/DebugFishCommand.java b/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/DebugFishCommand.java index 4153c63461..7b0c85b277 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/core/commands/subcommands/DebugFishCommand.java +++ b/src/main/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/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/OreWasher.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/OreWasher.java index 1bbe35c472..b8b7bcae6f 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/items/multiblocks/OreWasher.java +++ b/src/main/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 index de3c095e32..6a10937092 100644 --- 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 index 1eef53d488..421a4248f3 100644 --- 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/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/autocrafters/TestAutoCrafter.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/autocrafters/TestAutoCrafter.java index 73dbacfe11..0158b7cb86 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/items/autocrafters/TestAutoCrafter.java +++ b/src/test/java/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/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBeeListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBeeListener.java index b031eb7139..fbc563ea6f 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBeeListener.java +++ b/src/test/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 index d8dde298ce..2d53c865fa 100644 --- 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,7 +84,7 @@ 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) { @@ -92,7 +92,7 @@ void testRadiactivity(boolean hazmat, boolean radioactiveFire) throws Interrupte MockHazmatSuit armor = new MockHazmatSuit(itemGroup, chestplate); armor.register(plugin); - player.getInventory().setChestplate(chestplate.cloneItem()); + player.getInventory().setChestplate(chestplate.item()); } ArmorTask task = new ArmorTask(radioactiveFire);