Solved Confusing Error

Discussion in 'Plugin Help/Development/Requests' started by ibc2244, Aug 1, 2015.

Thread Status:
Not open for further replies.
  1. Hello Bukkit, today I've been trying to stop players moving a blaze rod around in there inventory (It's for particle trails) but I've found that when you click outside of the inventory, it throws an error and I'm not sure why!
    !- Error -!
    Code:
    [17:24:43 ERROR]: Could not pass event InventoryClickEvent to BoomCraftMain v1.0
    org.bukkit.event.EventException
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:310) ~[spigot-1.8
    .8.jar:git-Spigot-6c9b0a1-de5c261]
            at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62) ~[spigot-1.8.8
    .jar:git-Spigot-6c9b0a1-de5c261]
            at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:502) [spigot-1.8
    .8.jar:git-Spigot-6c9b0a1-de5c261]
            at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:487) [spigot-1.8
    .8.jar:git-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.PlayerConnection.a(PlayerConnection.java:1630) [spigot-1.8.8
    .jar:git-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.PacketPlayInWindowClick.a(SourceFile:31) [spigot-1.8.8.jar:g
    it-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.PacketPlayInWindowClick.a(SourceFile:9) [spigot-1.8.8.jar:gi
    t-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.PlayerConnectionUtils$1.run(SourceFile:13) [spigot-1.8.8.jar
    :git-Spigot-6c9b0a1-de5c261]
            at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) [?:1.8.0_51]
            at java.util.concurrent.FutureTask.run(Unknown Source) [?:1.8.0_51]
            at net.minecraft.server.v1_8_R3.SystemUtils.a(SourceFile:44) [spigot-1.8.8.jar:git-Spigot-6c
    9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.MinecraftServer.B(MinecraftServer.java:714) [spigot-1.8.8.ja
    r:git-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.DedicatedServer.B(DedicatedServer.java:374) [spigot-1.8.8.ja
    r:git-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.MinecraftServer.A(MinecraftServer.java:653) [spigot-1.8.8.ja
    r:git-Spigot-6c9b0a1-de5c261]
            at net.minecraft.server.v1_8_R3.MinecraftServer.run(MinecraftServer.java:556) [spigot-1.8.8.
    jar:git-Spigot-6c9b0a1-de5c261]
            at java.lang.Thread.run(Unknown Source) [?:1.8.0_51]
    Caused by: java.lang.NullPointerException
            at com.ibc2244.boomcraftMain.events.EventInvClick.onInvClick(EventInvClick.java:20) ~[?:?]
            at sun.reflect.GeneratedMethodAccessor45.invoke(Unknown Source) ~[?:?]
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_51]
            at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_51]
            at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:306) ~[spigot-1.8
    .8.jar:git-Spigot-6c9b0a1-de5c261]
            ... 15 more
     
  2. Offline

    schwabfl

  3. Offline

    BizarrePlatinum

    @ibc2244 could this possibly be due to not testing null before testing type.
     
  4. Offline

    Boomer

    It is the result of assuming that the click event means that a slot was selected and an item picked up or placed down, and most likely is occuring on a getSlot() check, because the click was actually peformed 'off stage' where a drop-item is executed, and there is no slot involved. Or rather, assuming the slot exists, and diving directly for the item in it getSlot().getSomething(), which 'isn't a problem on empty cells since it returns air and not null' so you wouldnt think that its possible to generate a null object from a quick test, until meeting the off-stage drop item
     
  5. @schwabfl @BizarrePlatinum @Boomer
    This is line 20 of the EventInvClick class:
    Code:
          
    if (!(e.getCurrentItem().getType() == Material.BLAZE_ROD)) {
    return;
    }
    
    And here's the whole class:
    Code:
    package com.ibc2244.boomcraftMain.events;
    
    import org.bukkit.Material;
    import org.bukkit.event.EventHandler;
    import org.bukkit.event.Listener;
    import org.bukkit.event.inventory.InventoryClickEvent;
    
    import com.ibc2244.boomcraftMain.Main;
    import com.ibc2244.boomcraftMain.utils.UtilStrings;
    
    public class EventInvClick implements Listener {
    
    public EventInvClick(Main plugin) {
            plugin.getServer().getPluginManager().registerEvents(this, plugin);
    }
    
    @EventHandler
    public void onInvClick(InventoryClickEvent e) {
    
    if (!(e.getCurrentItem().getType() == Material.BLAZE_ROD)) {
    return;
    }
    
    if (e.getSlot() == 4) {
    if (e.getCurrentItem().getType() == Material.BLAZE_ROD) {
    if (e.getCurrentItem().getItemMeta().getDisplayName().contains(UtilStrings.ccLightPurple + "" + UtilStrings.ccBold + "Particle Trails")) {
    e.setCancelled(true);
    }
    }
    }
    }
    }
    
     
    Last edited: Aug 2, 2015
  6. Offline

    Boomer

    if (!(e.getCurrentItem().getType() == Material.BLAZE_ROD)) {

    Lets look at every possibility of null and how that might impact that line:

    In this case, Material is an Enum, so Material.BLAZE_ROD always exists, there is no chance of Material being null

    e.getCurrentItem().getType() .. that object is allowed to be null, and will not throw an error because you are merely checking if a null thing == something else, which is a valid test. So the getType() could return a null, BUT it would result in no error on THIS line.

    e.getCurrentItem() object could be null, in which case, you can not dereference it further to access its .getType() method. There is no .getType() method for the null object.

    e object: your event object. 0.00000000% probability of it being null

    Thus, the winner is e.getCurrentItem() is null at this point in execution. Why so? Because by clicking offstage and dropping the object.. there is no current item. (You should also try clicking on an empty space and see if that throws the same error, or if as I recalled, it registers AIR for the click on an empty slot)

    Solution: If that object is null.. you dont have anything manipulated and thus, this nothingness cant possibly be blazerod, so check for null, and if it is null, return out of the event.
    Otherwise, it falls through, and non-null e.getCurrentItem() object now can have its type gotten and checked as you have.

    ====

    "BUT WAIT! I have 'If the item is not blazerod then return' so why doesn't that work?" you cry out in defence and confusion.
    You dont. You have 'if the material type of the item is not a blazerod material then return'
    You're one layer deeper than 'the item' , you've accessed a property on 'the item' when 'the item' doesn't exist
    If you merely compared 'the item' to 'an itemstack of blazerod' then you would be directly comparing a null object to something. One layer deeper, you're comparing null.someThing() which is the impossible thing to get for manipulating or comparing. A null can be a valid return, sub-accessing a null object is the failure.
     
    Last edited: Aug 2, 2015
  7. @Boomer
    When I click any other slot there's no error.
    Also, how would I check if "e.getCurrentItem == null"? Would I use something like this:

    Code:
    if (e.getCurrentItem == null) {
    return;
    }
     
  8. Offline

    Boomer

    Yeah, as i thought - the empty other slots are registering as an air item, with material AIR as type , so you would have never expected a null was possible in planning until that fateful one time clicking off-stage :)

    Yes, thats exactly you you check for null - except of course, valid method :) e.getCurrentItem()

    Same principle applies ANY time you ever get a null pointer error, they are fantastic in that they always point to the exact line of an NPE, you just do what I did - break down everything on the line and say 'can this be null - never; can this be null - maybe. If it could, is it a final return object (.getType() ) in which case, if its null it wont through the error here. If it could, is it something I'm dereferencing (e.getCurrentItem().getType() de-references e.getCurrentItem() ) if so, and that is null, it would throw the null pointer error. Do for everything, then figure out where you need to check for null to confirm or clarify, and what that means - it may mean in this case, there is nothing to act on. In other cases it may mean you haven't initialized something... we see a lot of NPEs when folks pass a plugin through a constructor, but dont do anything with it, and the local plugin variable remains uninitalized.

    If you ever have a monster like:
    Z=objectX.getY().doStuff(objectA.getB().getC().getD().doSomething()); being the line throwing errors, it may take forever to figure out what is null, and it may not be logically obvious - sometimes it is "oh dumb me, i forgot to set objectX after initializing it two lines above).. Most often, not, so you absolutely need to test in order to have a clue how to make sense of it and solve. obviously, objectA could be null, as could the returns from getB, getC and getD. In this case, if doSomething is null, the argument is null, and will likely be responsible for breaking other code elsewhere; some core bukkit code might point the npe at here, but usually it will be in some other code, yet ..refer back to this line in the stacktrace. And of course, objectX.getY() could be null, or objectX either throwing the error.

    Once you know the trick to listing off the possibilities, how to check the possibilities, and then what it means to you (in your case, it meant it was an oddball result for the click event but translated to 'i disregard that case anyways' which was nice; in other cases it will mean you forgot to initialize things, didn't set the right objects, or have other deeper issues, but at least doing the footwork and saying "ah hah, its the getC() that is null, but i dont know why" folks will be able to help identify why, and appreciate that you say "and its because getC() is the null object, i tested that.."
     
  9. @Boomer
    But how would I stop it throwing the error?
     
  10. Offline

    Boomer

    you stop throwing an error when you are no longer derefereincing a null object.

    By catching the .getCurrentItem() == null you are able to determine that you have no need to bother with additional code, so then .getCurrentItem().getType() will NEVER EVER be operating with a null object - it will ALWAYS be a non-null item object that has a material.

    Catch the null object, dont dereference it any further --> No null pointer object.

    In this case you are dealing with, the null is thrown by Bukkit, and you have ABSOLUTELY NO control over that being a null - its not your fault, its not something you can go back elsewhere and change something so that it wont be null. It will be null in the circumstance you have seen. So you handle it. You dont get fooled by it being null, you fish it out of there right away, and in this case, you return; out of the code since the practical implication of it being null is "whatever it is, its not a blazerod, i dont care'. So your flow is ... "First, is that item null - yes, stop right here, go away. No - then lets find out what material it has. Not blazerod - stop right there, go away. Otherwise, blazerod, lets do our thing"

    if you've added your
    Code:
    if (e.getCurrentItem() == null) {
        return;
    } 
    as the first part of that event handler , before checking its type...
    You would see that you are no longer generating a null pointer error when clicking off stage, as you had before. You solved the problem at that spot.
     
    ibc2244 likes this.
  11. @Boomer
    Thank you. I added it after the "if (!())" statement. I'll test that soon It works thank you! :D
     
    Last edited: Aug 2, 2015
Thread Status:
Not open for further replies.

Share This Page