feat: redesign Dynamic

build.gradle.kts

@@ -22,7 +22,7 @@ repositories {
 }
 
 val fabricVersion = "0.87.0+1.20.1"
-val muscriptVersion = "1.3-SNAPSHOT"
+val muscriptVersion = "1.4-SNAPSHOT"
 jfMod {
     minecraftVersion = "1.20.1"
     yarn("build.10")

docs/additional/Migrations.md

@@ -67,9 +67,14 @@ Corresponds to version 3.0.0-3.1.0
 - Removal of fabric conditions API interop in favor of enabling cleaner syntax
 
 ## v9
-Corresponds to version 4.0.0
+Corresponds to version 4.0.0-4.3.1
 
 - New config screen backend powered by LibJF (needed to support serverside)
 - New translation key syntax
 - Removal of manual configuration for sliders vs input boxes
-- Support for respackopts configs in the pack root (`/respackopts.json5` instead of `/assets/respackopts/conf.json`)
+- Support for respackopts configs in the pack root (`/respackopts.json5` instead of `/assets/respackopts/conf.json`)
+
+## v10
+Corresponds to version 4.4.0
+
+- Stricter enforcement of legal entry names: instead of sanitization, unsupported names are logged and ignored

run/resourcepacks/testpack/respackopts.json5

@@ -1,6 +1,6 @@
 {
   id: "lumi",
-  version: 8,
+  version: 10,
   capabilities: ["FileFilter", "DirFilter", "DirFilterAdditive"],
   conf: {
     tonemap: [

src/client/java/io/gitlab/jfronny/respackopts/RespackoptsClient.java

@@ -43,7 +43,7 @@ public class RespackoptsClient implements ClientModInitializer, SaveHook {
         StringBuilder sb = new StringBuilder();
         sb.append("#ifndef respackopts_loaded");
         sb.append("\n#define respackopts_loaded");
-        MetaCache.forEach((key, state) -> state.configBranch().buildShader(sb, Respackopts.sanitizeString(state.packId())));
+        MetaCache.forEach((key, state) -> state.configBranch().buildShader(sb, state.packId()));
         sb.append("\n#endif");
         RespackoptsClient.shaderImportSource = sb.toString();
         if (FREX_LOADED) {

src/main/java/io/gitlab/jfronny/respackopts/Respackopts.java

@@ -26,9 +26,10 @@ import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.concurrent.CompletableFuture;
+import java.util.regex.Pattern;
 
 public class Respackopts implements ModInitializer, SaveHook {
-    public static final Integer META_VERSION = 9;
+    public static final Integer META_VERSION = 10;
     public static final String FILE_EXTENSION = ".rpo";
     public static final Gson GSON = new GsonBuilder()
             .registerTypeAdapter(ConfigEnumEntry.class, new EnumEntrySerializer())
@@ -93,15 +94,20 @@ public class Respackopts implements ModInitializer, SaveHook {
         return CompletableFuture.completedFuture(null);
     }
 
+    // ^ = start of string
+    // $ = end of string
+    // * = zero or more times
+    // [\\s_] = whitespace or underscores
+    // | = or
+    // [^a-zA-Z_] = not character or underscore
+    private static final Pattern UNSUPPORTED = Pattern.compile("[^a-zA-Z_]|^[\\s_]*|[\\s_]*$");
+
     public static String sanitizeString(String s) {
         // This trims whitespace/underscores and removes non-alphabetical or underscore characters
+        return UNSUPPORTED.matcher(s).replaceAll("");
+    }
 
-        // ^ = start of string
-        // $ = end of string
-        // * = zero or more times
-        // [\\s_] = whitespace or underscores
-        // | = or
-        // [^a-zA-Z_] = not character or underscore
-        return s.replaceAll("[^a-zA-Z_]|^[\\s_]*|[\\s_]*$", "");
+    public static boolean isLegal(String s) {
+        return !UNSUPPORTED.matcher(s).find();
     }
 }

src/main/java/io/gitlab/jfronny/respackopts/filters/util/FileExpansionProvider.java

@@ -10,7 +10,7 @@ import java.io.*;
 import java.util.Map;
 
 public class FileExpansionProvider {
-    public static synchronized InputStream replace(Dynamic<?> parameter, InputStream is, Map<String, StringExpr> expansions) throws IOException {
+    public static synchronized InputStream replace(Dynamic parameter, InputStream is, Map<String, StringExpr> expansions) throws IOException {
         String s = new String(is.readAllBytes());
         for (Map.Entry<String, StringExpr> entry : expansions.entrySet()) {
             s = s.replace("${" + entry.getKey() + "}", entry.getValue().get(parameter));

src/main/java/io/gitlab/jfronny/respackopts/gson/entry/EnumEntrySerializer.java

@@ -5,6 +5,8 @@ import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.model.tree.ConfigEnumEntry;
 
 import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.List;
 
 public class EnumEntrySerializer implements JsonSerializer<ConfigEnumEntry>, JsonDeserializer<ConfigEnumEntry> {
     @Override
@@ -29,18 +31,19 @@ public class EnumEntrySerializer implements JsonSerializer<ConfigEnumEntry>, Jso
                 throw new JsonSyntaxException("Expected primitive string key for enum");
         }
         else if (json.isJsonArray()) {
-            result.values.clear();
+            List<String> replacement = new ArrayList<>();
             for (JsonElement e : json.getAsJsonArray()) {
                 if (e.isJsonPrimitive() && e.getAsJsonPrimitive().isString()) {
-                    result.values.add(e.getAsString());
+                    replacement.add(e.getAsString());
                 }
                 else
                     throw new JsonSyntaxException("Expected string entry in enum");
             }
-            if (result.values.isEmpty())
+            result.setValues(replacement);
+            if (replacement.isEmpty())
                 Respackopts.LOGGER.warn("Enum entry empty");
             else
-                result.setDefault(result.values.get(0));
+                result.setDefault(replacement.get(0));
             return result;
         }
         else if (json.isJsonObject()) {

src/main/java/io/gitlab/jfronny/respackopts/mixin/ResourcePackManagerMixin.java

@@ -84,6 +84,12 @@ public class ResourcePackManagerMixin {
     private static String rpo$readConfiguration(InputStream is, Path dataLocation, String packName, String displayName, Set<Path> dataLocations, Set<Path> toRemove) throws IOException {
         try (InputStreamReader isr = new InputStreamReader(is)) {
             PackMeta conf = Respackopts.GSON.fromJson(isr, PackMeta.class);
+            if (!Respackopts.isLegal(conf.id)) {
+                if (conf.version >= 10) {
+                    Respackopts.LOGGER.error(displayName + " was not loaded as it uses an unsupported pack id");
+                    return null;
+                } else conf.id = Respackopts.sanitizeString(conf.id);
+            }
             if (RespackoptsConfig.debugLogs) Respackopts.LOGGER.info("Discovered pack: " + conf.id);
             if (Respackopts.META_VERSION < conf.version) {
                 Respackopts.LOGGER.error(displayName + " was not loaded as it specifies a newer respackopts version than is installed");

src/main/java/io/gitlab/jfronny/respackopts/model/tree/ConfigBooleanEntry.java

@@ -7,7 +7,7 @@ import io.gitlab.jfronny.respackopts.RespackoptsConfig;
 
 import java.util.Objects;
 
-public class ConfigBooleanEntry extends ConfigEntry<Boolean> {
+public class ConfigBooleanEntry extends ConfigEntry<Boolean> implements DBool {
     public ConfigBooleanEntry(boolean v) {
         super(Boolean.class);
         setValue(v);
@@ -29,11 +29,6 @@ public class ConfigBooleanEntry extends ConfigEntry<Boolean> {
         }
     }
 
-    @Override
-    public DBool getDynamic() {
-        return this::getValue;
-    }
-
     @Override
     public CategoryBuilder<?> buildEntry(GuiEntryBuilderParam args) {
         return args.builder().value(args.name(), getDefault(), this::getValue, v -> {

src/main/java/io/gitlab/jfronny/respackopts/model/tree/ConfigBranch.java

@@ -5,8 +5,9 @@ import io.gitlab.jfronny.gson.reflect.TypeToken;
 import io.gitlab.jfronny.libjf.config.api.v1.dsl.CategoryBuilder;
 import io.gitlab.jfronny.libjf.config.api.v1.dsl.ConfigBuilder;
 import io.gitlab.jfronny.muscript.data.Scope;
-import io.gitlab.jfronny.muscript.data.dynamic.*;
+import io.gitlab.jfronny.muscript.data.dynamic.Dynamic;
 import io.gitlab.jfronny.muscript.data.dynamic.additional.DFinal;
+import io.gitlab.jfronny.muscript.data.dynamic.additional.DelegateDynamic;
 import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.RespackoptsConfig;
 import io.gitlab.jfronny.respackopts.integration.SaveHook;
@@ -18,12 +19,30 @@ import io.gitlab.jfronny.respackopts.util.MetaCache;
 import java.nio.file.Path;
 import java.util.*;
 
-public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> {
+public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> implements DelegateDynamic {
     public ConfigBranch() {
         super(new TypeToken<Map<String, ConfigEntry<?>>>(){}.getRawType());
         setValue(new LinkedHashMap<>());
     }
 
+    private void checkValue() {
+        if (version < 10) return;
+        for (Iterator<String> iterator = getValue().keySet().iterator(); iterator.hasNext(); ) {
+            String s = iterator.next();
+            if (!Respackopts.isLegal(s)) {
+                Respackopts.LOGGER.error("Illegal entry for " + getName() + ", skipping: " + s);
+                iterator.remove();
+            }
+        }
+    }
+
+    @Override
+    public Map<String, ConfigEntry<?>> setValue(Map<String, ConfigEntry<?>> value) {
+        var res = super.setValue(value);
+        checkValue();
+        return res;
+    }
+
     @Override
     public void sync(ConfigEntry<Map<String, ConfigEntry<?>>> source, ConfigSyncMode mode) {
         for (Map.Entry<String, ConfigEntry<?>> e : source.getValue().entrySet()) {
@@ -59,11 +78,15 @@ public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> {
     }
 
     public <T> void add(String name, ConfigEntry<T> val) {
+        if (version >= 10 && !Respackopts.isLegal(name)) {
+            Respackopts.LOGGER.error("Illegal entry for " + getName() + ", skipping: " + name);
+            return;
+        }
         val.setVersion(version);
         val.parent = this;
         super.getValue().put(name, val);
     }
-    
+
     public ConfigEntry<?> get(String key) {
         return super.getValue().get(key);
     }
@@ -76,7 +99,7 @@ public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> {
         }
         throw new IndexOutOfBoundsException();
     }
-    
+
     public boolean has(String key) {
         return super.getValue().containsKey(key);
     }
@@ -94,17 +117,20 @@ public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> {
     }
 
     @Override
-    public DObject getDynamic() {
-        Map<String, Dynamic<?>> map = new HashMap<>();
+    public Dynamic getDelegate() {
+        Map<String, ConfigEntry<?>> map = new HashMap<>();
         super.getValue().forEach((key, value) -> {
-            map.put(Respackopts.sanitizeString(key), value.getDynamic());
+            if (Respackopts.isLegal(key)) map.put(key, value);
+            else if (version >= 10) {
+                Respackopts.LOGGER.error("Illegal key in " + getName() + ", skipping: " + key);
+            } else map.put(Respackopts.sanitizeString(key), value);
         });
         return DFinal.of(map);
     }
 
     public Scope addTo(Scope scope) {
         super.getValue().forEach((key, value) -> {
-            scope.set(Respackopts.sanitizeString(key), value.getDynamic());
+            scope.set(version >= 10 ? key : Respackopts.sanitizeString(key), value);
         });
         return scope;
     }
@@ -166,6 +192,7 @@ public class ConfigBranch extends ConfigEntry<Map<String, ConfigEntry<?>>> {
         for (ConfigEntry<?> value : getValue().values()) {
             value.setVersion(version);
         }
+        checkValue();
     }
 
     @Override

src/main/java/io/gitlab/jfronny/respackopts/model/tree/ConfigEntry.java

@@ -1,7 +1,7 @@
 package io.gitlab.jfronny.respackopts.model.tree;
 
 import io.gitlab.jfronny.libjf.config.api.v1.dsl.CategoryBuilder;
-import io.gitlab.jfronny.muscript.data.dynamic.Dynamic;
+import io.gitlab.jfronny.muscript.data.dynamic.DynamicBase;
 import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.model.enums.ConfigSyncMode;
 import io.gitlab.jfronny.respackopts.model.enums.PackReloadType;
@@ -9,7 +9,7 @@ import io.gitlab.jfronny.respackopts.util.IndentingStringBuilder;
 
 import java.util.Objects;
 
-public abstract class ConfigEntry<T> {
+public abstract class ConfigEntry<T> implements DynamicBase {
     private final Class<? super T> entryClass;
     private T defaultValue;
     private T value;
@@ -53,7 +53,7 @@ public abstract class ConfigEntry<T> {
             this.value = value;
         return this.value;
     }
-    
+
     public T getDefault() {
         if (defaultValue == null) {
             defaultValue = getValue();
@@ -76,7 +76,7 @@ public abstract class ConfigEntry<T> {
     }
     
     public void appendString(IndentingStringBuilder sb) {
-        sb.line(value + " (" + defaultValue + ")");
+        sb.line(getValue() + " (" + getDefault() + ")");
     }
 
     @Override
@@ -102,8 +102,6 @@ public abstract class ConfigEntry<T> {
 
     public abstract void buildShader(StringBuilder sb, String valueName);
 
-    public abstract Dynamic<?> getDynamic();
-
     public abstract CategoryBuilder<?> buildEntry(GuiEntryBuilderParam args);
 
     public Class<? super T> getEntryClass() {

src/main/java/io/gitlab/jfronny/respackopts/model/tree/ConfigEnumEntry.java

@@ -1,7 +1,9 @@
 package io.gitlab.jfronny.respackopts.model.tree;
 
 import io.gitlab.jfronny.libjf.config.api.v1.dsl.CategoryBuilder;
+import io.gitlab.jfronny.muscript.data.dynamic.Dynamic;
 import io.gitlab.jfronny.muscript.data.dynamic.additional.DEnum;
+import io.gitlab.jfronny.muscript.data.dynamic.additional.DelegateDynamic;
 import io.gitlab.jfronny.respackopts.Respackopts;
 import io.gitlab.jfronny.respackopts.RespackoptsConfig;
 import io.gitlab.jfronny.respackopts.model.enums.ConfigSyncMode;
@@ -9,8 +11,8 @@ import io.gitlab.jfronny.respackopts.util.IndentingStringBuilder;
 
 import java.util.*;
 
-public class ConfigEnumEntry extends ConfigEntry<String> {
-    public final List<String> values = new ArrayList<>();
+public class ConfigEnumEntry extends ConfigEntry<String> implements DelegateDynamic {
+    private final List<String> values = new ArrayList<>();
     private Integer nextValue;
 
     public ConfigEnumEntry() {
@@ -46,27 +48,56 @@ public class ConfigEnumEntry extends ConfigEntry<String> {
         return v;
     }
 
+    public List<String> getValues() {
+        return List.copyOf(values);
+    }
+
+    public void setValues(List<String> replacement) {
+        values.clear();
+        values.addAll(replacement);
+        checkValues();
+    }
+
     @Override
-    public void sync(ConfigEntry<String> source, ConfigSyncMode mode) {
-        super.sync(source, mode);
-        ConfigEnumEntry n = (ConfigEnumEntry) source;
-        if (mode == ConfigSyncMode.RESPACK_LOAD && !n.values.isEmpty()) {
-            values.clear();
-            values.addAll(n.values);
-        }
-        if (getValue() == null && nextValue != null) {
-            if (n.nextValue >= 0 && n.nextValue < values.size()) {
-                setValue(values.get(n.nextValue));
-            }
-            else {
-                Respackopts.LOGGER.error("Could not load default value for enum in " + getName());
+    public void setVersion(int version) {
+        super.setVersion(version);
+        checkValues();
+    }
+
+    private void checkValues() {
+        if (version < 10) return;
+        for (Iterator<String> iterator = values.iterator(); iterator.hasNext();) {
+            String value = iterator.next();
+            if (!Respackopts.isLegal(value)) {
+                Respackopts.LOGGER.error("Illegal enum entry for " + getName() + ", skipping: " + value);
+                iterator.remove();
             }
         }
     }
 
+    @Override
+    public void sync(ConfigEntry<String> source, ConfigSyncMode mode) {
+        super.sync(source, mode);
+
+        ConfigEnumEntry n = (ConfigEnumEntry) source;
+        if (mode == ConfigSyncMode.RESPACK_LOAD && !n.values.isEmpty()) {
+            setValues(n.values);
+        }
+        if (nextValue != null) {
+            if (getValue() == null) {
+                if (n.nextValue >= 0 && n.nextValue < values.size()) {
+                    setValue(values.get(n.nextValue));
+                }
+                else {
+                    Respackopts.LOGGER.error("Could not load default value for enum in " + getName());
+                }
+            } else nextValue = null;
+        }
+    }
+
     @Override
     public void appendString(IndentingStringBuilder sb) {
-        sb.line(getValue() + " (" + getDefault() + ") of:");
+        sb.line(getValue() + " (default=" + getDefault() + ") of:");
         IndentingStringBuilder isb = sb.indent();
         for (String e : values) {
             isb.line("- " + e);
@@ -110,7 +141,7 @@ public class ConfigEnumEntry extends ConfigEntry<String> {
     }
 
     @Override
-    public DEnum getDynamic() {
+    public Dynamic getDelegate() {
         return new DEnum(values, getValue());
     }
 

src/main/java/io/gitlab/jfronny/respackopts/model/tree/ConfigNumericEntry.java

@@ -9,7 +9,7 @@ import io.gitlab.jfronny.respackopts.util.IndentingStringBuilder;
 
 import java.util.Objects;
 
-public class ConfigNumericEntry extends ConfigEntry<Double> {
+public class ConfigNumericEntry extends ConfigEntry<Double> implements DNumber {
     public Double min = null;
     public Double max = null;
     
@@ -60,11 +60,6 @@ public class ConfigNumericEntry extends ConfigEntry<Double> {
         sb.append(getValue().toString());
     }
 
-    @Override
-    public DNumber getDynamic() {
-        return this::getValue;
-    }
-
     @Override
     public CategoryBuilder<?> buildEntry(GuiEntryBuilderParam args) {
         return args.builder().value(args.name(), getValue(), min == null ? Double.NEGATIVE_INFINITY : min, max == null ? Double.POSITIVE_INFINITY : max, this::getValue, v -> {

src/main/java/io/gitlab/jfronny/respackopts/util/MetaCache.java

@@ -161,9 +161,8 @@ public class MetaCache {
     public static Scope getScope(@Nullable CacheKey key) {
         Scope scope = (key == null ? Respackopts.ROOT_SCOPE : MetaCache.getState(key).executionScope()).fork();
         MetaCache.forEach((id, state) -> {
-            String packId = Respackopts.sanitizeString(state.packId());
-            if (!scope.has(packId)) {
-                scope.set(packId, state.configBranch().getDynamic());
+            if (!scope.has(state.packId())) {
+                scope.set(state.packId(), state.configBranch());
             }
         });
         return scope;