Make ItemPreUpdateEvent#newItem immutable

Previously, the newItem could be modified / replaced and changes would sometimes be reflected in the source. However, this was not consistent and could in other cases lead to e.g. items being deleted. This commit removes the functionality to modify the newItem.

Author: NichtStudioCode

invui/src/main/java/xyz/xenondevs/invui/inventory/event/ItemPreUpdateEvent.java

@@ -4,14 +4,12 @@
 import org.bukkit.inventory.ItemStack;
 import org.jspecify.annotations.Nullable;
 import xyz.xenondevs.invui.inventory.Inventory;
-import xyz.xenondevs.invui.util.ItemUtils;
 
 /**
  * An event that is called before a slot in an {@link Inventory} is updated.
  * <p>
  * Cancelling this event affects the source of the change, i.e. a {@link Player} would for example keep the item they
- * tried to place on their cursor. In certain situations, changing the amount of {@link #getNewItem()} will also be
- * reflected in the source.
+ * tried to place on their cursor.
  * <p>
  * Note that a fired {@link ItemPreUpdateEvent} does not necessitate that the given action will actually take place,
  * even if the event remains uncancelled. For example, moving an item from one inventory to another via shift-clicking
@@ -41,17 +39,6 @@ public ItemPreUpdateEvent(Inventory inventory, int slot, @Nullable UpdateReason
         super(inventory, slot, updateReason, previousItem, newItem);
     }
 
-    /**
-     * Change the {@link ItemStack} that will appear in the {@link Inventory}
-     * to a different one.
-     *
-     * @param newItem The {@link ItemStack} to appear in the {@link Inventory}
-     *                if the {@link ItemPreUpdateEvent} is not cancelled.
-     */
-    public void setNewItem(@Nullable ItemStack newItem) {
-        this.newItemStack = ItemUtils.takeUnlessEmpty(newItem);
-    }
-    
     /**
      * Gets the cancellation state of this event.
      *

invui/src/main/java/xyz/xenondevs/invui/inventory/event/ItemUpdateEvent.java

@@ -28,6 +28,8 @@ abstract class ItemUpdateEvent {
      */
     public ItemUpdateEvent(Inventory inventory, int slot, @Nullable UpdateReason updateReason,
                            @Nullable ItemStack previousItem, @Nullable ItemStack newItem) {
+        if (updateReason == UpdateReason.SUPPRESSED)
+            throw new IllegalArgumentException("UpdateReason.SUPPRESSED cannot be used for ItemUpdateEvents");
 
         this.inventory = inventory;
         this.slot = slot;
@@ -60,7 +62,7 @@ public Inventory getInventory() {
      * @return The {@link ItemStack}
      */
     public @Nullable ItemStack getPreviousItem() {
-        return previousItemStack;
+        return ItemUtils.cloneUnlessEmpty(previousItemStack);
     }
 
     /**
@@ -69,7 +71,7 @@ public Inventory getInventory() {
      * @return The new {@link ItemStack}
      */
     public @Nullable ItemStack getNewItem() {
-        return newItemStack;
+        return ItemUtils.cloneUnlessEmpty(newItemStack);
     }
 
     /**
@@ -128,7 +130,7 @@ public int getRemovedAmount() {
             throw new IllegalStateException("No items have been removed");
 
         assert previousItemStack != null;
-        if (newItemStack == null) 
+        if (newItemStack == null)
             return previousItemStack.getAmount();
         return previousItemStack.getAmount() - newItemStack.getAmount();
     }

invui/src/test/java/xyz/xenondevs/invui/inventory/CompositeInventoryTest.java

@@ -14,7 +14,6 @@
 
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicReference;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -185,22 +184,17 @@ public void testPreUpdateEventStateVisibleInCompositeHandler() {
         var composite = new CompositeInventory(inv1);
 
         AtomicBoolean doCancel = new AtomicBoolean();
-        AtomicReference<ItemStack> newItemStack = new AtomicReference<>(ItemStack.empty());
         inv1.addPreUpdateHandler(event -> {
             event.setCancelled(doCancel.get());
-            event.setNewItem(newItemStack.get());
         });
         composite.addPreUpdateHandler(event -> {
             assertEquals(event.isCancelled(), doCancel.get());
-            assertEquals(event.getNewItem(), newItemStack.get());
         });
 
         doCancel.set(false);
-        newItemStack.set(ItemStack.of(Material.DIRT));
         composite.setItem(null, 0, ItemStack.of(Material.DIAMOND));
 
         doCancel.set(true);
-        newItemStack.set(ItemStack.of(Material.STONE));
         composite.setItem(null, 0, ItemStack.of(Material.DIAMOND));
     }
 

invui/src/test/java/xyz/xenondevs/invui/inventory/ObscuredInventoryTest.java

@@ -14,7 +14,6 @@
 
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicReference;
 
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -173,22 +172,17 @@ public void testPreUpdateEventStateVisibleInObscuredHandler() {
         var obscured = new ObscuredInventory(backing, i -> i == 0);
 
         AtomicBoolean doCancel = new AtomicBoolean();
-        AtomicReference<ItemStack> newItemStack = new AtomicReference<>(ItemStack.empty());
         backing.addPreUpdateHandler(event -> {
             event.setCancelled(doCancel.get());
-            event.setNewItem(newItemStack.get());
         });
         obscured.addPreUpdateHandler(event -> {
             assertEquals(event.isCancelled(), doCancel.get());
-            assertEquals(event.getNewItem(), newItemStack.get());
         });
 
         doCancel.set(false);
-        newItemStack.set(ItemStack.of(Material.STONE));
         obscured.setItem(null, 0, ItemStack.of(Material.DIAMOND));
 
         doCancel.set(true);
-        newItemStack.set(ItemStack.of(Material.STONE));
         obscured.setItem(null, 0, ItemStack.of(Material.DIAMOND));
     }