Paper
Paper copied to clipboard
PaperMC
Raw Itemstack improvements
Closes https://github.com/PaperMC/Paper/issues/5188 Paper has fixed enchantment order inconsistencies but has also implemented the fix on getter which is useless because no write operation is done here In fact i think all methods outside of the ItemMeta class shouldn't call itemmeta at all even for setter (or at least it should be documented) because developer who use this method don't think that the whole tag will be reparsed
Closes https://github.com/PaperMC/Paper/issues/5071 Remove the reparse of the whole itemmeta because the tag is already here at this stage. This method just change the item in the nms class (also remove the bukkit cached itemstack impl), and the itemmeta will be reparsed when the developer recall getItemMeta() without the help of bukkit
As said on discord, the second fix keep bad data, the new one doesn't allow any bad data including (count overflow, enchantment, tags, custom model and damage on undamageable item). I have also change all the enchantment related methods to just use the TreeMap instead of a plain ItemMeta
I really don't like a lot of what's going on here. You have some failing item meta tests that have to be addressed, and I think you're going to encounter more that fail due to the item meta changes. There is a lot more nbt that has to be addressed than just enchantments, model data, and damage. Some nbt tags apply to more than one item type, like book pages, some doesn't. The way item meta is setup now, with the applyTo method and the constructors that take an instance of CraftMetaItem to copy stuff over... all of that logic is going to have to be replicated inside the setType method.
Yes i know the things is that itemmeta rebuild the whole tag from the ground. And it's hard to replicate but if the test covered all the cases it should be easier to fix
There's a lot of stuff going on in here, one or two opinionated things which shouldn't be in there, and generally a lot of extra stuff which has 0 validation around a system that is already a massive arse mess...
Some of this may make sense as separate PRs, but, as is, this is unreviewable to us
Yes that's fine, in fact the whole item meta is bad since it recreate the nbttag from the ground (and that happens even if you move your item from one pixel in your inventory). The question is can i reopen for the first issue: 5188 ? without all that mess