Client side crash when hovering over an item >99 stack size
Natanaelel opened this issue · 11 comments
Describe the bug
Client crashes when trying to save stacks larger than ItemStack.CODEC allows
https://github.com/Caltinor/Project-MMO-2.0/blob/main/src/main/java/harmonised/pmmo/util/TagUtils.java#L72
Paste your crash log here
crash-2024-06-21_00.35.36-client.txt
debug.log
Natanaelel/BankStorage#21
would it be fine if I pr'ed a fix?
As mentioned in the issue on your project, your choice to violate the stack limit minecraft sets makes your mod incompatible with others, not the other way around. You need to find a better way to store larger-than-vanilla stack sizes. It's PMMO today, but it will be something else tomorrow.
BLUF: don't waste your time on a PR. The issue isn't with pmmo.
The only thing that >99 stack limits violate is the vanilla codec CODEC. I think it is my responsibility as the storage mod to serialize items. Similar mods (dank, sophisticated) have the same approach of simply storing the count in the stack. I would say that your usecase of serializing items clientside is rather unique and therefore requires special caution. I'm happy to PR my suggested fix. If you don't think it is a good idea I will not spend any more time on this and simply consider pmmo incompatible.
The only thing that >99 stack limits violate is the vanilla codec CODEC. I think it is my responsibility as the storage mod to serialize items. Similar mods (dank, sophisticated) have the same approach of simply storing the count in the stack. I would say that your usecase of serializing items clientside is rather unique and therefore requires special caution. I'm happy to PR my suggested fix. If you don't think it is a good idea I will not spend any more time on this and simply consider pmmo incompatible.
In pre 1.20.6 it didn't matter if you serialized more than the stack size because the NBT limit was the integer limit. In 1.20.6+ data component have stricter limitations. PepperFly hasn't even released a 1.20.6 or 1.21 version for sophisticated backpacks yet. and Itemstack hasn't released a version for dank after 1.20.1. They will have to develop their own solutions for storing items as well. I would reach out to them to see what they plan to do. If their plan is to serialize with a custom codec as you do, then I will reconsider changing pmmo.
The line you reference is a vanilla method for serializing ItemStacks to NBT. How do you save itemstacks in your mod with a value greater than 99?
This may work for your custom serialization inside your mod, but vanilla isn't using this codec to serialize these itemstacks.
I wonder why do you have to serialize items clientside?
Project MMO let's users define the player skills that are required to interact/use items based on their NBT (for example potions are one item with different NBT values based on the potion). The tooltips show this information and therefore the client has to parse the NBT tag according to the mod configuration to display the correct requirement for the item.
I've looked at your code and I see no reason for serializing instead of just passing the ItemStack directly. Am I missing something?
yes. I need the NBT value. There is no way around reading itemstack properties dynamically without having a JSON-like representation of the data. This feature allows for mods like Tinkers/Tetra/SilentGear to (which all have one item that changes based on NBT) have custom XP and requirements based on its composite properties.
the alternative would be to have explicit compat with every possible mod and their specific items so that I could parse the config string explicitly to data components, which is not practical.