fix: Track known keys as they are loaded, as setKey is not called when it matches the original default

Author: BlayTheNinth

common/src/main/java/net/blay09/mods/defaultoptions/DefaultOptionsInitializer.java

@@ -2,7 +2,10 @@
 
 import net.blay09.mods.balm.api.event.client.ClientStartedEvent;
 import net.blay09.mods.defaultoptions.api.*;
+import net.minecraft.client.Options;
+import net.minecraft.nbt.CompoundTag;
 
+import java.util.HashSet;
 import java.util.ServiceLoader;
 
 public class DefaultOptionsInitializer {
@@ -24,12 +27,18 @@ public static void postLoad(ClientStartedEvent event) {
 
     private static void loadDefaults(DefaultOptionsLoadStage stage) {
         for (DefaultOptionsHandler handler : DefaultOptions.getDefaultOptionsHandlers()) {
-            if (handler.shouldLoadDefaults() && handler.getLoadStage() == stage) {
-                try {
-                    handler.loadDefaults();
-                    DefaultOptions.logger.info("Loaded default options for {}", handler.getId());
-                } catch (DefaultOptionsHandlerException e) {
-                    DefaultOptions.logger.error("Failed to load default options for {}", e.getHandlerId(), e);
+            if (handler.getLoadStage() == stage) {
+                if (handler.shouldLoadDefaults()) {
+                    try {
+                        handler.loadDefaults();
+                        DefaultOptions.logger.info("Loaded default options for {}", handler.getId());
+                    } catch (DefaultOptionsHandlerException e) {
+                        DefaultOptions.logger.error("Failed to load default options for {}", e.getHandlerId(), e);
+                    }
+                } else if (handler.hasDefaults()) {
+                    DefaultOptions.logger.debug("Skipping default options for {}; defaults are present but should not be loaded", handler.getId());
+                } else {
+                    DefaultOptions.logger.debug("Skipping default options for {}; no defaults available", handler.getId());
                 }
             }
         }
@@ -40,4 +49,19 @@ public static void postSave() {
             handler.saveAdditional();
         }
     }
+
+    public static void detectAndMarkModifiedKeys(Options options, CompoundTag fields) {
+        final var knownKeys = new HashSet<String>();
+        for (final var option : fields.getAllKeys()) {
+            if (option.startsWith("key_")) {
+                final var name = option.substring("key_".length());
+                knownKeys.add(name);
+            }
+        }
+        for (final var keyMapping : options.keyMappings) {
+            if (knownKeys.contains(keyMapping.getName())) {
+                ((DefaultOptionsKeyMapping) keyMapping).defaultoptions$setUserModified(true);
+            }
+        }
+    }
 }

common/src/main/java/net/blay09/mods/defaultoptions/DefaultOptionsKeyMapping.java

@@ -2,4 +2,5 @@
 
 public interface DefaultOptionsKeyMapping {
     boolean defaultoptions$wasUserModified();
+    void defaultoptions$setUserModified(boolean modified);
 }

common/src/main/java/net/blay09/mods/defaultoptions/mixin/KeyMappingMixin.java

@@ -15,11 +15,18 @@ public class KeyMappingMixin implements DefaultOptionsKeyMapping {
 
     @Inject(method = "setKey", at = @At("HEAD"))
     void setKey(InputConstants.Key key, CallbackInfo ci) {
+        // setKey is only called when the key didn't match the default on options load, so it's not reliable.
+        // We just track it additionally to cover all bases.
         defaultoptions$userModified = true;
     }
 
     @Override
     public boolean defaultoptions$wasUserModified() {
         return defaultoptions$userModified;
     }
+
+    @Override
+    public void defaultoptions$setUserModified(boolean modified) {
+        defaultoptions$userModified = modified;
+    }
 }

common/src/main/java/net/blay09/mods/defaultoptions/mixin/OptionsMixin.java

@@ -18,4 +18,5 @@ private void load(CallbackInfo ci) {
     private void save(CallbackInfo ci) {
         DefaultOptionsInitializer.postSave();
     }
+
 }

fabric/src/main/java/net/blay09/mods/defaultoptions/fabric/mixin/FabricOptionsMixin.java

@@ -0,0 +1,21 @@
+package net.blay09.mods.defaultoptions.fabric.mixin;
+
+import net.blay09.mods.defaultoptions.DefaultOptionsInitializer;
+import net.minecraft.client.Options;
+import net.minecraft.nbt.CompoundTag;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.ModifyArg;
+
+@Mixin(Options.class)
+public class FabricOptionsMixin {
+    @ModifyArg(method = "load()V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/Options;dataFix(Lnet/minecraft/nbt/CompoundTag;)Lnet/minecraft/nbt/CompoundTag;"))
+    private CompoundTag processPreDataFixedOptions(CompoundTag fields) {
+        // This will operate on raw options data before data fixers are applied to it, but I think that's fine
+        // considering key mapping options have been stable for a long time and even if they ever change,
+        // it would be extremely rare to happen in a modpack environment.
+        DefaultOptionsInitializer.detectAndMarkModifiedKeys((Options) (Object) this, fields);
+        return fields;
+    }
+
+}

fabric/src/main/resources/defaultoptions.fabric.mixins.json

@@ -7,6 +7,7 @@
   "mixins": [
   ],
   "client": [
+    "FabricOptionsMixin"
   ],
   "injectors": {
     "defaultRequire": 1

forge/src/main/java/net/blay09/mods/defaultoptions/forge/mixin/ForgeKeyMappingMixin.java

@@ -0,0 +1,22 @@
+package net.blay09.mods.defaultoptions.forge.mixin;
+
+import com.mojang.blaze3d.platform.InputConstants;
+import net.blay09.mods.defaultoptions.DefaultOptionsKeyMapping;
+import net.minecraft.client.KeyMapping;
+import net.minecraftforge.client.settings.KeyModifier;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+@Mixin(KeyMapping.class)
+public class ForgeKeyMappingMixin {
+
+    @Inject(method = "setKeyModifierAndCode", at = @At("HEAD"), remap = false)
+    void setKeyModifierAndCode(KeyModifier keyModifier, InputConstants.Key keyCode, CallbackInfo ci) {
+        // setKey is only called when the key didn't match the default on options load, so it's not reliable.
+        // We just track it additionally to cover all bases.
+        ((DefaultOptionsKeyMapping) this).defaultoptions$setUserModified(true);
+    }
+
+}

forge/src/main/java/net/blay09/mods/defaultoptions/forge/mixin/ForgeOptionsMixin.java

@@ -0,0 +1,21 @@
+package net.blay09.mods.defaultoptions.mixin;
+
+import net.blay09.mods.defaultoptions.DefaultOptionsInitializer;
+import net.minecraft.client.Options;
+import net.minecraft.nbt.CompoundTag;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.ModifyArg;
+
+@Mixin(Options.class)
+public class ForgeOptionsMixin {
+    @ModifyArg(method = "load(Z)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/Options;dataFix(Lnet/minecraft/nbt/CompoundTag;)Lnet/minecraft/nbt/CompoundTag;"))
+    private CompoundTag processPreDataFixedOptions(CompoundTag fields) {
+        // This will operate on raw options data before data fixers are applied to it, but I think that's fine
+        // considering key mapping options have been stable for a long time and even if they ever change,
+        // it would be extremely rare to happen in a modpack environment.
+        DefaultOptionsInitializer.detectAndMarkModifiedKeys((Options) (Object) this, fields);
+        return fields;
+    }
+
+}

forge/src/main/resources/defaultoptions.forge.mixins.json

@@ -7,7 +7,9 @@
   "mixins": [
   ],
   "client": [
-    "ForgeKeyMappingAccessor"
+    "ForgeOptionsMixin",
+    "ForgeKeyMappingAccessor",
+    "ForgeKeyMappingMixin"
   ],
   "injectors": {
     "defaultRequire": 1

neoforge/src/main/java/net/blay09/mods/defaultoptions/neoforge/mixin/NeoForgeKeyMappingMixin.java

@@ -0,0 +1,22 @@
+package net.blay09.mods.defaultoptions.neoforge.mixin;
+
+import com.mojang.blaze3d.platform.InputConstants;
+import net.blay09.mods.defaultoptions.DefaultOptionsKeyMapping;
+import net.minecraft.client.KeyMapping;
+import net.neoforged.neoforge.client.settings.KeyModifier;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Inject;
+import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
+
+@Mixin(KeyMapping.class)
+public class NeoForgeKeyMappingMixin {
+
+    @Inject(method = "setKeyModifierAndCode", at = @At("HEAD"), remap = false)
+    void setKeyModifierAndCode(KeyModifier keyModifier, InputConstants.Key keyCode, CallbackInfo ci) {
+        // setKey is only called when the key didn't match the default on options load, so it's not reliable.
+        // We just track it additionally to cover all bases.
+        ((DefaultOptionsKeyMapping) this).defaultoptions$setUserModified(true);
+    }
+
+}