Wynntils

Wynntils

611k Downloads

Config exception might reset config

kristofbolyai opened this issue · 7 comments

commented

While this is not something that would ever make it into main, I would like to fix this, so in the rare case it does, it doesn't cause mayhem.

[23:19:53] [Render thread/ERROR] (wynntils) Failed to initialize Wynntils features
 java.lang.RuntimeException: A Config is missing @RegisterConfig annotation:private final com.wynntils.core.config.Config com.wynntils.features.MythicFoundFeature.playSound
	at com.wynntils.core.config.ConfigManager.getConfigOptions(ConfigManager.java:248) ~[main/:?]
	at com.wynntils.core.config.ConfigManager.registerConfigOptions(ConfigManager.java:92) ~[main/:?]
	at com.wynntils.core.config.ConfigManager.registerFeature(ConfigManager.java:80) ~[main/:?]
commented

Yes.

commented

Ah, you mean if the config is incorrectly annotated, the user value gets permanently lost? That is bad. :( Having the check for the annotation is good, though. (I think we agree on this)

commented

Ah, you mean if the config is incorrectly annotated, the user value gets permanently lost?
I am not sure we are on the same understanding so I will just state this here to make it obvious. Whenever there is an not annotated config, the whole user config get's reset. Even in production.

commented

@magicus

In #1926 you added a verifyAnnotations function which from what I can presume solves this issue. Can this be closed? Or atleast mentioned that it already has a step made to solving it?

Thank you ❤️

commented

No, it doesn't. It solves this cause, but the system is the same.

commented

The concern from the looks of it is that if something is incorrectly annotated, it will destroy the entire user config. Which shouldn't happen in production with this check because checks and tests should immediately bring up the error. Say the user doesn't, shouldn't the automated ones still catch it? I don't see this as an issue that can actually destroy any production installation

commented

This is going to be fixed in time. I'm working on rewriting the config system step by step. I still think this can happen, but I will not investigate until the rewrite is done, since I expect it will resolve itself during the rewrite. So let's keep the bug open to remind me to check for this case when I'm done.