[Swords] When Hand contains Sword

Discussion in 'Spigot Plugin Development' started by LuckyLuckiest, Apr 24, 2017.

  1. When a Hand contains a sword it will add a shield to the offHand, so everything works fine except one thing, once you don't have a sword in your hand it won't give a shield to yours offHand.
    Code (Java):
    @SuppressWarnings("deprecation")
        @EventHandler
        public void onPlayerSwordRightClick(PlayerInteractEvent e) {
            if (e.getPlayer() instanceof Player) {
                Player p = e.getPlayer();
                PlayerInventory inv = p.getInventory();

                ItemStack shield = new ItemStack(Material.SHIELD);
                ItemStack air = new ItemStack(Material.AIR);

                boolean sword = p.getItemInHand().getType().toString().contains("SWORD");

                if (sword) {
                    inv.setItemInOffHand(shield);
                } else if (!sword) {
                    inv.setItemInOffHand(air);
                }
            }
        }
     
  2. You want to use ItemHeldEvent
     
  3. Uhm, yes, that's what this code does. If that's not what you want it to do, why did you declare and assign the sword boolean?
    Why do you use the deprecated getItemInHandMethod? It makes no sense, use the getItemInMainHand() method.
     
  4. Nothing is called ItemHeldEvent
    Is there another event ?
     
  5. Thanks @Trigary for telling me about PlayerItemHeldEvent
    It solved my problem because with PlayerInteractEvent it uses Action RightClick and that made everything go wrong!
    Code (Java):
    @EventHandler
        public void onPlayerSword(PlayerItemHeldEvent e) {
            if (e.getPlayer() instanceof Player) {
                Player p = e.getPlayer();
                PlayerInventory inv = p.getInventory();

                ItemStack shield = new ItemStack(Material.SHIELD);
                ItemStack air = new ItemStack(Material.AIR);

                boolean sword = inv.getItemInMainHand().getType().toString().contains("SWORD");

                if (!sword) {
                    inv.setItemInOffHand(shield);
                } else {
                    inv.setItemInOffHand(air);
                }
            }
        }
     
    #6 LuckyLuckiest, Apr 24, 2017
    Last edited: Apr 24, 2017
  6. What now?!
    That's what I told you. :mad:
     
  7. Didn't see that replay lol
    Yeah couldn't quite remember the name
    It's PlayerItemHeldEvent
     
  8. Hate to be that one, but here's a few nitpicks regarding the code - I'm sorry.
    1. You have an useless check right there. a Player is always an instanceof Player, so may aswell just remove the line
    Code (code (Unknown Language)):
    if (e.getPlayer() instanceof Player) {
    .. because it'll always return true.
    2. Your way to check for a sword may work, but it's in no way the best way you could do that. That said, I can never be the way I'd do it would be the best option ever, but I'd say it's better than doing it like that. So, there's this handy class called EnchantmentTarget, which exists for pretty much this purpose. You can check if an ItemStack is a sword with the following:
    Code (Java):
    EnchantmentTarget.WEAPON.includes(yourItemStack)
    3. As stated above, don't use the deprecated Player#getItemInHand() method. I do see why most of people end up with that, so I can't really blame you for it. Use Player.getInventory().getItemIn<Main/Off>Hand() instead. Main in this case, assuming I understood what you're trying to achieve right. Also, you already have the PlayerInventory as a variable, so you can just use that.
    4. That IF-statement. You know, if the first condition returns true, the 'else' block will not be called in the first place. And vice versa, if the first block returns false, the 'else' block will be called; but the 'if' block never will. Now, it's not a big deal, but the second IF-clause:
    Code (Java):
    if (!sword)
    (Note the '!') will always return true. Might as well remove it :)
    5. You shouldn't use PlayerInteractEvent here; use the PlayerItemHeldEvent as others already suggested. That'll be called every time the player changes the item in main hand.
    (5.5. You're never checking if the player actually right-clicks. You're just assuming the interaction was a right-click, while left-click will here provide the exact same results - and I believe that isn't something you want. You can check what the performed action was by calling PlayerInteractEvent#getAction() and checking if it's the correct one.)
    6. The order in which you do things. You first create the ItemStack instances, and THEN check if the conditions are met and whether you should do anything with them. Not only are you creating useless ItemStacks on every single interaction, but also performing useless changes to the inventory all the time. Not to forget that if someone puts something in their offhand, this'll just brutally remove it. So, do it in the following order:
    - FIRST check if the main hand is a sword
    - If it is, ensure the offhand slot is empty (otherwise stop here, of course)
    - Now, create the Shield ItemStack and put it to the offhand slot

    and if the main hand isn't a sword, check if the offhand slot isn't empty OR it's a shield; and if so, set it to null. You needn't to pass an AIR itemstack as Bukkit stores empty inventory slots as null values.

    EDIT: The heck's going on with the positive ratings? Thank you!
     
    #9 jetp250, Apr 24, 2017
    Last edited: Apr 24, 2017
    • Like Like x 2
    • Winner Winner x 1
    • Informative Informative x 1
    • Creative Creative x 1