Why is this not working

Discussion in 'Spigot Plugin Development' started by Known, May 3, 2017.

  1. public class CharmListener implements Listener {


    @EventHandler

    public void onPlayerInteractEvent(PlayerInteractEvent e) {

    if(e.getPlayer().getItemInHand().getType() == Material.GOLD_INGOT){

    ItemStack charm = new ItemStack(Material.GOLD_INGOT);

    ItemMeta charmmeta = charm.getItemMeta();

    if(!(charmmeta.getDisplayName() == ChatColor.RED + "Painbringer")) {

    return;

    }

    if(e.getAction() == Action.RIGHT_CLICK_AIR || e.getAction() == Action.RIGHT_CLICK_BLOCK) {


    e.getPlayer().addPotionEffect(new PotionEffect(PotionEffectType.REGENERATION, 160, 1));

    not sure why this isnt working,,, i am new to this aha
    }

    }

    }

    }
     
  2. I know it is something to do with the item meta.. but im not sure what
     
  3. Compare strings with .equals(), not ==
     
    • Agree Agree x 1
  4. When doing if(!(charmmeta.getDisplayName() == ChatColor.RED + "Painbringer")) {, you can just have it != instead of inverting the result of being equals, and strings should be compared with .equals
     
  5. need to check if player has a null item in there hand first
     
  6. Post your entire class so we can see what the problem is, please.
     
  7. public class CharmListener implements Listener {


    @EventHandler

    public void onPlayerInteractEvent(PlayerInteractEvent e) {

    if(e.getPlayer().getItemInHand().getType() == Material.GOLD_INGOT){

    ItemStack charm = new ItemStack(Material.GOLD_INGOT);

    ItemMeta charmmeta = charm.getItemMeta();



    if(!(charmmeta.getDisplayName().equals(ChatColor.RED + "Painbringer"))) {



    return;



    }

    if(e.getAction() == Action.RIGHT_CLICK_AIR || e.getAction() == Action.RIGHT_CLICK_BLOCK) {


    e.getPlayer().addPotionEffect(new PotionEffect(PotionEffectType.REGENERATION, 160, 1));


    }

    }

    }

    }
     
  8. Thats my class, not sure what is wrong with it
     
  9. Can use code tags? This is very hard to read since some lines have two random line breaks in between and no formatting. And I think it's throwing an NPE because you create a random item, check its item meta which is probably null. Why don't you use the item in the player's hand?
     
  10. It is the item in the players hand, line 9 is throwing the error : https://hastebin.com/iyebeculat.java
     
  11. Code (Text):
    charmmeta.getDisplayName()
    Because you have created a new ItemStack and got the meta of that, its display name is still null and thus causing a NPE.
     
  12. No, you don't use the item in the player's hand, you make a new item with a material of gold. Just use the item in the player's hand as the itemstack...
     
  13. You should check the event action BEFORE you check the player's held item. For example:
    Code (Text):
    public class CharmListener implements Listener {


    @EventHandler
    public void onInteract(PlayerInteractEvent e) {
    if(e.getAction() == Action.RIGHT_CLICK_AIR || e.getAction() == Action.RIGHT_CLICK_BLOCK) {
    if(e.getPlayer().getItemInHand().getType() == Material.GOLD_INGOT) {

    ItemStack charm = new ItemStack(Material.GOLD_INGOT);

    ItemMeta charmMeta = charm.getItemMeta();

    if(!(charmMeta.getDisplayName() == ChatColor.RED + "Painbringer")) {
    return;
    }

    e.getPlayer().addPotionEffect(new PotionEffect(PotionEffectType.REGENERATION, 160, 1));
                            }
                            return;
                    }
    return;
            }
    return;
    }
    lmao you guys should've said this by now.
    P.S. I gave up formatting half way through the example (I'm on my phone), so don't criticize on it.
     
  14. You didn't take in any of the previous points criticizing this. You could have said check for the event action first, instead of giving bad code.
     
  15. Out of all things wrong, I'm surprised you didn't pick anything from that list. (action checking is sort of unrelated to why his code isn't even working)
    Fun fact, every time you call getItemMeta() it creates a fresh ItemMeta instance (except for AIR stacks).

    @Known to summarise what others have said before:
    • Check if the player is holding something (in other words, if the held ItemStack isn't null.
    • Check if that specific ItemStack, not some new one you just create, has ItemMeta, and a display name
      • Checking for ItemMeta can be done through ItemStack::hasItemMeta() (don't blindly call getItemMeta(), it'll create instances you don't need)
      • Checking for a display name can be done through ItemMeta::hasDisplayName() (after you've verified the ItemStack has ItemMeta, you can grab it and call this method)
    • Then check if the name matches
      • you can technically also just swap the equals around, i.e. using
        Code (Java):
        (ChatColor.RED + "Foo").equals(meta.getDisplayName())
        since the name is a literal - i.e. never null, which would mean you never need to check hasDisplayName()
     
    • Agree Agree x 2
    • Like Like x 1
    • Useful Useful x 1
  16. I would suggest using ItemStack#isSimilar

    Your template ItemStack:

    Code (Java):

    private static final ItemStack template = new ItemStack(Material.GOLD_INGOT);
    static {
    ItemMeta itemMeta = template.getItemMeta();
    itemMeta.setDisplayName(ChatColor.RED + "Painbringer");
    template.setItemMeta(itemMeta);
    }
     
    Checking for similarity in your event method:

    Code (Java):

    @EventHandler
    public void onPlayerInteract(PlayerInteractEvent event) {
    if (template.isSimilar(event.getItem()) {
    //here you take action upon the used item being similar
    }
    }
     
    Quoted from the docs:
    "This method is the same as equals, but does not consider stack size (amount)."