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

Update JEI config support #83

Merged
merged 10 commits into from
Mar 11, 2023
Merged

Conversation

mezz
Copy link
Contributor

@mezz mezz commented Jan 23, 2023

I added some methods to JEI's API so that Configured can get the config information without reflection.
This should make the compatibility much more stable, since JEI does not change the API much.

There are some regressions here, like Configured does not understand my record values.
I added some extra support for enums in lists, but I wasn't able to make it use a selector like with regular enum values.

Despite the drawbacks, I hope this is a good clean starting point at least.

Update: I have added full support for serializing and deserializing these values in the configs.

@MrCrayfish
Copy link
Owner

MrCrayfish commented Jan 23, 2023

Configured doesn't know how to convert records like ColorName into a String and vice versa. In my original implementation I added my own parser but that depended on the full codebase, since switching to API only of JEI this would no longer be possible. Anything that is not a primitive it won't understand, that's why IListType exists.

@mezz
Copy link
Contributor Author

mezz commented Jan 23, 2023

Yeah, that is a drawback of my implementation here.
I noted that in the PR description as the main regression from this PR.
I would like to think a bit more about how I could support that properly.

Here are some improvements that I think Configured could implement later to improve support for JEI's configs:

  • do not allow editing values that are unknown, instead offer a link to open the config file
  • improve editing of List<Enum>, so the player is provided an enum selection instead of a string entry.

@mezz
Copy link
Contributor Author

mezz commented Jan 24, 2023

I may be able to expose the string serialization and deserialization in JEI's API somehow.
I will try some things out and see how it looks.

@mezz
Copy link
Contributor Author

mezz commented Jan 24, 2023

I have updated this PR, and now it uses new features of JEI's API to implement IListType for List<ColorName>. 🎉

@mezz
Copy link
Contributor Author

mezz commented Feb 28, 2023

I have updated the PR to use the latest JEI version, which exposes configs much earlier.
This way, players can access all the JEI configs before entering a world.

@MrCrayfish
Copy link
Owner

Everything checks out. Merging now. Thanks for your work into making these mods compatible again.

@MrCrayfish MrCrayfish merged commit 006febc into MrCrayfish:1.19.X Mar 11, 2023
MrCrayfish added a commit that referenced this pull request Mar 11, 2023
@mezz mezz deleted the new-jei-config branch March 11, 2023 21:26
@MrCrayfish
Copy link
Owner

Hey @mezz, I am currently relicensing Configured. I want to change the mod to be under a more appropriate license of LGPLv3.
I want to get your permission to change the license of your contributions under this new license. Thank you.

@mezz
Copy link
Contributor Author

mezz commented Jan 30, 2024

Hello! You have my permission to relicense my changes to Configured under LGPLv3 👍

@MrCrayfish
Copy link
Owner

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants