Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jellysquid3 opened this issue Aug 6, 2021 · 5 comments
Labels
area: mod code Issue is about mod development good first issue Good for newcomers

Comments

@jellysquid3
Copy link
Member

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 jellysquid3 added area: mod code Issue is about mod development good first issue Good for newcomers labels Aug 6, 2021
@jellysquid3
Copy link
Member Author

Depends on #3.

@altrisi
Copy link

altrisi commented Nov 7, 2021

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.

@2No2Name
Copy link
Member

2No2Name commented Dec 7, 2021

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

@jellysquid3
Copy link
Member Author

@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. 🙂 (if you're happy with it)

@jellysquid3
Copy link
Member Author

@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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: mod code Issue is about mod development good first issue Good for newcomers
Development

No branches or pull requests

3 participants