maruohon / malilib

Library mod for masa's client-side Minecraft mods
GNU Lesser General Public License v3.0
294 stars 125 forks source link

Mod Configs only loaded after mod initialisation #25

Closed s-maurice closed 2 years ago

s-maurice commented 4 years ago

Tested on fabric_1.16_snapshots_temp branch in tweakeroo but seems to be issue on all branches.

Mods are loaded with the default config values before the configs are loaded.

https://github.com/maruohon/malilib/blob/fb3d667409aa6aca258c7510b56ea04497b28c0b/src/main/java/fi/dy/masa/malilib/event/InitializationHandler.java#L32-L44

This causes mod features that only check when toggled on or off, aka using IValueChangeCallback to not have their settings properly loaded.

s-maurice commented 4 years ago

Is there a specific reason why configs aren't loaded directly with the mods?

maruohon commented 2 years ago

I don't remember if this was discussed elsewhere in more detail, so I'll leave a note here about how these around going to work in the new code base (after the rewrites that have been going on in 1.12.2).

So first of all, addressing the issue title: Mod Configs only loaded after mod initialisation:

It's obviously not possible for malilib to load anything before the mods have had a chance to register their handlers, which is meant to happen from the malilib InitializationHandler#registerModHandlers() method. So the configs will get loaded after all the mod handlers have been registered.

As for some features not getting the callback to update to the values loaded from the config: There is a separate ValueLoadCallback<T> class with the void onValueLoaded(T newValue); method, that will get called with the new value after configs are de-serialized and loaded (usually from a config file). I don't remember how long this callback has existed in some form, and if it existed when this issue was first created. But the way these are intended to work, is that the void onValueChanged(T newValue, T oldValue); method from the ValueChangeCallback<T> class only gets called when the value changes after it has been loaded from the config. So usually this change would be the user changing the config manually via the config screen, or using the RESET button. It will also be called if a config value override is applied, and the overridden value is different from the previous value.

Now there is one case I haven't yet properly thought through: switching config profiles (which is also a thing in the new code). From the point of view of a "feature" in a mod, that needs to be notified of a config change, this would be more equivalent to a config change than a config load. But I'm pretty sure the way the code works, it would call the onValueLoaded() callback, since the config profile change will first save the current configs, and then load them from a different directory, so it's equivalent to a "normal" config load in that sense.

So I'm not sure if I should rewrite some stuff there to cause the onValueChanged() callback to get called instead during a config profile change... There is also the question of should these even be different? The thing I mentioned in an earlier comment about config change callbacks not wanting to be called "too early" is kinda "hmmmm, is that actually an issue?". As of right now it's quite common at least in my mods that the same config will have both callbacks set to the same method, but via a lambda wrapper since the arguments are different (newValue and oldValue vs. just newValue). So maybe I should get rid of the separate value load callback and just use the value change callback also for config loading.