StrikerRockers-Mods / VanillaTweaks

A Vanilla enhancing mod without going overboard
https://minecraft.curseforge.com/projects/vanillatweaks/files
MIT License
41 stars 15 forks source link

[1.18.2] Enchantments reporting their max levels as 0 #164

Closed 0xJoeMama closed 2 years ago

0xJoeMama commented 2 years ago

Hello there. It has come to my attention that you are using max level 0, in order to disable enchantments. I would like to ask whether it is a common practice or something that you thought of. That is, because unsuspecting mods may use the max level of an enchantment for something other than figuring out whether a book can be updated.

This fact came to my attention after realising Lllamarama has been getting a lot of crashing issues. After looking into the matter, I realised it's only vanilla tweaks reporting invalid enchantment levels.

We are planning on fixing the issue on our side, however I thought it would be important to let you know. I hope we can achieve a better solution wherever possible together.

The discussion about the aforementioned case can be found here: https://github.com/LlamaramaTeam/Llamarama/issues/15

Please let us know about the way you feel about it. We are glad to collaborate with you. Thank you for your time!

strikerrocker commented 2 years ago

Hi, sorry to hear that my code may be the culprit causing your code to crash. I use max lvl 0 to disable enchantments instead of not registering them due to forge's policy on deregistering/not registering stuff after config load. Another factor is registry changes by mojang in 1.18.2. Also to my knowledge this is the first time its breaking a mod.

As I use multi-loader I try my best to make code common enough for both loaders. If you can think of any other solution other than using 0 max lvl which would be compatible with forge and fabric Let me know.

strikerrocker commented 2 years ago

My discord server link if you want to talk further. https://discord.gg/tqcqeSj

0xJoeMama commented 2 years ago

I use max lvl 0 to disable enchantments instead of not registering them due to forge's policy on deregistering/not registering stuff after config load.

Yeah I figured you'd have trouble doing that in another way. I will handle 0 as a max level as a common practice and just filter out those mods when parsing the registry, since I don't see any other reason that would happen. This will also prevent any issues when creating invalid enchantment books from disabled enchantments.. I feel like that will help prevent similar issues in the future.