Fix slot click handling to consider all layouts (#34)

* Improve click handling to check if a clicked slot is in both layouts present

* Add test if the clicked item passes through the static layout of the inventory

* Cleanup imports and improve variable name

---------

Co-authored-by: theEvilReaper <theEvilReaper@users.noreply.github.com>

Author: theEvilReaper

src/main/java/net/theevilreaper/aves/inventory/InventoryBuilder.java

@@ -62,19 +62,17 @@ protected InventoryBuilder(@NotNull InventoryType type) {
                 return;
             }
 
+            ISlot clickedSlot = null;
+
             if (this.dataLayout != null) {
-                var clickedSlot = this.dataLayout.getSlot(slot);
-                acceptClick(clickedSlot, player, clickType, slot, stack, result);
-                return;
+                clickedSlot = this.dataLayout.getSlot(slot);
             }
 
-            if (this.inventoryLayout != null) {
-                var clickedSlot = this.inventoryLayout.getSlot(slot);
-                acceptClick(clickedSlot, player, clickType, slot, stack, result);
-                return;
+            if ((clickedSlot == null || clickedSlot instanceof EmptySlot) && this.inventoryLayout != null) {
+                clickedSlot = this.inventoryLayout.getSlot(slot);
             }
 
-            result.accept(ClickHolder.noClick());
+            acceptClick(clickedSlot, player, clickType, slot, stack, result);
         };
     }
 

src/test/java/net/theevilreaper/aves/inventory/InventoryClickLogicIntegrationTest.java

@@ -16,18 +16,28 @@
 import net.minestom.testing.extension.MicrotusExtension;
 import net.theevilreaper.aves.inventory.click.ClickHolder;
 import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 @ExtendWith(MicrotusExtension.class)
 class InventoryClickLogicIntegrationTest {
 
+    private static ItemStack testItem;
+
+    @BeforeAll
+    static void setup() {
+        testItem = ItemStack.builder(Material.DIAMOND)
+                .customName(Component.text("Test Item"))
+                .build();
+    }
+
     @Test
     void testInventoryClickFlow(@NotNull Env env) {
         Instance instance = env.createEmptyInstance();
@@ -38,12 +48,8 @@ void testInventoryClickFlow(@NotNull Env env) {
         assertNotNull(builder);
 
         InventoryLayout layout = InventoryLayout.fromType(builder.getType());
-        ItemStack item = ItemStack.builder(Material.DIAMOND)
-                .customName(Component.text("Test Item"))
-                .build();
-
         AtomicBoolean clicked = new AtomicBoolean(false);
-        layout.setItem(0, item, (player, slot, click, stack, result) -> {
+        layout.setItem(0, testItem, (player, slot, click, stack, result) -> {
             result.accept(ClickHolder.cancelClick());
             clicked.set(true);
         });
@@ -57,7 +63,7 @@ void testInventoryClickFlow(@NotNull Env env) {
 
         Collector<WindowItemsPacket> windowsPacketCollector = testConnection.trackIncoming(WindowItemsPacket.class);
 
-        testLeftClick(testPlayer, 0, item);
+        testLeftClick(testPlayer, 0, testItem);
         // The return value should only be one packet, the one that updates the slots
         windowsPacketCollector.assertCount(2);
         assertTrue(clicked.get());
@@ -68,6 +74,50 @@ void testInventoryClickFlow(@NotNull Env env) {
     }
 
 
+    @Test
+    void testIfClickPassesInventoryLayout(@NotNull Env env) {
+        Instance instance = env.createEmptyInstance();
+        TestConnection testConnection = env.createConnection();
+        Player testPlayer = testConnection.connect(instance);
+        InventoryBuilder builder = new GlobalInventoryBuilder(Component.text("Test Inventory"), InventoryType.CHEST_3_ROW);
+
+        assertNotNull(builder);
+
+        InventoryLayout layout = InventoryLayout.fromType(builder.getType());
+
+        AtomicBoolean clicked = new AtomicBoolean(false);
+        layout.setItem(0, testItem, (player, slot, click, stack, result) -> {
+            result.accept(ClickHolder.cancelClick());
+            clicked.set(true);
+        });
+
+        builder.setLayout(layout);
+        builder.register();
+
+        testPlayer.openInventory(builder.getInventory());
+
+        assertNotNull(testPlayer.getOpenInventory());
+
+        Collector<WindowItemsPacket> windowsPacketCollector = testConnection.trackIncoming(WindowItemsPacket.class);
+
+        testLeftClick(testPlayer, 0, testItem);
+        // The return value should only be one packet, the one that updates the slots
+        windowsPacketCollector.assertCount(2);
+
+        WindowItemsPacket inventoryPacket = windowsPacketCollector.collect().getLast();
+        assertNotNull(inventoryPacket);
+
+        List<ItemStack> items = inventoryPacket.items();
+        assertNotNull(items, "Items in the inventory packet should not be null");
+        assertEquals(layout.getSize(), items.size(), "Items size should match layout size");
+
+        assertEquals(testItem, items.getFirst(), "First item should be the test item");
+
+        env.destroyInstance(instance, true);
+        assertTrue(instance.getPlayers().isEmpty(), "Instance should not have any players");
+    }
+
+
     private void testLeftClick(@NotNull Player player, int slot, @NotNull ItemStack stack) {
         _leftClick(player.getOpenInventory(), true, player, slot, stack);
     }