MerchantPug / apugli

An extension to Apoli for certain mods' use.
MIT License
5 stars 4 forks source link

NPE from enchantment helper mixin #45

Closed Shadows-of-Fire closed 1 year ago

Shadows-of-Fire commented 1 year ago

title - more information can be found in the original report that was directed at me https://github.com/Shadows-of-Fire/Apotheosis/issues/943

Aside from the NPE issue, you should never be modifying the enchantment levels returned from getTagEnchantmentLevel, it is explicitly designed to return the level as written in NBT.

If you need to mess with levels, you need to target IForgeItemStack#getEnchantmentLevel

MerchantPug commented 1 year ago

Ah, I think I see the issue, I've targeted the incorrect method in that mixin.

The reason why this mixin exists is because my modifications are supposed to modify empty ItemStacks as well, which makes the check a thing that I need to touch. (It's a "power" that allows for modifications to an entity's enchantment level.)

I do use a coremod into IForgeItemStack#getEnchantmentLevel for the actual modification part.

Thanks for telling me about this, because now I know to avoid touching getTagEnchantmentLevel, which I wasn't doing outside of this single oversight.

MerchantPug commented 1 year ago

I'll remove the offending mixin, see if I even have to move it to where I meant to put it and go from there.

This offending mixin was able to be removed, but I'll take a look at my other mixins and readjust them to minimise the chance of something like this happening in the future.