Attribute#mergeAttributes doesn't respect modifier UUIDs
bluebear94 opened this issue ยท 15 comments
Attribute#mergeAttributes completely ignores the UUIDs of the modifiers in the input multimaps, causing problems for code that depends on particular UUIDs (such as Item.ATTACK_DAMAGE_MODIFIER_ID
). I'm not sure what the intended behavior for this method is.
Actually, since the purpose of mergeAttributes
is to prevent duplicate entries in the tooltip, couldn't the current calls to that method be replaced with one that merges only the multimaps (using ImmutableMultimap.Builder#putAll
), while handling duplicate entries when the tooltip is displayed?
#855 concerns mining levels. It isn't relevant to attribute modifiers at all from what I can get from the code.
IMO they should become two different modules then.
Also yes #855 will not affect this.
I mean, AttributeManager
solely exists to help deal with Fabric Tool Attribute API, so it doesn't make sense to spin it out. We're not NPM.
#855 might be helping with it? I've taken a step back from modding for the time being due to personal circumstance.
No, I mean that the mining level stuff is completely unrelated to tool attributes
uhhh, no? the mining level stuff was literally one of the primary points of tool attributes
The attributes mean the actual entity attributes, like attack speed and stuff. The mining speed and is effective is not really an attribute in my opinion.
Anyway, I completely went off-topic here, sorry about that. As stated, #855 won't fix this issue and this issue should be looked into.
I may look into this a bit later and write unit tests for the merging. No guarantees though
Closed by #1355.