Solved Where to register Listeners, that are created multiple times?

Discussion in 'Spigot Plugin Development' started by Emilius123, Feb 7, 2020.

  1. Hello. First off, I'd like to apologize for this question's terrible title, though I have no idea for a better one.

    Currently, I am working on an Inventory-Gui think, following this https://www.spigotmc.org/wiki/creating-a-gui-inventory/, since I think that it's a neat way to do UI.
    Since I'm going to always create a new instance of that Gui Object for every player, I can't just register it in the main method.
    Where else should I run the registerEvents and when?


    Thanks in advance - Emil.

    If you would like any clarification on what I mean, don't be afraid to ask.
     
  2. You'd normally only register any events once...
    I think what you mean by GUI-Object is something else than en event... Can you elaborate more on what kind of events you mean?
     
  3. You need to register once a InventoryClickEvent class. In that class you check if the inventory is one of your custom gui
     
  4. Ok, @Maxx_Qc and @Schottky. Apparently, I suck at explaining. Maybe this code can help you understand, what I mean?

    The Gui class:
    Code (Java):

    public class VerifyGui implements InventoryHolder, Listener{
        //...
        public VerifyGui() {
               //...
        }
       
        @Override
        public Inventory getInventory() {
            return this.inventory;
        }
       
       
        public void addItems() {
            //...
        }
       
        //Prevents items from being moved
        @EventHandler
        public void onInventoryItemMove(InventoryMoveItemEvent e){
            if(e.getSource().getHolder() != this) {
                //Not this inventory
                return;
            }
           
            e.setCancelled(true);
        }
       
        //Prevents the inventory from being closed
        @EventHandler
        public void onInventoryClosed(InventoryCloseEvent e) {
            if(e.getInventory().getHolder() != this) {
                //Not this inventory
                return;
            }
            e.getPlayer().openInventory(this.inventory);
        }
    }
    How I'm opening it:
    Code (Java):

        //...
        @EventHandler
        public void onPlayerJoin(PlayerJoinEvent event) {
            Player player = event.getPlayer();
            VerifyGui gui = new VerifyGui("...");
            player.openInventory(gui.getInventory());
        }
     
    As you see, the Listener is the GUI and I need to register that every time for every new player.
     
  5. Wait almost all of this is deprecated/bad practice, why is this in the spigot wiki?

    InventoryHolder shouldn’t be used like this (even thou I admit having committed that crime myself), instead you should use a Map to store inventories in or use a special title (the latter one is not that good)

    You should place your listener in another class and then check if that map contains your inventory, if so, execute the special action.
     
    • Agree Agree x 2
  6. I have heard this so many times, and still can't understand, I know that this is misuse of InventoryHolder, but, why is it actually bad?!
     
  7. I mean... It'll work fine if you use it like this. I can only repeat what I have heard; InventoryHolder hasn't been designed for the purpose it is used here. This leads to two effects:
    1) No forward compatibility; newer versions of spigot might not support this
    2) Your intended behavior might not work as expected

    One example that I could think of right now where using InventoryHolder in such a deprecated way is this:
    Code (Java):
    for (Entity entity: world.getEntities()) {
        if (entity instanceod InventoryHolder) // does not account for every InventoryHolder
    }
     
  8. Sure, I'll do that. :)
     
  9. In your PlayerJoinEvent where you create an instance of the gui for them simply store that gui in a Map<UUID, VerifyGui> openGui

    Then check that map for the players uuid, also remove them from the map when your done with them.

    Edit: When you a create a listener, that listener is called for every event of that type on the server not just from your code. So one for each type you need is all you need.
     
  10. Wouldn't it be more appropiate to have the Map with the Player and the Gui (Map<Player, Gui>)?
     
  11. Very risky topic here on spigot :p. There are pros and cons to using a player as the keys of a map:

    The most important con first: If you do not clean up your map properly, there will be multiple players stored inside your map. This means, that if a player logs out, the Garbage Collector has no chance of removing that player. This will cause memory leaks eventually since every time a player logs in & out, a new Player will be stored in that map.
    There are ways of preventing that; for example you could simply remove the player on logging out. Or you could use a Weak Map; essentially saying that if there is no one that wants to keep this player in the inventory, you are not preventing the Garbage Collector from removing it.
    This results in more overhead / more potentials for errors if not done correctly

    Pros: You can have way simpler access to your player and do not need to do an extra-lookup for a player-uuid every time you access that map (which in your case would actually not matter).

    Common misunderstanding and therefore no con: Putting a player in a map will not access a single byte of additional memory compared to a UUID. Putting a player in a map will also not yield a better/worse efficiency in comparing the player with another (compared to UUIDs). Additionally, the quality of the hash is basically equal to the quality of a UUID-hash
     
    • Like Like x 1
    • Agree Agree x 1
  12. Its not good practice to store the player object, always store their uuid. Basically having the player object in a map prevents that player object from being cleaned up by garbage collection when they quit which creates a memory leak.
     
    • Agree Agree x 1