1.16.5 When to clone an ItemStack (why does this code not work otherwise?)

Discussion in 'Spigot Plugin Development' started by Sigong, Jun 17, 2021.

  1. Hello! I have the following code in a class. The code is working as desired (after an inventory click in the helmet slot, a half-second passes and then the player is sent a message saying whether or not the item that was on the cursor when the click occurred was placed in the helmet slot or remained on the cursor).
    In order to get this code to work, however, I needed to call .clone() on the cursor itemstack that is passed to the runnable. If I remove that call, the runnable seems to have the same itemstack for both beforeCursorItem and AfterCursorItem, and thus says the item wasn't placed always.

    Can anyone give me a detailed explanation on why I had to do this? I have never had to do this before for other things I made that interacted with the inventory.

    Code (Java):
    @EventHandler
    public void onClickInHelmetSlot(InventoryClickEvent event){
     
        if(event.getInventory().getType() == InventoryType.CRAFTING &&
                event.getSlotType() == InventoryType.SlotType.ARMOR &&
                event.getRawSlot() == 5 &&
                event.getWhoClicked().getItemOnCursor().getType() != Material.AIR){

            Bukkit.getConsoleSender().sendMessage("Detected valid click, scheduling runnable");
         
            new clickCheckRunnable(event.getWhoClicked().getItemOnCursor().clone(), event.getWhoClicked().getUniqueId()).runTaskLater(instance,10L);
        }
    }

    private class clickCheckRunnable extends BukkitRunnable {

        public clickCheckRunnable(ItemStack beforeCursorItem, UUID playerUUID){
            this.beforeCursorItem = beforeCursorItem;
            this.playerUUID = playerUUID;
        }

        private final ItemStack beforeCursorItem;
        private final UUID playerUUID;

        @Override
        public void run() {
            Player player = Bukkit.getPlayer(playerUUID);
         
            ItemStack afterCursorItem = player.getItemOnCursor(); //Will be air if nothing on cursor

            Bukkit.getConsoleSender().sendMessage(beforeCursorItem + "; " + afterCursorItem);

            //If cursor item is the same before and after the click occurs (it won't be air, this is prevented before getting to this point)
            if(beforeCursorItem.getType() == afterCursorItem.getType()){
                player.sendMessage("Items are the same (Item was not placed in slot)");
            }else{
                player.sendMessage("Items are different (item was placed in slot)");
            }
        }
    }
     
  2. ItemStacks, in my opinion, have weird behavior that can be expected after learning how it works. In this situation, the beforeCursorItem variable you made holds a reference to the ItemStack object from the player's inventory at the time you used the getter to receive it. When you do not clone an ItemStack, it can still be mutated in game and when a mutation happens (maybe a hotbar switch, or the player drops an item, etc.) it will change the item in reference being held by the beforeCursorItem. For me, whenever comparing ItemStacks, I always clone to make sure these errors don't happen. I hope that makes sense.
     
    • Winner Winner x 1