API implementation discrepancy: Inventory setItem does not create a copy of the passed item stack

[blablubbabc]

Although this does not seem to be documented in the Bukkit API itself, I noticed that some of my Bukkit plugins are relying on the assumption that the API implementation creates copies of the passed ItemStacks when setting the item of some inventory slot. I.e. to avoid unnecessarily copying the item (including all of its meta data, which can be relatively costly performance-wise on CraftBukkit), the plugin omits creating another set of item stack copies when using Inventory#setItem(int, ItemStack).

On CraftBukkit and Spigot this behaves as expected. Those implementations already copy the item stack when converting it to an underlying Minecraft item stack. But I wanted to check if there are other implementations that behave differently and came across Glowstone.

Here, the items passed by plugins seem to be stored stored directly in the inventory slots (at least in some cases, if it is not replaced during sanitization, see https://github.com/GlowstoneMC/Glowstone/blob/dev/src/main/java/net/glowstone/inventory/GlowInventorySlot.java#L68). If a plugin sets the same item stack instance to multiple slots and then modifies it, I assume that the items in all of those slots are automatically modified as well. And if another plugin retrieves the item stack from the inventory and modifies it, this will likely affect the original item stack instance which the first plugin might still hold a reference to.

I don't know if this discrepancy might be something to look into for the Glowstone implementation. I would really prefer (and probably continue) to not have to unnecessarily create copies of these item stacks in my plugins when run on CraftBukkit/Spigot/derivatives (which the majority of users run them on).

Edit: This might also apply in other contexts as well, such as setting the item on a player's cursor.

[mastercoms]

Hi, sorry for the delay and thanks for the report! We do care about compatibility with the vanilla based CraftBukkit implementations, so will accept any patches that add this cloning feature for setItem.