CODEWITHSUNDEEP
javabukkitspigot
DerArkeN
DerArkeN

Reputation: 45

InventoryClickEvent gets fired multiple times

I have an abstract GUI parent class handling a click event.

public abstract class GUI implements Listener {

    private final Inventory inventory;

    public GUI(Player player) {
        Bukkit.getPluginManager().registerEvents(this, NPCs.getPlugin());

        ItemStack fillerItem = new ItemStack(getFiller());
        ItemMeta fillerItemMeta = fillerItem.getItemMeta();
        fillerItemMeta.setDisplayName("");
        fillerItem.setItemMeta(fillerItemMeta);

        int inventorySize = (getFunctionalItems().size()>=54) ? 54 : getFunctionalItems().size()+(9-getFunctionalItems().size()%9)*Math.min(1, getFunctionalItems().size()%9);

        inventory = Bukkit.createInventory(player, inventorySize, getName());

        for(int i = 0; i < inventory.getSize(); i++) {
            inventory.setItem(i, fillerItem);
        }
        for(int i = 0; i < getFunctionalItems().size(); i++) {
            inventory.setItem(i, getFunctionalItems().get(i));
        }
    }

    @EventHandler
    public void onClick(InventoryClickEvent event) {
        handle(event);
    }

    public abstract ArrayList<ItemStack> getFunctionalItems();

    public abstract String getName();

    protected abstract void handle(InventoryClickEvent event);

    public Material getFiller() {
        return Material.GRAY_STAINED_GLASS_PANE;
    }

    public Inventory getInventory() {
        return inventory;
    }

    protected final ItemStack createFunctionalItem(String name, Material material) {
        ItemStack itemStack = new ItemStack(material);
        ItemMeta itemMeta = itemStack.getItemMeta();
        itemMeta.setDisplayName(name);
        itemStack.setItemMeta(itemMeta);

        return itemStack;
    }
}

In my child class it gets handled like following

    @Override
    public void handle(InventoryClickEvent event) {
        ItemStack clicked = event.getCurrentItem();
        Player player = (Player) event.getWhoClicked();

        MainGUI mainGUI = new MainGUI(player);
        NameGUI nameGUI = new NameGUI(player);
        SkinGUI skinGUI = new SkinGUI(player);

        //Main GUI
        if(Arrays.equals(event.getClickedInventory().getContents(), mainGUI.getInventory().getContents())) {
            switch(clicked.getItemMeta().getDisplayName()) {
                case "Set Name" -> player.openInventory(nameGUI.getInventory());
                case "Set Skin" -> player.openInventory(skinGUI.getInventory());
            }
            event.setCancelled(true);
        }
    }

But if I test it gets called 2 times on the first click and on the following so many times it even forces my game to crash. I know I can just put in a delay but I really want to know why this is the case.

Thank you

Upvotes: 0

Views: 532

Answers (1)

Elikill58
Elikill58

Reputation: 4908

It's running multiple times because you are adding each new GUI to listener.

For example, here:

MainGUI mainGUI = new MainGUI(player);
NameGUI nameGUI = new NameGUI(player);
SkinGUI skinGUI = new SkinGUI(player);

You are creating 3 GUIs, so 3 new inventory click event register in listener.

To fix this, I suggest you to :

  1. Make ONE class that receive InventoryClickEvent event, and call GUI that you be called.

For example, you have a list with all GUI:

public static List<GUI> ALL_GUIS = new ArrayList<>();

Then, you should have a way to determine in which inventory the player clicked, such as:

  • Inventory name. Not very good solution if this can change, but easier to create and use
  • Inventory holder like this:
public class MainGUIHolder implements InventoryHolder {

    @Override
    public Inventory getInventory() {
        return null;
    }
}

Now use it like that:

Inventory mainInv = Bukkit.createInventory(new MainGUIHolder(), 9, "My inv");

You can check with inventory instanceof MainGUIHolder, then get the holder instance with maybe some object that is currently edited by the player

  1. Don't create multiple times GUI instance. For me, it's not a good way, and I think it's better to do like this :
MainGUI mainGUI = GuiManager.getMainGUID(); // here get the alone main GUI instance
// now use it

Upvotes: 2

Related Questions