Skip to content

Add setSettingName and setSettingDescription methods to Mod #1246

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TheBearodactyl
Copy link
Contributor

v2 bc neovim fucked with formatting last time (used notepad++ this time)

@HJfod
Copy link
Member

HJfod commented May 31, 2025

Why does this add methods to Mod directly? What's the use case for this? Does it warrant having a quick-access method in Mod?

@TheBearodactyl
Copy link
Contributor Author

updated my fork

@TheBearodactyl
Copy link
Contributor Author

TheBearodactyl commented Jun 1, 2025

Why does this add methods to Mod directly? What's the use case for this? Does it warrant having a quick-access method in Mod?

well for one they're companion methods to the get methods so it'd be kinda weird NOT to have them directly in Mod. like imagine if a class had a getter for foo and the setter was in a completely different class for no reason. it'd just be unnecessarily complicated opposed to having both the getter and setter in the same class.
and the reason they exist is to make customizing setting menus easier (e.g. if someone makes a localization mod and they want to add support for setting menus and not just the base game)

@HJfod
Copy link
Member

HJfod commented Jun 1, 2025

If this is just for localization, I'd rather add a dedicated localization API that'd let people provide a JSON file or something with translations listed. Allowing changing setting names and descriptions arbitarily at runtime seems like a bad move that'll confuse users when suddenly their setting has a different name.

If we do add this, I will say there should not be setters in Mod. Yes the getters are there, but the getters are meant to be used much much more frequently than a setter would be, and for the usecases provided I think people can write two more lines of code to set the value. Having it be in Mod directly both bloats its interface and makes it more encouraging to misuse.

@TheBearodactyl
Copy link
Contributor Author

fair enough. still gonna keep this open though until said localization api is a thing though

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.

4 participants