caffeine-meta icon indicating copy to clipboard operation
caffeine-meta copied to clipboard

Move our Mixin configuration system for each mod into a common module

Open jellysquid3 opened this issue 4 years ago • 6 comments
trafficstars

Right now, our mods (Sodium, Lithium, and Phosphor) all implement more or less the same system for managing Mixin configurations. Most of the work on this gets done in Lithium and is then manually copied into the other projects. This tends to be error prone and means our implementations are subtly different between each.

An example of this Mixin plugin in Lithium can be found here, and the configuration system can be found here.

Moving this code into a Maven project that each one of our mods can instead pull in would be a big win for hygiene. We've also seen community interest here and there for a public library, so fixing this also opens that door up.

jellysquid3 avatar Aug 06 '21 16:08 jellysquid3

Depends on #3.

jellysquid3 avatar Aug 06 '21 18:08 jellysquid3

Hey!

I've worked on this and extracted (and modified to work as an external library) the config system in the last few days, and pushed the code to https://github.com/altrisi/CaffeineConfig.

A summary of what I've changed (other than the obvious of not being tied to any specific mod):

  • Changed options to be added in a builder instead of in a private constructor This was basically needed because you can't really make it a consumable library by declaring the options in the constructor.
  • Changed it to use NIO It no longer uses File, all is handled via Path and the rest of NIO.
  • Added docs I've documented how to use the library in the repo's readme, only missing the maven since it's not on any maven atm
  • Created a testmod Not extremely useful since I didn't know how to make it fail the regular build when testmod fails to build, but it can be launched in the IDE and gradle and has some values in the mod json for disabling options with both correct, incorrect and dependent values
  • Renamed everything to option Previously there was a mix between "option" and "rule" naming, with both having the same meaning. I changed them to all be "option"
  • Made it into a JiJ-able fabric mod Since the library is going to be used in more than one mod (at least in Lithium and Sodium), that's the recommended thing to do according to the old JiJ diagram in the fabric wiki
  • Made the game crash if if a mixin in the config is not assigned to any option IMO it's better than just not enabling them, makes sure every mixin can be configured and prevents finding functionality not working because it was almost silently (it's printed to the log, yes) disabled
  • Made wiki URL optional Given Sodium for example doesn't have a configuration page on the wiki, I made that optional, and the library will skip the paragraph if that's not given

I also have branches of Lithium and Sodium moved to use the library that I'll probably open as draft PRs later.

All that's missing is anything you'd like me to change, and then I can transfer the repo I think, and you should be able to publish it into maven and start using it (I tested both Sodium and Lithium via mavenLocal, I have NOT prepared the build to get published to a maven, since I don't know how to do that).

For anything feel free to reply here or @ me on discord, same name. Or close the PRs if you don't want them, also ok.

Edit: Also it doesn't have an icon since I'm unable to make those things, neither the name is very original.

altrisi avatar Nov 07 '21 13:11 altrisi

Just some more ideas (not sure whether they would be used):

Allow / Force a description string when defining an option. Allow writing all options to a file (maybe even config file as comments) including the description (could use it for faster wiki page updating). Allow non boolean options.

These ideas are partially inspired by carpet mod

2No2Name avatar Dec 07 '21 22:12 2No2Name

@altrisi Hey, sorry for the tremendously late response. I'm super thankful for your work here and it looks like a great launch pad. In the next few days I'll be able to more extensively review things here, and we can discuss potentially incorporating your work into a new repository. :slightly_smiling_face: (if you're happy with it)

jellysquid3 avatar Dec 14 '21 16:12 jellysquid3

@2No2Name I think as far as improvements go, we should focus on getting a new repository / maven artifacts set up prior to making any more changes, so we can move this code out of each of our mods. It seems Altrisi has covered all that important work in their repository already (i.e. JiJ and some generalization.)

jellysquid3 avatar Dec 14 '21 16:12 jellysquid3