Merge pull request #286

Fix GUI Item Input Buttons deleting stacks on certain interactions

Author: WolfyScript

src/main/java/me/wolfyscript/customcrafting/gui/InteractionUtils.java

@@ -24,18 +24,23 @@
 
 import java.util.Objects;
 import java.util.Set;
+import java.util.WeakHashMap;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import me.wolfyscript.customcrafting.CustomCrafting;
 import me.wolfyscript.utilities.util.inventory.ItemUtils;
 import org.bukkit.Bukkit;
+import org.bukkit.event.inventory.InventoryAction;
 import org.bukkit.event.inventory.InventoryClickEvent;
 import org.bukkit.event.inventory.InventoryDragEvent;
+import org.bukkit.event.inventory.InventoryEvent;
 import org.bukkit.event.inventory.InventoryInteractEvent;
 import org.bukkit.inventory.ItemStack;
 
 public class InteractionUtils {
 
+    private static final WeakHashMap<InventoryEvent, Integer> CACHE_COLLECT_TO_CURSOR = new WeakHashMap<>();
+
     /**
      * Used to apply ItemStack changes from the InventoryInteractEvent.
      *
@@ -64,6 +69,14 @@ public static boolean applyItemFromInteractionEvent(int clickedSlot, InventoryIn
                         // Cancel the event when trying to shift-click the items from the bottom to the top inventory.
                         return true;
                     }
+                    /*
+                     * # issue: WolfyUtils calls all the ItemInputButtons on Shift-interaction,
+                     *      we need to only update the clicked slot when moving stack into bottom inventory,
+                     *      but still update all when stacks are moved into the GUI (top inventory), due to non-deterministic stack placement!
+                     */
+                    if (clickEvent.getAction().equals(InventoryAction.MOVE_TO_OTHER_INVENTORY) && Objects.equals(clickEvent.getClickedInventory(), event.getView().getTopInventory()) && clickedSlot != clickEvent.getSlot()) {
+                        return false; // The event should not be cancelled yet.
+                    }
                     applyItemStack.accept(current);
                 }
                 // Other than the PICKUP_ALL, DROP_ALL is not called when there is only 1 item.
@@ -91,8 +104,28 @@ public static boolean applyItemFromInteractionEvent(int clickedSlot, InventoryIn
                 }
                 // All these actions will keep remaining items in the slot, so lets just use the current stack.
                 // The stack will be updated, as it is a reference to the stack in the inventory.
-                case PICKUP_ONE, PICKUP_SOME, COLLECT_TO_CURSOR, DROP_ALL_CURSOR ->
+                case PICKUP_ONE, PICKUP_SOME, DROP_ALL_CURSOR -> applyItemStack.accept(clickEvent.getCurrentItem());
+                case COLLECT_TO_CURSOR -> {
+                    /*
+                     * # issue: We need to determine which slots are cleared completely and which slots
+                     *      are updated, because there are remaining items.
+                     *      So we need to keep track of the collected amount to figure that out, since Spigot has no API for that.
+                     *      This requires WolfyUtils v4.16.12 to properly function.
+                     */
+                    if (clickedSlot == clickEvent.getSlot()) {
+                        CACHE_COLLECT_TO_CURSOR.put(event, clickEvent.getCursor().getAmount());
                         applyItemStack.accept(clickEvent.getCurrentItem());
+                        break;
+                    }
+                    ItemStack item = clickEvent.getClickedInventory().getItem(clickedSlot);
+                    if (item == null) break;
+                    int total = CACHE_COLLECT_TO_CURSOR.compute(event, (inventoryEvent, count) -> count == null ? item.getAmount() : (count + item.getAmount()));
+                    if (total > clickEvent.getCursor().getMaxStackSize()) {
+                        applyItemStack.accept(clickEvent.getClickedInventory().getItem(clickedSlot));
+                        break;
+                    }
+                    applyItemStack.accept(null);
+                }
                 // Hotbar swaps work all the same, so lets take the hotbar stack.
                 case HOTBAR_SWAP, HOTBAR_MOVE_AND_READD ->
                         applyItemStack.accept(event.getWhoClicked().getInventory().getItem(clickEvent.getHotbarButton()));

src/main/java/me/wolfyscript/customcrafting/gui/elite_crafting/ButtonSlotResult.java

@@ -44,7 +44,7 @@ class ButtonSlotResult extends ItemInputButton<CCCache> {
         super("result_slot", new ButtonState<>("", Material.AIR, (cache, guiHandler, player, inventory, slot, event) -> {
             CacheEliteCraftingTable cacheEliteCraftingTable = cache.getEliteWorkbench();
             if (event instanceof InventoryClickEvent clickEvent && inventory.getWindow() instanceof CraftingWindow) {
-                if (!CraftingWindow.RESULT_SLOTS.contains(slot)) {
+                if (!CraftingWindow.RESULT_SLOTS.contains(slot) || slot != clickEvent.getSlot()) { // # issue: We don't want to cause a recipe consumption when the result button wasn't actually clicked!
                     return true;
                 }
                 if (clickEvent.getAction().equals(InventoryAction.MOVE_TO_OTHER_INVENTORY) && event.getView().getBottomInventory().equals(clickEvent.getClickedInventory())) {