Solved Trouble w/ "If, Else If, Else" Statement

Discussion in 'Spigot Plugin Development' started by Skyfallin, Jul 24, 2018.

  1. Hello! I am trying to write a bit of code that disables players from interacting with their inventory slot 0 (the first slot on the hotbar) unless there is an item on their cursor with a specific metadata.

    • If the player simply clicks on slot 0, I would like to cancel the event and display a message: "This is your weapon slot."
    • If the player clicks on slot 0 with an item that doesn't have the metadata, I would like to cancel the event and display a DIFFERENT message: "You may only place weapons in this slot."
    • If the player clicks on slot 0 with an item with the correct metadata, I want the InventoryClickEvent to actually fire and display: "You have changed your weapon!"
    Here is my current code:
    Code (Java):

    @EventHandler
    public void onInventoryClick(InventoryClickEvent weaponevent) {

        Player p = (Player) weaponevent.getWhoClicked();
        int itemslot = weaponevent.getSlot();

        if (itemslot == 0 && p.getGameMode() == GameMode.SURVIVAL && p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(false);
            p.sendMessage("You have changed your weapon!");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && !p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(true);
            p.sendMessage("Only weapons may go in this slot.");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && p.getItemOnCursor() == null){

            weaponevent.setCancelled(true);
            p.sendMessage("This is your weapon slot.");

            }
        }

     
     
  2. Hello there!
    What exactly is the problem with the if, else or if else statements?

    Edit: Also I would recommend checking only for "weapon", instead of ChatColor + "weapon" because it could be a bit tricky in item metadata
     
  3. Hello!
    Well, the sections which are supposed to cancel the event simply do not cancel the event. Additionally, there is this error:
    [01:02:32] [Server thread/ERROR]: Could not pass event InventoryClickEvent to NewRebirthCore v1.0
    org.bukkit.event.EventException: null
    at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62) ~[spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:500) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:485) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.PlayerConnection.a(PlayerConnection.java:1889) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.PacketPlayInWindowClick.a(SourceFile:33) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.PacketPlayInWindowClick.a(SourceFile:10) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.PlayerConnectionUtils$1.run(SourceFile:13) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) [?:?]
    at java.util.concurrent.FutureTask.run(Unknown Source) [?:?]
    at net.minecraft.server.v1_12_R1.SystemUtils.a(SourceFile:46) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.MinecraftServer.D(MinecraftServer.java:748) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.DedicatedServer.D(DedicatedServer.java:406) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.MinecraftServer.C(MinecraftServer.java:679) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at net.minecraft.server.v1_12_R1.MinecraftServer.run(MinecraftServer.java:577) [spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    at java.lang.Thread.run(Unknown Source) [?:?]
    Caused by: java.lang.NullPointerException
    at Events.WeaponSlotEvent.onInventoryClick(WeaponSlotEvent.java:28) ~[?:?]
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:?]
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:?]
    at java.lang.reflect.Method.invoke(Unknown Source) ~[?:?]
    at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:302) ~[spigot_1.12.2.jar:git-Spigot-642f6d2-fbe3046]
    ... 15 more
     
  4. The error is probably because you are checking for null after trying to get item on cursor, at least my guess.
     
  5. Is there another way to ask it to check if the player's cursor is empty?
     
  6. I would probably just try changing this:
    Code (Java):
    @EventHandler
    public void onInventoryClick(InventoryClickEvent weaponevent) {

        Player p = (Player) weaponevent.getWhoClicked();
        int itemslot = weaponevent.getSlot();

        if (itemslot == 0 && p.getGameMode() == GameMode.SURVIVAL && p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(false);
            p.sendMessage("You have changed your weapon!");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && !p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(true);
            p.sendMessage("Only weapons may go in this slot.");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && p.getItemOnCursor() == null){

            weaponevent.setCancelled(true);
            p.sendMessage("This is your weapon slot.");

            }
        }
    to this:
    Code (Java):
    @EventHandler
    public void onInventoryClick(InventoryClickEvent weaponevent) {

        Player p = (Player) weaponevent.getWhoClicked();
        int itemslot = weaponevent.getSlot();

        if (itemslot == 0 && p.getGameMode() == GameMode.SURVIVAL && p.getItemOnCursor() != null && p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(false);
            p.sendMessage("You have changed your weapon!");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && p.getItemOnCursor() != null && !p.getItemOnCursor().getItemMeta().getLore().contains(ChatColor.GREEN + "Weapon")) {

            weaponevent.setCancelled(true);
            p.sendMessage("Only weapons may go in this slot.");

        } else if ((itemslot == 0) && (p.getGameMode() == GameMode.SURVIVAL) && p.getItemOnCursor() == null){

            weaponevent.setCancelled(true);
            p.sendMessage("This is your weapon slot.");

            }
        }
    EDIT: And while you are at it try checking if the item has a ItemMeta and lore aswell.
    EDIT2: I put the code in spoilers.
     
    #7 KarimAKL, Jul 24, 2018
    Last edited: Jul 24, 2018
  7. While it's not really related to the question, I would suggest to never use:
    Code (Java):
    weaponevent.setCancelled(false);
    Right now you're basically saying with that piece of code that the event is no longer cancelled. If there is any other plugin that cancelled it for whatever other reason, you're now overriding it. For example, you have 2 plugins. Plugin 1 cancelled the event. Your plugin, plugin 2, would now uncancel it. You're better off only using the Event#setCancelled function with a value of true, just to prevent conflicting with other plugins.

    Get the cursor item object, check if that's null OR if the material is equal to air. I'm not fully sure what Bukkit does nowadays. Sometimes it returns null when an item doesn't exist, sometimes it returns an itemstack of air.
     
    • Agree Agree x 1
  8. This error occurs because your Lore is may be null, and you are trying go get something from the Lore. I would check first if the Lore is null.

    Oh and check also if the item isnt null, because if you would click somewhere outside your inventory, it wont return an item.
     
  9. For one, you can use the method provided by the event, InventoryClickEvent#getCursor(), which won't return null but instead an ItemStack with the material type air. If it's an empty slot in an inventory, on the other hand, it's null.
    ^^^ ;P

    Edit:// also check that the clicked inventory/(maybe?) slot isn't null either. If you click off the inventory, NPE.
     
  10. I think I fixed your code, but I haven't tested it.

    Code (Java):
        @EventHandler
        public void onInventoryClick(InventoryClickEvent weaponevent) {

            Player p = (Player) weaponevent.getWhoClicked();
            int itemslot = weaponevent.getSlot();
            if (weaponevent.getClickedInventory() != null && itemslot == 0 && p.getGameMode() == GameMode.SURVIVAL) {
                if (p.getItemOnCursor() != null) {
                    boolean hasLore = false;
                    if (p.getItemOnCursor().hasItemMeta() && p.getItemOnCursor().getItemMeta().hasLore()) {
                        for (String lore : p.getItemOnCursor().getItemMeta().getLore()) {
                            if (lore.equals("┬žaWeapon")) {
                                hasLore = true;
                                break;
                            }
                        }
                    }
                    if (hasLore) {
                        p.sendMessage("You have changed your weapon!");
                    } else {
                        weaponevent.setCancelled(true);
                        p.sendMessage("Only weapons may go in this slot.");
                    }
                } else {
                    p.sendMessage("This is your weapon slot.");
                }
               
     
    #11 Olivo, Jul 24, 2018
    Last edited: Jul 24, 2018
    • Optimistic Optimistic x 1
  11. Unnecessary for loop. Unnecessary boolean. Still null checking against something that won't be null. No null check against the clicked inventory.

    Also, now that I think about it, to OP, you need to check that it's the correct inventory otherwise it'll do this for the first slot of any inventory.
     
  12. I've tried this, and this is much closer to the desired effect! Although, I must admit this code is a bit over my head. Escad is right, it needs to check for a player's inventory or the code will trigger on chests, etc. Aside from that, the one thing still missing is the different message when the slot is clicked with no item on the cursor. Currently, it displays "Only weapons may go in this slot." in the two cases of a weapon with incorrect metadata and no weapon at all.

    EDIT: The console error is gone as well! :)
     
  13. So I fixed the code based on what Escad said:

    Code (Java):
        @EventHandler
        public void onInventoryClick(InventoryClickEvent weaponevent) {

            Player p = (Player) weaponevent.getWhoClicked();
            int itemslot = weaponevent.getSlot();
            if (weaponevent.getClickedInventory() != null && itemslot == 0 && p.getGameMode() == GameMode.SURVIVAL && weaponevent.getClickedInventory().getType().equals(InventoryType.PLAYER)) {
                if (p.getItemOnCursor() != null) {
                    if (p.getItemOnCursor().hasItemMeta() && p.getItemOnCursor().getItemMeta().hasLore() && p.getItemOnCursor().getItemMeta().getLore().contains("┬žaWeapon")) {
                        p.sendMessage("You have changed your weapon!");
                    } else {
                        weaponevent.setCancelled(true);
                        p.sendMessage("Only weapons may go in this slot.");
                    }
                } else {
                    p.sendMessage("This is your weapon slot.");
                }
            }
        }
    Edit: The code still isn't tested
     
  14. @Olivo you shouldn't spoonfeed code though.
    InventoryMoveItemEvent is fired by i.e. hoppers, not Players.

    @OP I'd first check if the player is either in creative, or if the slot isn't 0, then return (preferring early returns over nested if statements yields to less indentation).

    Then just do the same checks you did before, but first check whether they don't hold an item, so that the other two statements aren't ran if the player holds no items.
     
  15. Solved the problem, thanks everyone and Olivo!

    Problem was I wasn't paying attention to my brackets, so certain "if" conditions were nested too deep and would never fire.
     
  16. And the error (NullPointerException) is gone now aswell?
     
  17. I had to move some things around, but the error is gone as well.
     
  18. I see, good to know. :)