Bernasss12 / BetterEnchantedBooks

Makes it easier to identify different enchantment books.
MIT License
12 stars 9 forks source link

Refactor config #41

Closed Fourmisain closed 3 years ago

Fourmisain commented 3 years ago

I based this on #40, so please look at that first. This is basically a continuation of #31.

Changelog

The data in mappedEnchantmentColors, mappedEnchantmentIndices was redundant to what's in enchantmentDataMap and mappedEnchantmentTargets wasn't being used.

This is probably the biggest part. try-with-resources ensures the reader/writer/Closeable is always being closed. Path is basically the "modern" File while Files (note the 's') has tons of useful one-liner methods like readString() that simplify a lot of common operations without sacrificing performance.

This was really confusing to read, I wouldn't be surprised if there was a bug somewhere in there. To "compensate" for this, a saveConfig() was put in the title screen mixin, which will generate the default config on first load (and also add new enchantments to the enchantment_data.json).

This is to make the prerequisites for loading more clear: Other mods need to have their enchantments registered before the title screen mixin (and thereby populateEnchantmentData()) is executed.

Wanted to do this back then, this makes reading and writing the enchantment data much easier. The only thing that one needs to be careful of when using Gson or other serialization methods is that you cannot assume that constructors are being called, not even default constructors. This is basically the reason why populateEnchantmentData() starts with putting data into EnchantmentData.

Map.of() (for up to 10 entries) or Map.ofEntries() are just a really nice way to define immutable maps. I also used List.of instead of Arrays.asList so that all values in ModConstants are immutable.

I'll probably add some more comments.