Skip to content

Conversation

@FireInstall
Copy link
Contributor

Minedown instead of MiniMessage

@FireInstall FireInstall force-pushed the upstreamComponents branch from 9a596a5 to f1f7f18 Compare May 6, 2024 00:11

public void loadConfig() {
saveDefaultConfig();
getConfig().options().copyDefaults(true); // save newly added config options to disk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually do anything on its own? I would assume this needs the config to be saved afterwards. (And you wouldn't want that on loading as that would overwrite existing values) Also with the removed saveDefaultConfig I would assume that it doesn't initially save the config at all.

I don't think it really matters that much that the keys are in the config though. If they are missing they will be pulled in from the file in the jar and if someone with a config which is missing them wants to edit it they can just get the ones they want from the default file in the jar or on github.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well yes, it was missing the call to saveConfig(). That was the point where I lacked enough concentration to go to sleep.

However, in my opinion it is pretty inconvenient and confusing, for the config file to miss newly added stuff, when it is really no effort to save defaults.

Copy link
Member

@Phoenix616 Phoenix616 May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will now cause your local file changes to be overwritten when running /syncinv reload as that calls loadConfig too. The only way to avoid that while still writing the new values into the config would be to first save the default config, then reload the config and then save the defaults into it again. (Or do the defaults saving in startup outside the loadConfig)

Copy link
Contributor Author

@FireInstall FireInstall May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just use the config in jar, if we don't overwrite the defaults in getLang() (would be also a problem if we would not save new config keys to disk).
In this case we just have to reload and save.

@FireInstall FireInstall force-pushed the upstreamComponents branch 4 times, most recently from 51fbb14 to 6cb84ae Compare May 8, 2024 14:48
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