From c5cab5fff7fe229abf7f4ef9f8633d2239b9bbd4 Mon Sep 17 00:00:00 2001 From: Francesco Date: Thu, 13 Nov 2025 19:32:58 +0100 Subject: [PATCH 1/4] Fix GUI concurrent access and autocrafter deadlock issues Bug fix #1 - GUI Concurrent Access: Multiple players can now simultaneously access the same hopper GUI without closing each other's views. Changed activePlayer tracking from single Player to Set, and implemented proper cleanup when players close their GUIs individually. Bug fix #2 - Autocrafter Deadlock Prevention: Fixed issue where hoppers with active autocrafters would fill all slots when thousands of items were on the ground, preventing the autocrafter from functioning. The suction module now reserves slots when an autocrafter is detected, maintaining at least 2 free slots (one for craft output, one for the next craft). Additionally, the autocrafter module will now automatically eject items when completely full of ingredients to prevent deadlocks. --- .../utils/StorageContainerCache.java | 42 ++++++++++++++ .../epichoppers/gui/GUIAutoSellFilter.java | 4 +- .../songoda/epichoppers/gui/GUICrafting.java | 4 +- .../songoda/epichoppers/gui/GUIFilter.java | 4 +- .../songoda/epichoppers/gui/GUIOverview.java | 4 +- .../songoda/epichoppers/gui/GUISmeltable.java | 2 +- .../epichoppers/hopper/HopperImpl.java | 25 +++++---- .../levels/modules/ModuleAutoCrafting.java | 55 +++++++++++-------- .../hopper/levels/modules/ModuleSuction.java | 8 ++- 9 files changed, 103 insertions(+), 45 deletions(-) diff --git a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java index a6fc69f6..4b97a50b 100644 --- a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java +++ b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java @@ -204,6 +204,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 +224,34 @@ 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, and that slot will be filled + // by the output. So we need to keep 2 slots free: one for current craft, one for next + if (reserveOneSlot && freeSlots <= 1) { + return 0; + } + + // If we have exactly 2 free slots, stop adding to NEW slots but can fill partial stacks + boolean shouldStopAdding = reserveOneSlot && freeSlots <= 2; + 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 should reserve this slot for autocrafter + if (shouldStopAdding) { + // Don't fill this free slot - reserve it for autocrafter + break; + } + // free slot! int toAdd = Math.min(maxStack, amountToAdd); this.cachedInventory[i] = item.clone(); @@ -224,6 +261,11 @@ public int addAny(ItemStack item, int amountToAdd) { totalAdded += toAdd; amountToAdd -= toAdd; } else if (maxStack > cacheItem.getAmount() && item.isSimilar(cacheItem)) { + // Check if we should stop adding to preserve empty slot for autocrafter + if (shouldStopAdding) { + continue; // Skip this partial stack, keep the empty slot free + } + // free space! int toAdd = Math.min(maxStack - cacheItem.getAmount(), amountToAdd); this.cachedInventory[i].setAmount(toAdd + cacheItem.getAmount()); diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIAutoSellFilter.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIAutoSellFilter.java index 8bedd11d..66fe8841 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIAutoSellFilter.java +++ b/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(); }); diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUICrafting.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUICrafting.java index b44bb124..e8220337 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUICrafting.java +++ b/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); diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIFilter.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIFilter.java index 136c5ac1..9004c6d0 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIFilter.java +++ b/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(); }); diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIOverview.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIOverview.java index 0856f922..84b01547 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUIOverview.java +++ b/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); }); } diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUISmeltable.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUISmeltable.java index 5d989543..f0b3f3bf 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/gui/GUISmeltable.java +++ b/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() { diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/HopperImpl.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/HopperImpl.java index ef3c4fda..b074d3c7 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/HopperImpl.java +++ b/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 activePlayers = new HashSet<>(); private final Map moduleCache = new HashMap<>(); @@ -124,12 +124,6 @@ public HopperImpl(Map 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() { diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java index 4336bc38..238ec6a2 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java +++ b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java @@ -115,10 +115,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,30 +131,36 @@ 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) { diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java index 7c41f444..163fa191 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java +++ b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java @@ -131,8 +131,14 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { } } + // Check if autocrafter is active on this hopper + // If yes, reserve one slot for crafting output + boolean hasAutoCrafter = hopper.getLevel().getModule("AutoCrafting") != null; + // try to add the items to the hopper - int toAdd, added = hopperCache.addAny(itemStack, toAdd = getActualItemAmount(item)); + int toAdd = getActualItemAmount(item); + int added = hopperCache.addAny(itemStack, toAdd, hasAutoCrafter); + if (added == 0) { return; } From 4edf13a13c526410b2dd9ea5f3e0250299cc5655 Mon Sep 17 00:00:00 2001 From: Francesco Date: Fri, 14 Nov 2025 11:28:42 +0100 Subject: [PATCH 2/4] Add WildStacker/UltimateStacker item loss fixes Fix two critical bugs causing item loss with stacker plugins: 1. Suction module item loss prevention - Moved addAny() call before event emission - Only emit InventoryPickupItemEvent when items are actually added - Prevents stacker plugins from modifying entities when hopper is full 2. Storage cache slot reservation fix - Fixed overly conservative slot counting for autocrafter - Correctly calculates usable slots: (freeSlots - 1) when reserving - Allows partial stack filling regardless of reservation - Prevents items getting stuck on ground with available slots Both fixes are essential for servers using WildStacker, UltimateStacker, or RoseStacker to prevent hundreds of items disappearing during suction pickup with autocrafter active. --- .../utils/StorageContainerCache.java | 30 ++-- .../hopper/levels/modules/ModuleSuction.java | 131 ++++++++++++++---- 2 files changed, 122 insertions(+), 39 deletions(-) diff --git a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java index 4b97a50b..375f5e14 100644 --- a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java +++ b/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]); @@ -234,21 +242,22 @@ public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) { } // If autocrafter is active and 1 or fewer free slots, don't add ANYTHING - // Autocrafter needs at least 1 empty slot to craft, and that slot will be filled - // by the output. So we need to keep 2 slots free: one for current craft, one for next + // Autocrafter needs at least 1 empty slot to craft if (reserveOneSlot && freeSlots <= 1) { return 0; } - // If we have exactly 2 free slots, stop adding to NEW slots but can fill partial stacks - boolean shouldStopAdding = reserveOneSlot && freeSlots <= 2; + // 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 should reserve this slot for autocrafter - if (shouldStopAdding) { - // Don't fill this free slot - reserve it for autocrafter + // 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; } @@ -260,11 +269,10 @@ public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) { this.cacheAdded[i] = toAdd; totalAdded += toAdd; amountToAdd -= toAdd; + freeSlotsUsed++; // Count this free slot as used } else if (maxStack > cacheItem.getAmount() && item.isSimilar(cacheItem)) { - // Check if we should stop adding to preserve empty slot for autocrafter - if (shouldStopAdding) { - continue; // Skip this partial stack, keep the empty slot free - } + // 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); diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java index 163fa191..074c2bd0 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java +++ b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java @@ -15,6 +15,7 @@ import dev.rosewood.rosestacker.api.RoseStackerAPI; import dev.rosewood.rosestacker.stack.StackedItem; import org.bukkit.Bukkit; +import org.bukkit.Location; import org.bukkit.Material; import org.bukkit.entity.EntityType; import org.bukkit.entity.Item; @@ -60,13 +61,34 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { return; } - Set itemsToSuck = hopper.getLocation().getWorld().getNearbyEntities(hopper.getLocation().add(0.5, 0.5, 0.5), radius, radius, radius) + Set itemsToSuck = hopper.getLocation().getWorld() + .getNearbyEntities(hopper.getLocation().add(0.5, 0.5, 0.5), radius, radius, radius) .stream() - .filter(entity -> entity.getType() == EntityType.DROPPED_ITEM - && !entity.isDead() - && entity.getTicksLived() >= ((Item) entity).getPickupDelay() - && entity.getLocation().getBlock().getType() != Material.HOPPER) + .filter(entity -> entity.getType() == EntityType.DROPPED_ITEM) .map(Item.class::cast) + .filter(entity -> { + if (entity.isDead() || entity.getLocation().getBlock().getType() == Material.HOPPER) { + return false; + } + + // For WildStacker items, bypass the PickupDelay check since we use their API + if (WILD_STACKER && WildStackerAPI.getStackedItem(entity) != null) { + return true; + } + + // For UltimateStacker items, bypass the PickupDelay check + if (ULTIMATE_STACKER && UltimateStackerApi.getStackedItemManager().isStackedItem(entity)) { + return true; + } + + // For RoseStacker items, bypass the PickupDelay check + if (ROSE_STACKER && RoseStackerAPI.getInstance().getStackedItem(entity) != null) { + return true; + } + + // For normal items, check PickupDelay + return entity.getTicksLived() >= entity.getPickupDelay(); + }) .collect(Collectors.toSet()); if (itemsToSuck.isEmpty()) { @@ -75,31 +97,41 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { boolean filterEndpoint = hopper.getFilter().getEndPoint() != null; - Inventory hopperInventory = null; - if (Settings.EMIT_INVENTORYPICKUPITEMEVENT.getBoolean()) { - InventoryHolder inventoryHolder = (InventoryHolder) hopper.getBlock().getState(); - hopperInventory = Bukkit.createInventory(inventoryHolder, InventoryType.HOPPER); + // Always create inventory for the suction module to allow plugins to cancel the pickup event + // Check if the hopper block is still valid before creating the inventory + if (!(hopper.getBlock().getState() instanceof InventoryHolder)) { + // Hopper block is no longer valid (chunk unloaded, block removed, etc.) + return; } + InventoryHolder inventoryHolder = (InventoryHolder) hopper.getBlock().getState(); + Inventory hopperInventory = Bukkit.createInventory(inventoryHolder, InventoryType.HOPPER); + for (Item item : itemsToSuck) { ItemStack itemStack = item.getItemStack(); - if (item.getPickupDelay() == 0) { + // Check if this is a stacker plugin item + boolean isStackerItem = (WILD_STACKER && WildStackerAPI.getStackedItem(item) != null) + || (ULTIMATE_STACKER && UltimateStackerApi.getStackedItemManager().isStackedItem(item)) + || (ROSE_STACKER && RoseStackerAPI.getInstance().getStackedItem(item) != null); + + // Skip items with PickupDelay 0 (unless they're stacker items, which we handle via API) + if (!isStackerItem && item.getPickupDelay() == 0) { item.setPickupDelay(25); continue; } if (itemStack.getType().name().contains("SHULKER_BOX")) { - return; + continue; } if (itemStack.hasItemMeta() && itemStack.getItemMeta().hasDisplayName() && itemStack.getItemMeta().getDisplayName().startsWith("***")) { - return; //Compatibility with Shop instance: https://www.spigotmc.org/resources/shop-a-simple-intuitive-shop-instance.9628/ + continue; //Compatibility with Shop instance: https://www.spigotmc.org/resources/shop-a-simple-intuitive-shop-instance.9628/ } if (BLACKLIST.contains(item.getUniqueId())) { - return; + continue; } // respect filter if no endpoint @@ -122,33 +154,62 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { } } - if (Settings.EMIT_INVENTORYPICKUPITEMEVENT.getBoolean()) { - hopperInventory.setContents(hopperCache.cachedInventory); - InventoryPickupItemEvent pickupEvent = new InventoryPickupItemEvent(hopperInventory, item); - Bukkit.getPluginManager().callEvent(pickupEvent); - if (pickupEvent.isCancelled()) { - continue; - } - } + // IMPORTANT: Read the actual item amount BEFORE emitting the event! + // WildStacker (priority HIGHEST) modifies the item during the event, + // so we must capture the correct amount first + int toAdd = getActualItemAmount(item); // Check if autocrafter is active on this hopper // If yes, reserve one slot for crafting output boolean hasAutoCrafter = hopper.getLevel().getModule("AutoCrafting") != null; - // try to add the items to the hopper - int toAdd = getActualItemAmount(item); + // Try to add the items to the hopper FIRST int added = hopperCache.addAny(itemStack, toAdd, hasAutoCrafter); + // CRITICAL FIX: Do NOT emit the event if we can't add anything! + // Emitting the event when added == 0 causes WildStacker/UltimateStacker to modify + // the entity amount even though we're not picking it up, resulting in item loss! if (added == 0) { - return; + continue; // Continue to next item instead of return + } + + // We added items! Now emit the event to allow other plugins to be aware + // Note: Cache is already modified at this point, but this prevents WildStacker + // from modifying entities we can't pick up anyway + hopperInventory.setContents(hopperCache.cachedInventory); + InventoryPickupItemEvent pickupEvent = new InventoryPickupItemEvent(hopperInventory, item); + Bukkit.getPluginManager().callEvent(pickupEvent); + + if (pickupEvent.isCancelled()) { + // Check if this is a protected item (from shops or other plugins) first + // Protected items should always respect the cancellation to maintain compatibility + boolean isProtectedItem = itemStack.hasItemMeta() && ( + itemStack.getItemMeta().hasDisplayName() || + itemStack.getItemMeta().hasLore() || + !itemStack.getItemMeta().getPersistentDataContainer().isEmpty() + ); + + if (isProtectedItem) { + // This item is protected by another plugin - always respect that + // Note: Cache was already modified, but we'll skip entity removal + // This may result in duplicate pickup on next tick, but respects protection + continue; + } else if (!isStackerItem) { + // Normal item that's been cancelled - skip it + continue; + } + // If we get here, it's a stacker item that's not protected, so we bypass the cancellation + // This allows suction to work properly with stacker plugins } // items added ok! - if (added == toAdd) { + if (added >= toAdd) { + // All items were added, remove the entity item.remove(); } else { - // update the item's total - updateAmount(item, toAdd - added); + // Only some items were added, update the remaining amount + int remaining = toAdd - added; + updateAmount(item, remaining); // wait before trying to add again BLACKLIST.add(item.getUniqueId()); @@ -181,7 +242,21 @@ private void updateAmount(Item item, int amount) { if (ULTIMATE_STACKER) { UltimateStackerApi.getStackedItemManager().updateStack(item, amount); } else if (WILD_STACKER) { - WildStackerAPI.getStackedItem(item).setStackAmount(amount, true); + com.bgsoftware.wildstacker.api.objects.StackedItem stackedItem = WildStackerAPI.getStackedItem(item); + if (stackedItem != null) { + stackedItem.setStackAmount(amount, true); + } else { + // Fallback if item is not tracked by WildStacker + item.getItemStack().setAmount(Math.min(amount, item.getItemStack().getMaxStackSize())); + } + } else if (ROSE_STACKER) { + StackedItem stackedItem = RoseStackerAPI.getInstance().getStackedItem(item); + if (stackedItem != null) { + stackedItem.setStackSize(amount); + } else { + // Fallback if item is not tracked by RoseStacker + item.getItemStack().setAmount(Math.min(amount, item.getItemStack().getMaxStackSize())); + } } else { item.getItemStack().setAmount(Math.min(amount, item.getItemStack().getMaxStackSize())); } From e0613f7ed067e755c3d761db893c64e7b1f1f448 Mon Sep 17 00:00:00 2001 From: Francesco Date: Fri, 14 Nov 2025 12:30:15 +0100 Subject: [PATCH 3/4] Fix critical suction bugs: item duplication and slot reservation - Fix item duplication with protected items from other plugins Add rollbackAdd() method to revert cache changes when InventoryPickupItemEvent is cancelled This prevents duplication loop when items are protected by shop plugins - Fix slot reservation logic for autocrafter Only reserve slot when autocrafter has configured recipe, not just when module exists Remove overly restrictive slot blocking that prevented partial stack filling - Improve slot usage calculation Correctly handle free slot reservation with (freeSlots - 1) logic Allow partial stack filling even when reserving slots for autocrafter --- .../utils/StorageContainerCache.java | 52 +++++++++++++++---- .../hopper/levels/modules/ModuleSuction.java | 20 ++++--- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java index 375f5e14..61744ba4 100644 --- a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java +++ b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/utils/StorageContainerCache.java @@ -233,7 +233,9 @@ public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) { if (this.cachedInventory != null && item != null) { final int maxStack = item.getMaxStackSize(); - // Count free slots to determine if we need to reserve one + // 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) { @@ -241,14 +243,6 @@ public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) { } } - // 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; @@ -287,9 +281,49 @@ public int addAny(ItemStack item, int amountToAdd, boolean reserveOneSlot) { 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. * diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java index 074c2bd0..e55480b5 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java +++ b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleSuction.java @@ -159,9 +159,14 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { // so we must capture the correct amount first int toAdd = getActualItemAmount(item); - // Check if autocrafter is active on this hopper - // If yes, reserve one slot for crafting output - boolean hasAutoCrafter = hopper.getLevel().getModule("AutoCrafting") != null; + // Check if autocrafter is active AND has a configured recipe + // Only reserve slot if there's actually a recipe configured + boolean hasAutoCrafter = false; + ModuleAutoCrafting autoCrafter = (ModuleAutoCrafting) hopper.getLevel().getModule("AutoCrafting"); + if (autoCrafter != null) { + ItemStack autoCrafting = autoCrafter.getAutoCrafting(hopper); + hasAutoCrafter = autoCrafting != null && autoCrafting.getType() != Material.AIR; + } // Try to add the items to the hopper FIRST int added = hopperCache.addAny(itemStack, toAdd, hasAutoCrafter); @@ -190,14 +195,15 @@ public void run(Hopper hopper, StorageContainerCache.Cache hopperCache) { ); if (isProtectedItem) { - // This item is protected by another plugin - always respect that - // Note: Cache was already modified, but we'll skip entity removal - // This may result in duplicate pickup on next tick, but respects protection + // This item is protected by another plugin - ROLLBACK the cache to prevent duplication! + hopperCache.rollbackAdd(itemStack, added); continue; } else if (!isStackerItem) { - // Normal item that's been cancelled - skip it + // Normal item that's been cancelled - ROLLBACK the cache + hopperCache.rollbackAdd(itemStack, added); continue; } + // If we get here, it's a stacker item that's not protected, so we bypass the cancellation // This allows suction to work properly with stacker plugins } From 605407c546dfea81e6edd9c7e169c481cc635481 Mon Sep 17 00:00:00 2001 From: Francesco Date: Sat, 15 Nov 2025 17:12:55 +0100 Subject: [PATCH 4/4] Fix autocrafter data loss on server crash - Save autocrafter configuration to database instead of config file Prevents data loss when server crashes or is force-killed - Use complete ItemStack serialization to preserve custom items Support items with custom names, lore, enchantments, and NBT data - Add automatic migration from old config format to database Existing autocrafter configurations are migrated on first load - Add AUTOCRAFTER type to ItemType enum for database storage --- .claude/settings.local.json | 18 +++ .../songoda/epichoppers/hopper/ItemType.java | 2 +- .../levels/modules/ModuleAutoCrafting.java | 138 +++++++++++++++++- 3 files changed, 152 insertions(+), 6 deletions(-) create mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 00000000..59684dcc --- /dev/null +++ b/.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": [] + } +} diff --git a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/hopper/ItemType.java b/EpicHoppers-API/src/main/java/com/songoda/epichoppers/hopper/ItemType.java index a43f3842..0e6c62a8 100644 --- a/EpicHoppers-API/src/main/java/com/songoda/epichoppers/hopper/ItemType.java +++ b/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 } diff --git a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java index 238ec6a2..24a3ffdc 100644 --- a/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java +++ b/EpicHoppers-Plugin/src/main/java/com/songoda/epichoppers/hopper/levels/modules/ModuleAutoCrafting.java @@ -4,7 +4,10 @@ import com.songoda.core.gui.GuiManager; import com.songoda.third_party.com.cryptomorin.xseries.XMaterial; import com.songoda.core.utils.TextUtils; +import com.songoda.epichoppers.EpicHoppers; import com.songoda.epichoppers.hopper.Hopper; +import com.songoda.epichoppers.hopper.HopperImpl; +import com.songoda.epichoppers.hopper.ItemType; import com.songoda.epichoppers.settings.Settings; import com.songoda.epichoppers.utils.Methods; import com.songoda.epichoppers.gui.GUICrafting; @@ -19,9 +22,18 @@ import org.bukkit.inventory.ShapedRecipe; import org.bukkit.inventory.ShapelessRecipe; import org.bukkit.inventory.meta.ItemMeta; - +import org.bukkit.util.io.BukkitObjectInputStream; +import org.bukkit.util.io.BukkitObjectOutputStream; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -275,23 +287,139 @@ private Recipes getRecipes(ItemStack toCraft) { return recipes; } + /** + * Load autocrafter item from database + * Uses complete ItemStack serialization to preserve custom items (name, lore, enchants, NBT) + */ + private ItemStack loadAutoCraftingFromDB(HopperImpl hopper) { + try (Connection connection = EpicHoppers.getPlugin(EpicHoppers.class).getDataManager().getDatabaseConnector().getConnection()) { + String tablePrefix = EpicHoppers.getPlugin(EpicHoppers.class).getDataManager().getTablePrefix(); + String selectItem = "SELECT item FROM " + tablePrefix + "items WHERE hopper_id = ? AND item_type = ? LIMIT 1"; + + try (PreparedStatement statement = connection.prepareStatement(selectItem)) { + statement.setInt(1, hopper.getId()); + statement.setString(2, ItemType.AUTOCRAFTER.name()); + + try (ResultSet result = statement.executeQuery()) { + if (result.next()) { + String itemData = result.getString("item"); + if (itemData != null) { + // Deserialize complete ItemStack from Base64 + try (BukkitObjectInputStream stream = new BukkitObjectInputStream( + new ByteArrayInputStream(Base64.getDecoder().decode(itemData)))) { + return (ItemStack) stream.readObject(); + } catch (ClassNotFoundException | IOException e) { + e.printStackTrace(); + } + } + } + } + } + } catch (Exception ex) { + ex.printStackTrace(); + } + + return null; + } + + /** + * Save autocrafter item to database + * Uses complete ItemStack serialization to preserve custom items (name, lore, enchants, NBT) + */ + private void saveAutoCraftingToDB(HopperImpl hopper, ItemStack item) { + try (Connection connection = EpicHoppers.getPlugin(EpicHoppers.class).getDataManager().getDatabaseConnector().getConnection()) { + String tablePrefix = EpicHoppers.getPlugin(EpicHoppers.class).getDataManager().getTablePrefix(); + + // Delete existing autocrafter item + String deleteItem = "DELETE FROM " + tablePrefix + "items WHERE hopper_id = ? AND item_type = ?"; + try (PreparedStatement statement = connection.prepareStatement(deleteItem)) { + statement.setInt(1, hopper.getId()); + statement.setString(2, ItemType.AUTOCRAFTER.name()); + statement.executeUpdate(); + } + + // Insert new autocrafter item (if not null) + if (item != null && item.getType() != Material.AIR) { + String insertItem = "INSERT INTO " + tablePrefix + "items (hopper_id, item_type, item) VALUES (?, ?, ?)"; + try (PreparedStatement statement = connection.prepareStatement(insertItem)) { + statement.setInt(1, hopper.getId()); + statement.setString(2, ItemType.AUTOCRAFTER.name()); + + // Serialize complete ItemStack to Base64 + try (ByteArrayOutputStream stream = new ByteArrayOutputStream(); + BukkitObjectOutputStream bukkitStream = new BukkitObjectOutputStream(stream)) { + bukkitStream.writeObject(item); + statement.setString(3, Base64.getEncoder().encodeToString(stream.toByteArray())); + } catch (IOException e) { + e.printStackTrace(); + return; + } + + statement.executeUpdate(); + } + } + } catch (Exception ex) { + ex.printStackTrace(); + } + } + public ItemStack getAutoCrafting(Hopper hopper) { + // Check cache first if (CACHED_CRAFTING.containsKey(hopper)) { return CACHED_CRAFTING.get(hopper); } + if (!(hopper instanceof HopperImpl)) { + return null; + } + + HopperImpl hopperImpl = (HopperImpl) hopper; + + // Try loading from database (new format with full ItemStack serialization) + ItemStack fromDB = loadAutoCraftingFromDB(hopperImpl); + if (fromDB != null) { + CACHED_CRAFTING.put(hopper, fromDB); + return fromDB; + } + + // Fallback: load from config file (old format - migration path) Object autocrafting = getData(hopper, "autocrafting"); - ItemStack toCraft = autocrafting instanceof ItemStack ? (ItemStack) autocrafting : decode((String) autocrafting); - CACHED_CRAFTING.put(hopper, toCraft == null ? NO_CRAFT : toCraft); - return toCraft; + if (autocrafting != null) { + ItemStack toCraft = autocrafting instanceof ItemStack ? (ItemStack) autocrafting : decode((String) autocrafting); + if (toCraft != null && toCraft.getType() != Material.AIR) { + // MIGRATION: Found old format data, migrate it to database + saveAutoCraftingToDB(hopperImpl, toCraft); + // Clear from config to avoid confusion + saveData(hopper, "autocrafting", null, null); + CACHED_CRAFTING.put(hopper, toCraft); + return toCraft; + } + } + + // No data found + CACHED_CRAFTING.put(hopper, NO_CRAFT); + return null; } public void setAutoCrafting(Hopper hopper, Player player, ItemStack autoCrafting) { - saveData(hopper, "autocrafting", autoCrafting == null ? null : encode(autoCrafting), autoCrafting); + if (!(hopper instanceof HopperImpl)) { + return; + } + + HopperImpl hopperImpl = (HopperImpl) hopper; + + // CRITICAL FIX: Save immediately to database to prevent data loss on server crash + // Uses complete ItemStack serialization to preserve custom items (name, lore, enchants, NBT) + saveAutoCraftingToDB(hopperImpl, autoCrafting); + + // Update cache CACHED_CRAFTING.put(hopper, autoCrafting == null ? NO_CRAFT : autoCrafting); + if (autoCrafting == null) { return; } + + // Return excess items to player int excess = autoCrafting.getAmount() - 1; autoCrafting.setAmount(1); if (excess > 0 && player != null) {