fix: reinvent DirFilterAdditive

docs/additional/Migrations.md

@@ -83,5 +83,4 @@ Corresponds to version 4.4.0
 Corresponds to version 4.5.0
 
 - Directory RPOs in subdirectories are respected. Previously, only the innermost directory RPO would be used
-- Multiple replacements when resolving fallbacks for directory RPOs are prevented
-- DirFilterAdditive is now ignored as it is no longer necessary
+- Multiple replacements when resolving fallbacks for directory RPOs are prevented

src/main/java/io/gitlab/jfronny/respackopts/filters/DebugEvents.java

@@ -7,6 +7,8 @@ import net.minecraft.resource.metadata.ResourceMetadataReader;
 import net.minecraft.util.Identifier;
 
 import java.io.InputStream;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.function.Supplier;
 
 public enum DebugEvents implements UserResourceEvents.FindResource, UserResourceEvents.ParseMetadata, UserResourceEvents.Open, UserResourceEvents.OpenRoot {
@@ -25,7 +27,13 @@ public enum DebugEvents implements UserResourceEvents.FindResource, UserResource
     @Override
     public ResourcePack.ResultConsumer findResources(ResourceType type, String namespace, String prefix, ResourcePack.ResultConsumer previous, ResourcePack pack) {
         Respackopts.LOGGER.info("FIND_RESOURCE " + type + " in " + namespace + " " + prefix + " of " + pack.getName());
-        return previous;
+        Set<Identifier> results = new HashSet<>();
+        return (identifier, inputStreamInputSupplier) -> {
+            if (!results.add(identifier)) {
+                Respackopts.LOGGER.warn("Duplicate identifier returned by findResources: " + identifier);
+            }
+            previous.accept(identifier, inputStreamInputSupplier);
+        };
     }
 
     @Override

src/main/java/io/gitlab/jfronny/respackopts/filters/DirFilterEvents.java

@@ -13,8 +13,7 @@ import net.minecraft.resource.*;
 import net.minecraft.util.Identifier;
 
 import java.io.*;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
 
 public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEvents.FindResource {
     INSTANCE;
@@ -28,7 +27,7 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
     public InputSupplier<InputStream> open(ResourceType type, Identifier id, InputSupplier<InputStream> previous, ResourcePack pack) {
         if (!MetaCache.hasCapability(pack, PackCapability.DirFilter)) return previous;
         String path = new ResourcePath(type, id).getName();
-        List<DirRpo> rpo = findDirRpos(pack, path);
+        List<DirRpo> rpo = findDirRpos(pack, parent(path));
         //TODO use pattern matching for switch
         DirRpoResult result = DirRpoResult.compute(path, rpo, MetaCache.getKeyByPack(pack));
         if (result == DirRpoResult.ORIGINAL) return previous; // Using original file
@@ -44,10 +43,14 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
         // Warning: the Identifiers here DON'T CONTAIN THE TYPE!
         // Therefore, it needs to be added when calling a method that generates a ResourcePath!
         if (!MetaCache.hasCapability(pack, PackCapability.DirFilter)) return previous;
+        boolean dirFilterAdditive = MetaCache.hasCapability(pack, PackCapability.DirFilterAdditive);
+        String searchPrefix = type.getDirectory() + "/" + namespace + "/" + prefix;
+        Set<String> additionalSearched = new HashSet<>();
         return (identifier, value) -> {
-            String path = type.getDirectory() + "/" + identifier.getNamespace() + "/" + identifier.getPath();
+            String path = path(type, identifier);
             //TODO use pattern matching for switch
-            DirRpoResult result = DirRpoResult.compute(path, findDirRpos(pack, path), MetaCache.getKeyByPack(pack));
+            List<DirRpo> relevantRpos = findDirRpos(pack, parent(path));
+            DirRpoResult result = DirRpoResult.compute(path, relevantRpos, MetaCache.getKeyByPack(pack));
             if (result == DirRpoResult.ORIGINAL) { // Using original file
                 previous.accept(identifier, value);
                 return;
@@ -56,34 +59,68 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
             // New search for fallback path
             DirRpoResult.Replacement replacement = (DirRpoResult.Replacement) result;
             String newPath = replacement.toFallback(path);
-            if (newPath.split("/", 3).length == 3) {
+            if (newPath.split("/", 3).length != 3) {
+                Respackopts.LOGGER.error("Directory fallback path MUST be long enough to support representation as identifier (3 segments), but is too short: " + newPath);
+                return;
+            }
+            if (!dirFilterAdditive) {
+                // Only return this single result, don't search for others
                 ResourcePath rp = new ResourcePath(newPath);
-                pack.findResources(rp.getType(), rp.getId().getNamespace(), rp.getId().getPath(), (resource, resVal) -> {
-                    String fallbackPath = rp.getType().getDirectory() + "/" + resource.getNamespace() + "/" + resource.getPath();
-                    previous.accept(new ResourcePath(replacement.toOriginal(fallbackPath)).getId(), resVal);
-                });
-            } else Respackopts.LOGGER.error("Directory fallback path MUST be long enough to support representation as identifier (3 segments), but is too short: " + newPath);
+                previous.accept(identifier, pack.open(rp.getType(), rp.getId()));
+                return;
+            }
+            // Find other files in the fallback directory
+            String fallbackDir = replacement.fallback.prefix();
+            if (!additionalSearched.add(fallbackDir)) return; // Already searched
+            int prefixSize = replacement.original.prefix().length();
+            if (prefixSize < searchPrefix.length()) {
+                if (!searchPrefix.startsWith(replacement.original.prefix())) {
+                    Respackopts.LOGGER.error("Unexpected prefix path " + replacement.original.prefix() + " for search prefix " + searchPrefix + ", skipping");
+                    return;
+                }
+                fallbackDir += searchPrefix.substring(prefixSize);
+            } else if (!replacement.original.prefix().startsWith(searchPrefix)) {
+                Respackopts.LOGGER.error("Unexpected prefix path " + replacement.original.prefix() + " for search prefix " + searchPrefix + ", skipping");
+                return;
+            }
+            if (fallbackDir.split("/", 3).length != 3) {
+                Respackopts.LOGGER.error("Directory fallback path MUST be long enough to support representation as identifier (3 segments), but is too short: " + fallbackDir);
+                return;
+            }
+            ResourcePath rp = new ResourcePath(fallbackDir);
+            pack.findResources(rp.getType(), rp.getId().getNamespace(), rp.getId().getPath(), (resource, resVal) -> {
+                String fallbackPath = path(rp.getType(), resource);
+                previous.accept(new ResourcePath(replacement.toOriginal(fallbackPath)).getId(), resVal);
+            });
         };
     }
 
+    private String path(ResourceType type, Identifier identifier) {
+        return type.getDirectory() + "/" + identifier.getNamespace() + "/" + identifier.getPath();
+    }
+
+    private String parent(String path) {
+        int li = path.lastIndexOf('/');
+        return li <= 0 ? null : path.substring(0, li);
+    }
+
     /**
-     * Identify all directory RPOs relevant to the file at name (IE in its parent directories), from outermost to innermost
+     * Identify all directory RPOs relevant to the directory at {@code path} (IE in it or its parents), from outermost to innermost
      */
-    private List<DirRpo> findDirRpos(ResourcePack pack, String name) {
-        int li = name.lastIndexOf('/');
-        if (li <= 0) return List.of();
-        name = name.substring(0, li);
+    private List<DirRpo> findDirRpos(ResourcePack pack, String path) {
+        if (path == null) return List.of();
+
         CachedPackState state = MetaCache.getState(MetaCache.getKeyByPack(pack));
         var cache = state.cachedDirRPOs();
 
         // This is outside computeIfAbsent because it could cause modification of the map, which is unsupported within it
-        if (cache.containsKey(name)) return cache.get(name);
-        List<DirRpo> parentRPOs = findDirRpos(pack, name);
+        if (cache.containsKey(path)) return cache.get(path);
+        List<DirRpo> parentRPOs = findDirRpos(pack, parent(path));
         synchronized (cache) { // This is synchronized as multiple resources might be accessed at the same time, potentially causing a CME here
-            return cache.computeIfAbsent(name, parentName -> {
+            return cache.computeIfAbsent(path, $ -> {
                 ResourcePath rp;
                 try {
-                    rp = new ResourcePath(parentName + "/" + Respackopts.FILE_EXTENSION);
+                    rp = new ResourcePath(path + "/" + Respackopts.FILE_EXTENSION);
                 } catch (Exception e) {
                     return parentRPOs;
                 }
@@ -92,7 +129,7 @@ public enum DirFilterEvents implements UserResourceEvents.Open, UserResourceEven
                 try (Reader w = new InputStreamReader(is.get())) {
                     List<DirRpo> currentRPOs = new LinkedList<>(parentRPOs);
                     DirRpo newRPO = AttachmentHolder.deserialize(state.metadata().version, w, DirRpo.class);
-                    newRPO.hydrate(parentName);
+                    newRPO.hydrate(path);
                     if (newRPO.fallback != null && !newRPO.fallback.endsWith("/"))
                         newRPO.fallback += "/";
                     currentRPOs.add(newRPO);

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

@@ -46,42 +46,48 @@ public sealed interface DirRpoResult {
     enum FixedState implements DirRpoResult {IGNORE, ORIGINAL}
 
     final class Replacement implements DirRpoResult {
-        private final String originalPrefix;
-        private final Pattern originalPattern;
-        private final String fallbackPrefix;
-        private final Pattern fallbackPattern;
+        public final Prefix original;
+        public final Prefix fallback;
         private final int version;
 
+        public record Prefix(String prefix, String quoted, Pattern pattern) {
+            public Prefix(String prefix) {
+                this(
+                        prefix,
+                        Matcher.quoteReplacement(prefix),
+                        Pattern.compile(Pattern.quote(prefix))
+                );
+            }
+        }
+
         public Replacement(String originalPrefix, String fallbackPrefix, int version) {
-            this.originalPrefix = Matcher.quoteReplacement(originalPrefix);
-            this.originalPattern = Pattern.compile(Pattern.quote(originalPrefix));
-            this.fallbackPrefix = Matcher.quoteReplacement(fallbackPrefix);
-            this.fallbackPattern = Pattern.compile(Pattern.quote(fallbackPrefix));
+            this.original = new Prefix(originalPrefix);
+            this.fallback = new Prefix(fallbackPrefix);
             this.version = version;
         }
 
         public String toFallback(String original) {
-            Matcher m = originalPattern.matcher(original);
+            Matcher m = this.original.pattern.matcher(original);
             if (version < 11) {
-                return m.replaceAll(fallbackPrefix);
+                return m.replaceAll(this.fallback.quoted);
             }
             if (!m.find()) {
                 Respackopts.LOGGER.error("Attempted conversion to fallback path, but could not find original prefix for: " + original);
                 return original;
             }
-            return m.replaceFirst(fallbackPrefix);
+            return m.replaceFirst(this.fallback.quoted);
         }
 
         public String toOriginal(String fallback) {
-            Matcher m = fallbackPattern.matcher(fallback);
+            Matcher m = this.fallback.pattern.matcher(fallback);
             if (version < 11) {
-                return m.replaceAll(originalPrefix);
+                return m.replaceAll(this.original.quoted);
             }
             if (!m.find()) {
                 Respackopts.LOGGER.error("Attempted conversion to original path, but could not find fallback prefix for: " + fallback);
                 return fallback;
             }
-            return m.replaceFirst(originalPrefix);
+            return m.replaceFirst(this.original.quoted);
         }
     }
 }