Fix GUI concurrent access and autocrafter deadlock

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,35 @@ public int addAny(ItemStack item, int amountToAdd) {
             int totalAdded = 0;
             if (this.cachedInventory != null && item != null) {
                 final int maxStack = item.getMaxStackSize();
+
+                // Count free slots to determine if we need to reserve one
+                int freeSlots = 0;
+                for (ItemStack stack : this.cachedInventory) {
+                    if (stack == null || stack.getAmount() == 0) {
+                        freeSlots++;
+                    }
+                }
+
+                // If autocrafter is active and 1 or fewer free slots, don't add ANYTHING
+                // Autocrafter needs at least 1 empty slot to craft
+                if (reserveOneSlot && freeSlots <= 1) {
+                    return 0;
+                }
+
+                // Calculate how many free slots we can actually use
+                // If reserving one slot, we can use (freeSlots - 1) new slots
+                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 +269,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());

EpicHoppers-Plugin/pom.xml

@@ -28,7 +28,7 @@
 
                             <shadedArtifactAttached>false</shadedArtifactAttached>
                             <useDependencyReducedPomInJar>true</useDependencyReducedPomInJar>
-                            <minimizeJar>true</minimizeJar>
+                            <minimizeJar>false</minimizeJar>
 
                             <relocations>
                                 <relocation>
@@ -123,6 +123,13 @@
             <scope>compile</scope>
         </dependency>
 
+        <dependency>
+            <groupId>com.songoda</groupId>
+            <artifactId>SongodaCore-NMS-v1_21_R5</artifactId>
+            <version>${songoda.coreVersion}</version>
+            <scope>compile</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.spigotmc</groupId>
             <artifactId>spigot-api</artifactId>

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() {

EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java

@@ -36,8 +36,32 @@ public class ModuleAutoCrafting extends Module {
     private static final Map<Hopper, ItemStack> CACHED_CRAFTING = new ConcurrentHashMap<>();
     private static final ItemStack NO_CRAFT = new ItemStack(Material.AIR);
 
+    // Cache for last output slot to optimize stacking (lag-free with timestamp)
+    // Key: Hopper, Value: {slot index, material type, timestamp}
+    private static final Map<Hopper, LastOutputSlot> LAST_OUTPUT_SLOT = new ConcurrentHashMap<>();
+    private static final long OUTPUT_SLOT_CACHE_TTL = 60000; // 60 seconds timeout
+
     private final boolean crafterEjection;
 
+    // Helper class to cache last output slot with timestamp
+    private static class LastOutputSlot {
+        final int slot;
+        final Material material;
+        final long timestamp;
+
+        LastOutputSlot(int slot, Material material) {
+            this.slot = slot;
+            this.material = material;
+            this.timestamp = System.currentTimeMillis();
+        }
+
+        boolean isValid(Material currentMaterial) {
+            // Cache is valid if material matches and not too old
+            return this.material == currentMaterial &&
+                   (System.currentTimeMillis() - this.timestamp) < OUTPUT_SLOT_CACHE_TTL;
+        }
+    }
+
     public ModuleAutoCrafting(SongodaPlugin plugin, GuiManager guiManager) {
         super(plugin, guiManager);
         this.crafterEjection = Settings.AUTOCRAFT_JAM_EJECT.getBoolean();
@@ -115,10 +139,11 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) {
                                                 recipe.result.isSimilar(item)));
 
                 // jam check: is this hopper gummed up?
-                if (this.crafterEjection && !freeSlotAfterRemovingIngredients) {
+                if (!freeSlotAfterRemovingIngredients) {
                     // Crafter can't function if there's nowhere to put the output
                     // ¯\_(ツ)_/¯
 
+                    // First try to find a slot that's NOT part of the ingredients
                     for (int i = 0; i < items.length; i++) {
                         if (!slotsToAlter.containsKey(i)) {
                             // and yeet into space!
@@ -130,33 +155,40 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) {
                         }
                     }
 
-                    // FIXME: In theory the code below should work. But if the last item is of the same type as the
-                    //        resulting item, the inventory won't update correctly
-                    //        (item is set correctly but reset to MaxStackSize)
-                    //        (CachedInventory doesn't like it's array to be edited?)
-                        /*
-                        // None of the slots can safely be freed. So we drop some leftover ingredients
-                        if (!freeSlotAfterRemovingIngredients) {
-
-                            int slot = items.length - 1;   // Last slot
-
-                            slotsToAlter.computeIfPresent(slot, (key, value) -> {
-                                items[slot].setAmount(value);
-
-                                return null;
-                            });
-
-                            // and yeet into space!
-                            items[slot].setAmount(slotsToAlter.getOrDefault(slot, items[slot].getAmount()));
-                            hopper.getWorld().dropItemNaturally(hopper.getLocation(), items[slot]);
-                            items[slot] = null;
-
-                            freeSlotAfterRemovingIngredients = true;
+                    // If all slots are ingredients, eject the last slot forcefully to free up space
+                    // This is necessary when the hopper is completely full of crafting ingredients
+                    if (!freeSlotAfterRemovingIngredients) {
+                        int slot = items.length - 1;   // Last slot
+
+                        // Drop only what won't be consumed by crafting
+                        Integer amountAfterCraft = slotsToAlter.get(slot);
+                        if (amountAfterCraft != null && amountAfterCraft > 0) {
+                            // Some items will remain after crafting, drop those
+                            ItemStack toDrop = items[slot].clone();
+                            toDrop.setAmount(amountAfterCraft);
+                            hopper.getLocation().getWorld().dropItemNaturally(hopper.getLocation(), toDrop);
+
+                            // Set the slot to exactly what will be consumed
+                            items[slot].setAmount(items[slot].getAmount() - amountAfterCraft);
+                            slotsToAlter.put(slot, 0);
+                        } else {
+                            // All items in this slot will be consumed, but we still drop one to make space
+                            ItemStack toDrop = items[slot].clone();
+                            toDrop.setAmount(1);
+                            hopper.getLocation().getWorld().dropItemNaturally(hopper.getLocation(), toDrop);
+                            items[slot].setAmount(items[slot].getAmount() - 1);
+
+                            if (items[slot].getAmount() == 0) {
+                                items[slot] = null;
+                            }
                         }
-                        */
+
+                        freeSlotAfterRemovingIngredients = true;
+                    }
                 }
 
                 if (freeSlotAfterRemovingIngredients) {
+                    // Remove ingredients
                     for (Map.Entry<Integer, Integer> entry : slotsToAlter.entrySet()) {
                         if (entry.getValue() <= 0) {
                             items[entry.getKey()] = null;
@@ -166,20 +198,55 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) {
                     }
 
                     // Add the resulting item into the inventory - Just making sure there actually is enough space
-                    for (int i = 0; i < items.length; i++) {
-                        if (items[i] == null ||
-                                (items[i].isSimilar(recipe.result)
-                                        && items[i].getAmount() + recipe.result.getAmount() <= items[i].getMaxStackSize())) {
-                            if (items[i] == null) {
-                                items[i] = recipe.result.clone();
-                            } else {
+                    boolean outputAdded = false;
+                    int outputSlot = -1;
+
+                    // OPTIMIZATION: Check cached slot first (lag-free with timestamp)
+                    LastOutputSlot cachedSlot = LAST_OUTPUT_SLOT.get(hopper);
+                    if (cachedSlot != null && cachedSlot.isValid(recipe.result.getType())) {
+                        int i = cachedSlot.slot;
+                        if (i < items.length && items[i] != null &&
+                            items[i].isSimilar(recipe.result) &&
+                            items[i].getAmount() + recipe.result.getAmount() <= items[i].getMaxStackSize()) {
+                            // Cached slot is still valid! Use it directly (no iteration needed)
+                            items[i].setAmount(items[i].getAmount() + recipe.result.getAmount());
+                            outputAdded = true;
+                            outputSlot = i;
+                        }
+                    }
+
+                    // If cached slot didn't work, do normal search
+                    if (!outputAdded) {
+                        // First pass: Look for existing stacks of the same item
+                        for (int i = 0; i < items.length; i++) {
+                            if (items[i] != null &&
+                                items[i].isSimilar(recipe.result) &&
+                                items[i].getAmount() + recipe.result.getAmount() <= items[i].getMaxStackSize()) {
                                 items[i].setAmount(items[i].getAmount() + recipe.result.getAmount());
+                                outputAdded = true;
+                                outputSlot = i;
+                                break;
                             }
+                        }
 
-                            break;
+                        // Second pass: Look for empty slots
+                        if (!outputAdded) {
+                            for (int i = 0; i < items.length; i++) {
+                                if (items[i] == null) {
+                                    items[i] = recipe.result.clone();
+                                    outputAdded = true;
+                                    outputSlot = i;
+                                    break;
+                                }
+                            }
                         }
                     }
 
+                    // Update cache with the slot we used
+                    if (outputAdded && outputSlot >= 0) {
+                        LAST_OUTPUT_SLOT.put(hopper, new LastOutputSlot(outputSlot, recipe.result.getType()));
+                    }
+
                     hopperCache.setContents(items);
                 }
             }