Fixes, fixes everywhere

Bernasss12 / BetterEnchantedBooks

Makes it easier to identify different enchantment books.

MIT License

12 stars 9 forks source link

Fixes, fixes everywhere #31

Closed Fourmisain closed 3 years ago

Fourmisain commented 3 years ago

Changelog

fix 2 NullPointerExceptions when items have unregistered enchantments

Items can have enchantments on them which are not registered. For testing you can give yourself such a book: /give @s enchanted_book{StoredEnchantments:[{id:nonexistent,lvl:3}]} 1 Tooltip sorting would crash (so much for my "sort should never throw an Exception"), as would rendering tooltip icons.

fix sorting by translated name

I thought I broke it, but it was already broken in 1.2.5

fix caching of default colored books

This I broke, caching is very important for performance (though default colored enchanted books are rare)

fix some books being default colored because they use id "id" instead of "minecraft:id"
add cloth-config2 as a 'depends'

Could be included in the build, though that would add 800kB.

enable pretty-printing of config
bump version to 1.2.7

I'll add some code comments soon.

Fourmisain commented 3 years ago

I had a bunch more changes for all the config stuff, since it can be simplified by a lot, but I stopped myself to not overblow the PR again. Maybe I'll continue it another day.

Bernasss12 commented 3 years ago

I've looked through it all it's looking good to merge, do you have anything else to add?

Fourmisain commented 3 years ago

Nope! I hope I didn't mess anything up again (like the caching), but I think I did decent enough testing.

Bernasss12 commented 3 years ago

I had a bunch more changes for all the config stuff, since it can be simplified by a lot, but I stopped myself to not overblow the PR again. Maybe I'll continue it another day.

Yeah the configs grew as stuff was added and I never really went back to look at what makes sense and what doesn't... My original plan was to make it so you can reorder the enchantments in the list to make the custom order that is at the moment random (or can only be defined by hand) but it turns out the way cloth config was designed it would require me to make a bunch of lower level elements from scratch to make it work so I "postponed the idea indefinitely".

Bernasss12 commented 3 years ago

Nope! I hope I didn't mess anything up again (like the caching), but I think I did decent enough testing.

If something big brakes I will fix it as soon as it is reported, I still "maintain" the mod just not enough as to rewrite it all :)

Bernasss12 commented 3 years ago

So with that said I will just merge it and ship it to Modrinth and Curseforge.