Skip to content

Conversation

@TheGridExpert
Copy link

Added configuration screen for Farmer's Delight using NeoForge's new features.

Important side changes:

  1. Removed configs for wild crop generation as they weren't used anywhere and affected nothing. Hope I didn't miss anything.
  2. Removed comments above categories (e.g. above "world" category there was "World generation"). This was done because NeoForge turned them into tooltips under category tooltips on the screen. For example under the same "World generation" there would be another "World generation" but one line below. I've tested this in my own addon mod, and there seems to be no way to fix it other than to remove the comments.
  3. Removed the "Client settings" category because... it's already client, and in the menu it looks like you have "Client settings" inside of "Client settings". All three client configs are now located straight in the client config with no additional category.
  4. Added extra pain for translators, as all config now have two translation keys per config. Well, at least we now have translatable configurations /:
2024-08-27.22-50-24.mov

@vectorwing
Copy link
Owner

Hmm... This seems simple enough. Tested locally, and it seems good. 👍

Regarding the points:

  1. Thanks for reminding me about them; I'll remove them from 1.20.1 as well, as all settings are now present in the datapack.
  2. Funny that it would default to the comment there, but not in the description... But fair enough, we don't lose much removing them from the .toml itself.
  3. Also agree with this change, it turned out to be redundant.
  4. My biggest concern is the translation part, yeah. But after thinking a bit, this should still be fine. Any untranslated langs will default to en_us.json just like in vanilla, so it's fine.

The PR seems to have a conflict, but it looks like a false positive. If you can fix it real quick, I'll merge this and update the langs after.

@vectorwing
Copy link
Owner

Seems the conflict was a new config line I added long after this PR's creation, so I fixed it myself.
I'll review the PR this week and decide on the merge.

@TheGridExpert
Copy link
Author

Hello! Sorry for not answering the previous comment, I forgot this existed because of the time. Is this poll still considered to be merged?

@vectorwing
Copy link
Owner

Yeah, this seems good. I'm just deciding on it due to the large quantity of new langs it adds. I'll re-review this PR later down. 👍

Something I'm also considering is whether to introduce direct support for the mod Configured, which has a more organized config screen. But that is something I can add in post, if I want to.

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