Fix GUI concurrent access and autocrafter deadlock

.claude/settings.local.json

@@ -0,0 +1,18 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(gh pr reopen:*)",
+      "Bash(git diff:*)",
+      "Bash(gh api:*)",
+      "Bash(git add:*)",
+      "Bash(git commit:*)",
+      "Bash(git push:*)",
+      "Bash(gh pr view:*)",
+      "Bash(git checkout:*)",
+      "Bash(git cherry-pick:*)",
+      "Bash(mvn clean package:*)"
+    ],
+    "deny": [],
+    "ask": []
+  }
+}

EpicHoppers-API/src/main/java/com/songoda/epichoppers/hopper/ItemType.java

@@ -1,5 +1,5 @@
 package com.songoda.epichoppers.hopper;
 
 public enum ItemType {
-    WHITELIST, BLACKLIST, VOID, AUTO_SELL_WHITELIST, AUTO_SELL_BLACKLIST
+    WHITELIST, BLACKLIST, VOID, AUTO_SELL_WHITELIST, AUTO_SELL_BLACKLIST, AUTOCRAFTER
 }

EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java

@@ -101,7 +101,15 @@ public static void update() {
                 .forEach(e -> {
                     final ItemStack[] cachedInventory = e.getValue().cachedInventory;
                     final boolean[] cacheChanged = e.getValue().cacheChanged;
+
+                    // Check if the block is still a valid InventoryHolder before casting
+                    if (!(e.getKey().getState() instanceof InventoryHolder)) {
+                        // Block is no longer an inventory holder (chunk unloaded, block removed, etc.)
+                        return;
+                    }
+
                     Inventory inventory = ((InventoryHolder) e.getKey().getState()).getInventory();
+
                     for (int i = 0; i < cachedInventory.length; i++) {
                         if (cacheChanged[i]) {
                             inventory.setItem(i, cachedInventory[i]);
@@ -204,6 +212,18 @@ public void removeItems(ItemStack item) {
          * @return how many items were added
          */
         public int addAny(ItemStack item, int amountToAdd) {
+            return addAny(item, amountToAdd, false);
+        }
+
+        /**
+         * Add a number of items to this container's inventory later.
+         *
+         * @param item          item to add
+         * @param amountToAdd   how many of this item to attempt to add
+         * @param reserveOneSlot if true, always keep at least one slot empty (for autocrafter)
+         * @return how many items were added
+         */
+        public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) {
             // Don't transfer shulker boxes into other shulker boxes, that's a bad idea.
             if (this.type.name().contains("SHULKER_BOX") && item.getType().name().contains("SHULKER_BOX")) {
                 return 0;
@@ -212,9 +232,29 @@ public int addAny(ItemStack item, int amountToAdd) {
             int totalAdded = 0;
             if (this.cachedInventory != null && item != null) {
                 final int maxStack = item.getMaxStackSize();
+
+                // Calculate how many free slots we can actually use
+                // If reserving one slot, we can use (freeSlots - 1) new slots
+                // The loop below will handle partial stacks separately (they don't consume free slots)
+                int freeSlots = 0;
+                for (ItemStack stack : this.cachedInventory) {
+                    if (stack == null || stack.getAmount() == 0) {
+                        freeSlots++;
+                    }
+                }
+
+                int usableFreeSlots = reserveOneSlot ? (freeSlots - 1) : freeSlots;
+                int freeSlotsUsed = 0;
+
                 for (int i = 0; amountToAdd > 0 && i < this.cachedInventory.length; i++) {
                     final ItemStack cacheItem = this.cachedInventory[i];
                     if (cacheItem == null || cacheItem.getAmount() == 0) {
+                        // Check if we've already used all available free slots
+                        if (reserveOneSlot && freeSlotsUsed >= usableFreeSlots) {
+                            // We've used all available free slots, reserve the rest for autocrafter
+                            break;
+                        }
+
                         // free slot!
                         int toAdd = Math.min(maxStack, amountToAdd);
                         this.cachedInventory[i] = item.clone();
@@ -223,7 +263,11 @@ public int addAny(ItemStack item, int amountToAdd) {
                         this.cacheAdded[i] = toAdd;
                         totalAdded += toAdd;
                         amountToAdd -= toAdd;
+                        freeSlotsUsed++;  // Count this free slot as used
                     } else if (maxStack > cacheItem.getAmount() && item.isSimilar(cacheItem)) {
+                        // Filling partial stacks does NOT consume free slots!
+                        // So we can ALWAYS do this, even when reserving slots for autocrafter
+
                         // free space!
                         int toAdd = Math.min(maxStack - cacheItem.getAmount(), amountToAdd);
                         this.cachedInventory[i].setAmount(toAdd + cacheItem.getAmount());
@@ -237,9 +281,49 @@ public int addAny(ItemStack item, int amountToAdd) {
                     this.dirty = true;
                 }
             }
+
             return totalAdded;
         }
 
+        /**
+         * Rollback items that were just added via addAny().
+         * This is used when an event is cancelled and we need to undo the cache changes.
+         *
+         * @param item        item to remove
+         * @param amountToRemove how many of this item to remove (should match what addAny() returned)
+         */
+        public void rollbackAdd(ItemStack item, int amountToRemove) {
+            if (this.cachedInventory == null || item == null || amountToRemove <= 0) {
+                return;
+            }
+
+            int remaining = amountToRemove;
+
+            // Reverse order: undo what we added last
+            for (int i = this.cachedInventory.length - 1; remaining > 0 && i >= 0; i--) {
+                // Only rollback slots that we actually added to in the last addAny() call
+                if (this.cacheAdded[i] > 0) {
+                    final ItemStack cacheItem = this.cachedInventory[i];
+                    if (cacheItem != null && item.isSimilar(cacheItem)) {
+                        int toRemove = Math.min(this.cacheAdded[i], remaining);
+                        int newAmount = cacheItem.getAmount() - toRemove;
+
+                        if (newAmount <= 0) {
+                            // Remove the entire stack
+                            this.cachedInventory[i] = null;
+                        } else {
+                            // Just reduce the amount
+                            this.cachedInventory[i].setAmount(newAmount);
+                        }
+
+                        this.cacheChanged[i] = true;
+                        this.cacheAdded[i] -= toRemove;
+                        remaining -= toRemove;
+                    }
+                }
+            }
+        }
+
         /**
          * Add an item to this container's inventory later.
          *

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIAutoSellFilter.java

@@ -40,9 +40,9 @@ public GUIAutoSellFilter(SongodaPlugin plugin, Hopper hopper) {
 
         setOnOpen((event) -> GUIAutoSellFilter.OPEN_INVENTORIES.add(this));
 
-        setOnClose((event) -> {
+        setOnClose(event -> {
             GUIAutoSellFilter.OPEN_INVENTORIES.remove(this);
-            hopper.setActivePlayer(null);
+            ((HopperImpl) hopper).removeActivePlayer(event.player);
             compile();
         });
 

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUICrafting.java

@@ -18,8 +18,8 @@ public GUICrafting(ModuleAutoCrafting module, SongodaPlugin plugin, Hopper hoppe
         super(plugin, "crafting");
         setRows(3);
         setTitle(Methods.formatName(hopper.getLevel().getLevel()) + TextUtils.formatText(" &8-&f Crafting"));
-        setOnClose((event) -> {
-            hopper.setActivePlayer(null);
+        setOnClose(event -> {
+            ((HopperImpl) hopper).removeActivePlayer(event.player);
             setItem(module, hopper, player);
         });
         setAcceptsItems(true);

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIFilter.java

@@ -44,9 +44,9 @@ public GUIFilter(SongodaPlugin plugin, Hopper hopper, Player player) {
 
         setOnOpen((event) -> GUIFilter.OPEN_INVENTORIES.add(this));
 
-        setOnClose((event) -> {
+        setOnClose(event -> {
             GUIFilter.OPEN_INVENTORIES.remove(this);
-            hopper.setActivePlayer(null);
+            ((HopperImpl) hopper).removeActivePlayer(event.player);
             compile();
         });
 

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIOverview.java

@@ -51,8 +51,8 @@ public GUIOverview(SongodaPlugin plugin, Hopper hopper, Player player) {
         setTitle(Methods.formatName(hopper.getLevel().getLevel()));
         runTask();
         constructGUI();
-        this.setOnClose(action -> {
-            hopper.setActivePlayer(null);
+        this.setOnClose(event -> {
+            ((HopperImpl) hopper).removeActivePlayer(event.player);
             Bukkit.getScheduler().cancelTask(this.task);
         });
     }

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUISmeltable.java

@@ -45,7 +45,7 @@ public GUISmeltable(ModuleAutoSmelter moduleAutoSmelter, SongodaPlugin plugin, H
         this.setOnPage((event) -> showPage());
         showPage();
 
-        this.setOnClose((event) -> hopper.setActivePlayer(null));
+        this.setOnClose(event -> ((HopperImpl) hopper).removeActivePlayer(event.player));
     }
 
     void showPage() {

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/HopperImpl.java

@@ -53,7 +53,7 @@ public class HopperImpl implements Hopper {
 
     private int syncId = -1;
 
-    private Player activePlayer;
+    private final Set<Player> activePlayers = new HashSet<>();
 
     private final Map<String, Object> moduleCache = new HashMap<>();
 
@@ -124,12 +124,6 @@ public HopperImpl(Map<String, Object> map) {
 
     @ApiStatus.Internal
     public boolean prepareForOpeningOverviewGui(Player player) {
-        if (this.lastPlayerOpened != null &&
-                this.lastPlayerOpened != player.getUniqueId() &&
-                Bukkit.getPlayer(this.lastPlayerOpened) != null) {
-            Bukkit.getPlayer(this.lastPlayerOpened).closeInventory();
-        }
-
         HopperAccessEvent accessEvent = new HopperAccessEvent(player, this);
         Bukkit.getPluginManager().callEvent(accessEvent);
         if (accessEvent.isCancelled()) {
@@ -153,9 +147,10 @@ public boolean prepareForOpeningOverviewGui(Player player) {
 
     @ApiStatus.Internal
     public void forceClose() {
-        if (this.activePlayer != null) {
-            this.activePlayer.closeInventory();
+        for (Player player : this.activePlayers) {
+            player.closeInventory();
         }
+        this.activePlayers.clear();
     }
 
     public void dropItems() {
@@ -460,11 +455,19 @@ public void setId(int id) {
     }
 
     public Player getActivePlayer() {
-        return this.activePlayer;
+        return this.activePlayers.isEmpty() ? null : this.activePlayers.iterator().next();
     }
 
     public void setActivePlayer(Player activePlayer) {
-        this.activePlayer = activePlayer;
+        if (activePlayer == null) {
+            this.activePlayers.clear();
+        } else {
+            this.activePlayers.add(activePlayer);
+        }
+    }
+
+    public void removeActivePlayer(Player player) {
+        this.activePlayers.remove(player);
     }
 
     private LevelManager getLevelManager() {