Wynntils

Wynntils

611k Downloads

Features do not respect the UserFeature superclass `userEnabled` field

kristofbolyai opened this issue ยท 7 comments

commented

The userEnabled field should act a super on-off switch to the feature, so if userEnabled is false, all parts of the feature should be turned off, even if other configs explicitly enable a specific thing (for example hotbarHighlightEnabled in ItemHighlightFeature).

commented

Should we unregister the feature as a listener for events when it is turned off?

commented

Should we unregister the feature as a listener for events when it is turned off?

I think we used to, at least in my head :). Unregistering greatly reduces the overhead of the mod and improves the performance of the event bus.

commented

We still do:

public final void disable() {
        if (!enabled) throw new IllegalStateException("Feature can not be disabled as it already is disabled");

        onDisable();

        enabled = false;

        if (isListener) {
            WynntilsMod.getEventBus().unregister(this);
        }
        for (KeyHolder key : keyMappings) {
            KeyManager.unregisterKeybind(key);
        }
    }
commented

Regarding the userEnabled functionality - I plan on tackling this next

commented

Then the issue is that it is not getting respected. See ItemHighlightFeature which, I tested, and did not work correctly. Or was it the config command not updating the state? If so, we need something to force a config save and feature state reload, which is missing from #112

commented

@P0keDev Oh, then can I assume that you will solve the issues I mentioned above, and not worry about them in the config command PR?

commented

fixed in #114