Bernasss12 / BetterEnchantedBooks

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

Fix some config regressions #44

Closed Fourmisain closed 3 years ago

Fourmisain commented 3 years ago

First of all, welcome back!

Second of all, my last PR wasn't well tested, while looking at it just now I found two issues:

edit: Also bumped the version to 1.2.11

Bernasss12 commented 3 years ago

Hey there! Thanks a lot for all the work you've put into this! I've been away and a lot changed since I last programmed so I'm having still a bit of trouble with my setup but I'm working through it. To get back to programming though I was thinking of rewriting the mod from the ground up, probably taking into account all the changes you made since I did learn a lot from all your PRs as well as try to actually add some features that were previously requested as well as some I had planned for a while. For now of course I will keep merging what you work on as well as try to fix any problems that might come up.

Again thanks for keeping this mod alive and releasing the 1.17 version while I was gone

Fourmisain commented 3 years ago

Sounds good!

Though I'm not sure a full rewrite is needed. I think the main thing which desperately needs a rewrite is all "current item" and tooltip icon/spacers stuff.

Currently, for tooltip icons, the mod adds an empty line and separately tries to render the icons on the correct position - except they're not always correct e.g. when there's more tooltips from other mods. It (generally) also tries to remember the "current item" being looked at and everything else depends on this, which is the main reason why the mod won't work with REI at all, since REI simply calls ItemStack.getTooltip (which internally calls Item.appendTooltip and ItemStack.appendEnchantments) on a list of items, on some REI internal worker threads even.

I think it should be possible to fix all this by

  1. sorting enchantments in ItemStack.appendEnchantments (just like already done here except removing all the other stuff).
  2. adding the tooltip icons as a some special kind of Text (by injecting into EnchantedBookItem.appendTooltip)
  3. modifying the tooltip or text renderer to render those Texts as tooltip icons

The hard part being 3.

To be a little bit more concrete about 2, you could e.g. add a LiteralText of two chars where the first is just '\0', so testing if a Text represents tooltip icons is just testing the first character, and the second char could be used as a bitset, so you could have up to 32 different tooltip icons. Alternatively you could add a new class implementing Text, test it using instanceof and read the icons to render from that instance (which has the advantage of allowing any representation you want).

Bernasss12 commented 3 years ago

As far as I can see I could probably implement TooltipComponent on the newer version.