Optimize list of material filters (#1464)

Signed-off-by: Pablo Herrera <[email protected]>

core/src/main/java/tc/oc/pgm/broadcast/Broadcast.java

@@ -17,7 +17,6 @@
 public class Broadcast implements Comparable<Broadcast> {
   public enum Type {
     TIP(translatable("misc.tip", NamedTextColor.BLUE), Sounds.TIP),
-
     ALERT(translatable("misc.alert", NamedTextColor.YELLOW), Sounds.ALERT);
 
     final Component prefix;

core/src/main/java/tc/oc/pgm/filters/matcher/block/MaterialFilter.java

@@ -17,6 +17,10 @@ public MaterialFilter(MaterialMatcher pattern) {
     this.pattern = pattern;
   }
 
+  public MaterialMatcher getPattern() {
+    return pattern;
+  }
+
   @Override
   public Class<? extends MaterialQuery> queryType() {
     return MaterialQuery.class;

core/src/main/java/tc/oc/pgm/filters/operator/AnyFilter.java

@@ -1,8 +1,11 @@
 package tc.oc.pgm.filters.operator;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import tc.oc.pgm.api.filter.Filter;
 import tc.oc.pgm.api.filter.query.Query;
+import tc.oc.pgm.filters.matcher.block.MaterialFilter;
+import tc.oc.pgm.util.material.MaterialMatcher;
 
 public class AnyFilter extends MultiFilterFunction {
 
@@ -26,10 +29,28 @@ public QueryResponse query(Query query) {
   }
 
   public static Filter of(Filter... filters) {
-    return MultiFilterFunction.of(AnyFilter::new, filters);
+    return MultiFilterFunction.of(AnyFilter::mergeMaterials, filters);
   }
 
   public static Filter of(Collection<Filter> filters) {
-    return MultiFilterFunction.of(AnyFilter::new, filters);
+    return MultiFilterFunction.of(AnyFilter::mergeMaterials, filters);
+  }
+
+  /**
+   * Optimization when creating any -> materials type of filters. The material filters get unwrapped
+   * and turned into just one material filter with a multi-material matcher.
+   *
+   * @param filters collection of the filters to wrap into an AnyFilter
+   * @return an equivalent of AnyFilter, but with optimized materials
+   */
+  private static Filter mergeMaterials(Collection<Filter> filters) {
+    var materials = MaterialMatcher.builder();
+    var children = new ArrayList<Filter>(filters.size());
+    for (Filter filter : filters) {
+      if (filter instanceof MaterialFilter mf) materials.add(mf.getPattern());
+      else children.add(filter);
+    }
+    if (!materials.isEmpty()) children.add(new MaterialFilter(materials.build()));
+    return MultiFilterFunction.of(AnyFilter::new, children);
   }
 }

core/src/main/java/tc/oc/pgm/filters/operator/FilterWrapper.java

@@ -0,0 +1,32 @@
+package tc.oc.pgm.filters.operator;
+
+import org.jdom2.Element;
+import tc.oc.pgm.api.feature.FeatureReference;
+import tc.oc.pgm.api.filter.Filter;
+import tc.oc.pgm.api.filter.query.Query;
+
+/**
+ * Wrapper around a child filter, exists only as a parsing optimization. Single-child "any", "all"
+ * or "one" filters can often be replaced with their single child, but when the child is a
+ * reference, they can instead be turned into a wrapped filter. <br>
+ * For example, {@code <all id="outer"><region id="other"/></all>}. Because the child is a
+ * reference, if we simplified parsing to returning "other", pgm wouldn't register it under the
+ * "outer" name (you can't register a reference, otherwise it creates duplicates). For those cases,
+ * we create a filter wrapper around "other", and return that instead.
+ */
+public class FilterWrapper extends SingleFilterFunction {
+
+  private FilterWrapper(Filter filter) {
+    super(filter);
+  }
+
+  @Override
+  public QueryResponse query(Query query) {
+    return filter.query(query);
+  }
+
+  public static Filter of(Element el, Filter filter) {
+    if (el.getAttribute("id") == null || !(filter instanceof FeatureReference<?>)) return filter;
+    return new FilterWrapper(filter);
+  }
+}

core/src/main/java/tc/oc/pgm/filters/operator/MultiFilterFunction.java

@@ -49,25 +49,19 @@ public String toString() {
   }
 
   public static Filter of(Function<Collection<Filter>, Filter> builder, Filter... filters) {
-    switch (filters.length) {
-      case 0:
-        return StaticFilter.ABSTAIN;
-      case 1:
-        return filters[0];
-      default:
-        return builder.apply(Arrays.asList(filters));
-    }
+    return switch (filters.length) {
+      case 0 -> StaticFilter.ABSTAIN;
+      case 1 -> filters[0];
+      default -> builder.apply(Arrays.asList(filters));
+    };
   }
 
   public static Filter of(
       Function<Collection<Filter>, Filter> builder, Collection<Filter> filters) {
-    switch (filters.size()) {
-      case 0:
-        return StaticFilter.ABSTAIN;
-      case 1:
-        return filters.iterator().next();
-      default:
-        return builder.apply(filters);
-    }
+    return switch (filters.size()) {
+      case 0 -> StaticFilter.ABSTAIN;
+      case 1 -> filters.iterator().next();
+      default -> builder.apply(filters);
+    };
   }
 }

core/src/main/java/tc/oc/pgm/filters/parse/FeatureFilterParser.java

@@ -89,12 +89,6 @@ public Filter parseDeny(Element el) throws InvalidXMLException {
     return new DenyFilter(parseChild(el));
   }
 
-  // Override with a version that only accepts a single child filter
-  @MethodParser("not")
-  public Filter parseNot(Element el) throws InvalidXMLException {
-    return new InverseFilter(parseChild(el));
-  }
-
   private static final Pattern INLINE_VARIABLE =
       Pattern.compile("(%VAR%)(?:\\[(\\d+)])?\\s*=\\s*(%RANGE%|%NUM%)"
           .replace("%VAR%", VariablesModule.Factory.VARIABLE_ID.pattern())

core/src/main/java/tc/oc/pgm/filters/parse/FilterParser.java

@@ -66,6 +66,7 @@
 import tc.oc.pgm.filters.modifier.SameTeamQueryModifier;
 import tc.oc.pgm.filters.operator.AllFilter;
 import tc.oc.pgm.filters.operator.AnyFilter;
+import tc.oc.pgm.filters.operator.FilterWrapper;
 import tc.oc.pgm.filters.operator.InverseFilter;
 import tc.oc.pgm.filters.operator.OneFilter;
 import tc.oc.pgm.filters.operator.TeamFilterAdapter;
@@ -220,22 +221,22 @@ public Filter parseNever(Element el) {
 
   @MethodParser("any")
   public Filter parseAny(Element el) throws InvalidXMLException {
-    return new AnyFilter(parseChildren(el));
+    return FilterWrapper.of(el, AnyFilter.of(parseChildren(el)));
   }
 
   @MethodParser("all")
   public Filter parseAll(Element el) throws InvalidXMLException {
-    return new AllFilter(parseChildren(el));
+    return FilterWrapper.of(el, AllFilter.of(parseChildren(el)));
   }
 
   @MethodParser("one")
   public Filter parseOne(Element el) throws InvalidXMLException {
-    return new OneFilter(parseChildren(el));
+    return FilterWrapper.of(el, OneFilter.of(parseChildren(el)));
   }
 
   @MethodParser("not")
   public Filter parseNot(Element el) throws InvalidXMLException {
-    return new InverseFilter(AnyFilter.of(parseChildren(el)));
+    return new InverseFilter(parseChild(el));
   }
 
   @MethodParser("team")

core/src/main/java/tc/oc/pgm/filters/parse/LegacyFilterParser.java

@@ -10,7 +10,9 @@
 import tc.oc.pgm.api.map.factory.MapFactory;
 import tc.oc.pgm.api.region.Region;
 import tc.oc.pgm.filters.matcher.block.MaterialFilter;
+import tc.oc.pgm.filters.operator.AnyFilter;
 import tc.oc.pgm.filters.operator.FilterNode;
+import tc.oc.pgm.filters.operator.InverseFilter;
 import tc.oc.pgm.util.MethodParser;
 import tc.oc.pgm.util.material.MaterialMatcher;
 import tc.oc.pgm.util.xml.InvalidXMLException;
@@ -112,6 +114,12 @@ public Filter parseFilter(Element el) throws InvalidXMLException {
     }
   }
 
+  // Legacy not allows for multiple children and is an implicit and
+  @MethodParser("not")
+  public Filter parseNot(Element el) throws InvalidXMLException {
+    return new InverseFilter(AnyFilter.of(parseChildren(el)));
+  }
+
   // Removed in proto 1.4 to avoid conflict with <block> region
   @MethodParser("block")
   public Filter parseBlock(Element el) throws InvalidXMLException {

platform/platform-modern/src/main/java/tc/oc/pgm/platform/modern/material/BlockStateMaterialMatcher.java

@@ -8,8 +8,7 @@
 import tc.oc.pgm.util.material.MaterialData;
 import tc.oc.pgm.util.material.MaterialMatcher;
 
-@SuppressWarnings("deprecation")
-public class BlockStateMaterialMatcher implements MaterialMatcher {
+public class BlockStateMaterialMatcher implements MaterialMatcher.Singular {
   private final BlockData data;
 
   public BlockStateMaterialMatcher(BlockData data) {
@@ -21,6 +20,11 @@ public Set<Material> getMaterials() {
     return Set.of(data.getMaterial());
   }
 
+  @Override
+  public Material getMaterial() {
+    return data.getMaterial();
+  }
+
   @Override
   public boolean matches(Material material) {
     return material == data.getMaterial();

platform/platform-sportpaper/src/main/java/tc/oc/pgm/platform/sportpaper/material/ExactMaterialMatcher.java

@@ -8,7 +8,7 @@
 import tc.oc.pgm.util.material.MaterialMatcher;
 
 @SuppressWarnings("deprecation")
-public class ExactMaterialMatcher implements MaterialMatcher {
+public class ExactMaterialMatcher implements MaterialMatcher.Singular {
   private final Material material;
   private final byte data;
 
@@ -22,6 +22,11 @@ public Set<Material> getMaterials() {
     return Set.of(material);
   }
 
+  @Override
+  public Material getMaterial() {
+    return material;
+  }
+
   @Override
   public boolean matches(Material material) {
     return material == this.material && this.data == 0;

util/src/main/java/tc/oc/pgm/util/material/MaterialMatcher.java

@@ -76,6 +76,10 @@ static MaterialMatcher.Builder builder() {
     return MATERIAL_UTILS.matcherBuilder();
   }
 
+  interface Singular extends MaterialMatcher {
+    Material getMaterial();
+  }
+
   interface Builder {
     /**
      * Set the builder to only accept materials, error on material:data syntax
@@ -206,6 +210,16 @@ public Builder addAll(Predicate<Material> materialPredicate) {
 
     @Override
     public Builder add(MaterialMatcher matcher) {
+      // Already part of the matcher in form of material, can ignore
+      if (matcher instanceof Singular s && materials.contains(s.getMaterial())) return this;
+      if (matcher instanceof SingularMaterialMatcher
+          || matcher instanceof MultipleMaterialMatcher) {
+        Set<Material> toAdd = matcher.getMaterials();
+        materials.addAll(toAdd);
+        // Ignore any matcher already handled by our generic material list
+        matchers.removeIf(m -> m instanceof Singular s && toAdd.contains(s.getMaterial()));
+        return this;
+      }
       matchers.add(matcher);
       return this;
     }

util/src/main/java/tc/oc/pgm/util/material/matcher/MultipleMaterialMatcher.java

@@ -55,4 +55,9 @@ public static MaterialMatcher of(Collection<Material> materials) {
       default -> new MultipleMaterialMatcher(materials);
     };
   }
+
+  @Override
+  public String toString() {
+    return "MultipleMaterialMatcher{" + "materials=" + materials + '}';
+  }
 }

util/src/main/java/tc/oc/pgm/util/material/matcher/SingularMaterialMatcher.java

@@ -9,7 +9,7 @@
 import tc.oc.pgm.util.material.MaterialData;
 import tc.oc.pgm.util.material.MaterialMatcher;
 
-public class SingularMaterialMatcher implements MaterialMatcher {
+public class SingularMaterialMatcher implements MaterialMatcher.Singular {
   private final Material material;
 
   private SingularMaterialMatcher(Material material) {
@@ -36,6 +36,11 @@ public Set<Material> getMaterials() {
     return Set.of(material);
   }
 
+  @Override
+  public Material getMaterial() {
+    return material;
+  }
+
   @Override
   public Set<BlockMaterialData> getPossibleBlocks() {
     return MATERIAL_UTILS.getPossibleBlocks(material);
@@ -44,4 +49,9 @@ public Set<BlockMaterialData> getPossibleBlocks() {
   public static MaterialMatcher of(Material material) {
     return material == null ? NoMaterialMatcher.INSTANCE : new SingularMaterialMatcher(material);
   }
+
+  @Override
+  public String toString() {
+    return "SingularMaterialMatcher{" + "material=" + material + '}';
+  }
 }