Rewrite variable parsing to use element names

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

core/src/main/java/tc/oc/pgm/filters/matcher/match/VariableFilter.java

@@ -37,8 +37,7 @@ public boolean respondsTo(Class<? extends Query> queryType) {
 
   @Override
   public boolean isDynamic() {
-    // Variables' setValue will always invalidate the filterable directly, no events required
-    return true;
+    return variable.isDynamic();
   }
 
   @Override

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

@@ -21,7 +21,6 @@
 import tc.oc.pgm.util.xml.Node;
 import tc.oc.pgm.util.xml.XMLUtils;
 import tc.oc.pgm.variables.VariableDefinition;
-import tc.oc.pgm.variables.VariableType;
 import tc.oc.pgm.variables.VariablesModule;
 
 public class FeatureFilterParser extends FilterParser {
@@ -100,9 +99,6 @@ public Filter parseNot(Element el) throws InvalidXMLException {
     if (varMatch.matches()) {
       VariableDefinition<?> variable =
           features.resolve(node, varMatch.group(1), VariableDefinition.class);
-      if (!variable.getVariableType().equals(VariableType.DUMMY)) {
-        throw new InvalidXMLException("Variable filters only support dummy variables!", node);
-      }
       Range<Double> range = XMLUtils.parseNumericRange(node, varMatch.group(2), Double.class);
       return new VariableFilter(variable, range);
     }

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

@@ -93,7 +93,6 @@
 import tc.oc.pgm.util.xml.Node;
 import tc.oc.pgm.util.xml.XMLUtils;
 import tc.oc.pgm.variables.VariableDefinition;
-import tc.oc.pgm.variables.VariableType;
 
 public abstract class FilterParser implements XMLParser<Filter, FilterDefinition> {
 
@@ -660,9 +659,6 @@ public PlayerCountFilter parsePlayerCountFilter(Element el) throws InvalidXMLExc
   public Filter parseVariableFilter(Element el) throws InvalidXMLException {
     VariableDefinition<?> varDef =
         features.resolve(Node.fromRequiredAttr(el, "var"), VariableDefinition.class);
-    if (!varDef.getVariableType().equals(VariableType.DUMMY)) {
-      throw new InvalidXMLException("Variable filters only support dummy variables!", el);
-    }
     Range<Double> range = XMLUtils.parseNumericRange(new Node(el), Double.class);
 
     if (varDef.getScope() == Party.class)

core/src/main/java/tc/oc/pgm/variables/VariableDefinition.java

@@ -1,53 +1,41 @@
 package tc.oc.pgm.variables;
 
+import java.util.function.Function;
 import tc.oc.pgm.api.match.Match;
 import tc.oc.pgm.features.SelfIdentifyingFeatureDefinition;
 import tc.oc.pgm.filters.Filterable;
 
 public class VariableDefinition<T extends Filterable<?>> extends SelfIdentifyingFeatureDefinition {
 
   private final Class<T> scope;
-  private final double def;
-  private final VariableType variableType;
-
-  public VariableDefinition(String id, Class<T> scope, double def, VariableType type) {
+  private final boolean isDynamic;
+  private final Function<VariableDefinition<T>, Variable<T>> builder;
+
+  public VariableDefinition(
+      String id,
+      Class<T> scope,
+      boolean isDynamic,
+      Function<VariableDefinition<T>, Variable<T>> builder) {
     super(id);
     this.scope = scope;
-    this.def = def;
-    this.variableType = type;
+    this.isDynamic = isDynamic;
+    this.builder = builder;
   }
 
   public Class<T> getScope() {
     return scope;
   }
 
-  public double getDefault() {
-    return def;
-  }
-
-  public Variable<?> buildInstance() {
-    return getVariableType().buildInstance(this);
+  public boolean isDynamic() {
+    return isDynamic;
   }
 
-  public VariableType getVariableType() {
-    return variableType;
+  public Variable<T> buildInstance() {
+    return builder.apply(this);
   }
 
   @SuppressWarnings("unchecked")
   public Variable<T> getVariable(Match match) {
     return (Variable<T>) match.getFeatureContext().get(this.getId());
   }
-
-  public static class Context<T extends Filterable<?>, C> extends VariableDefinition<T> {
-    private final C context;
-
-    public Context(String id, Class<T> scope, double def, VariableType type, C context) {
-      super(id, scope, def, type);
-      this.context = context;
-    }
-
-    public C getContext() {
-      return context;
-    }
-  }
 }

core/src/main/java/tc/oc/pgm/variables/VariableParser.java

@@ -0,0 +1,92 @@
+package tc.oc.pgm.variables;
+
+import java.lang.reflect.Method;
+import java.util.Map;
+import java.util.regex.Pattern;
+import org.jdom2.Element;
+import tc.oc.pgm.api.feature.FeatureReference;
+import tc.oc.pgm.api.filter.Filterables;
+import tc.oc.pgm.api.map.factory.MapFactory;
+import tc.oc.pgm.api.match.Match;
+import tc.oc.pgm.api.party.Party;
+import tc.oc.pgm.api.player.MatchPlayer;
+import tc.oc.pgm.filters.Filterable;
+import tc.oc.pgm.teams.TeamFactory;
+import tc.oc.pgm.util.MethodParser;
+import tc.oc.pgm.util.MethodParsers;
+import tc.oc.pgm.util.xml.InvalidXMLException;
+import tc.oc.pgm.util.xml.Node;
+import tc.oc.pgm.util.xml.XMLUtils;
+import tc.oc.pgm.variables.types.BlitzVariable;
+import tc.oc.pgm.variables.types.DummyVariable;
+import tc.oc.pgm.variables.types.ScoreVariable;
+import tc.oc.pgm.variables.types.TeamVariableAdapter;
+
+public class VariableParser {
+  // The limitation is due to them being used in exp4j formulas for.
+  public static final Pattern VARIABLE_ID = Pattern.compile("[A-Za-z_]\\w*");
+
+  private final MapFactory factory;
+  private final Map<String, Method> methodParsers;
+
+  public VariableParser(MapFactory factory) {
+    this.factory = factory;
+    this.methodParsers = MethodParsers.getMethodParsersForClass(getClass());
+  }
+
+  public VariableDefinition<?> parse(Element el) throws InvalidXMLException {
+    String id = Node.fromRequiredAttr(el, "id").getValue();
+    if (!VARIABLE_ID.matcher(id).matches())
+      throw new InvalidXMLException(
+          "Variable IDs must start with a letter or underscore and can only include letters, digits or underscores.",
+          el);
+
+    Method parser = methodParsers.get(el.getName().toLowerCase());
+    if (parser != null) {
+      try {
+        return (VariableDefinition<?>) parser.invoke(this, el, id);
+      } catch (Exception e) {
+        throw InvalidXMLException.coerce(e, new Node(el));
+      }
+    } else {
+      throw new InvalidXMLException("Unknown variable type: " + el.getName(), el);
+    }
+  }
+
+  @MethodParser("variable")
+  public VariableDefinition<?> parseDummy(Element el, String id) throws InvalidXMLException {
+    Class<? extends Filterable<?>> scope = Filterables.parse(Node.fromRequiredAttr(el, "scope"));
+    double def = XMLUtils.parseNumber(Node.fromAttr(el, "default"), Double.class, 0d);
+    return new VariableDefinition<>(id, scope, true, vd -> new DummyVariable<>(vd, def));
+  }
+
+  @MethodParser("lives")
+  public VariableDefinition<MatchPlayer> parseBlitzLives(Element el, String id)
+      throws InvalidXMLException {
+    return new VariableDefinition<>(id, MatchPlayer.class, false, BlitzVariable::new);
+  }
+
+  @MethodParser("score")
+  public VariableDefinition<Party> parseScore(Element el, String id) throws InvalidXMLException {
+    return new VariableDefinition<>(id, Party.class, false, ScoreVariable::new);
+  }
+
+  @MethodParser("with-team")
+  public VariableDefinition<Match> parseTeamAdapter(Element el, String id)
+      throws InvalidXMLException {
+    @SuppressWarnings("unchecked")
+    VariableDefinition<Party> var =
+        factory.getFeatures().resolve(Node.fromRequiredAttr(el, "var"), VariableDefinition.class);
+    if (var.getScope() != Party.class) {
+      throw new InvalidXMLException(
+          "Team scope is required for with-team variable, got " + var.getScope().getSimpleName(),
+          el);
+    }
+
+    FeatureReference<TeamFactory> team =
+        factory.getFeatures().createReference(Node.fromRequiredAttr(el, "team"), TeamFactory.class);
+
+    return new VariableDefinition<>(
+        id, Match.class, var.isDynamic(), vd -> new TeamVariableAdapter(vd, var, team));
+  }
+}

core/src/main/java/tc/oc/pgm/variables/VariablesModule.java

@@ -24,7 +24,6 @@
 import tc.oc.pgm.score.ScoreMatchModule;
 import tc.oc.pgm.teams.TeamMatchModule;
 import tc.oc.pgm.util.xml.InvalidXMLException;
-import tc.oc.pgm.util.xml.Node;
 import tc.oc.pgm.util.xml.XMLUtils;
 
 public class VariablesModule implements MapModule<VariablesMatchModule> {
@@ -101,41 +100,15 @@ public VariablesModule parse(MapFactory factory, Logger logger, Document doc)
         throws InvalidXMLException {
 
       ImmutableList.Builder<VariableDefinition<?>> variables = ImmutableList.builder();
-      for (Element variable :
-          XMLUtils.flattenElements(doc.getRootElement(), "variables", "variable")) {
-
-        String id = Node.fromRequiredAttr(variable, "id").getValue();
-        if (!VARIABLE_ID.matcher(id).matches())
-          throw new InvalidXMLException(
-              "Variable IDs must start with a letter or underscore and can only include letters, digits or underscores.",
-              variable);
-        VariableType type =
-            XMLUtils.parseEnum(
-                Node.fromAttr(variable, "type"), VariableType.class, VariableType.DUMMY);
-
-        Class<? extends Filterable<?>> scope = parseScope(variable, type);
-        double def = XMLUtils.parseNumber(Node.fromAttr(variable, "default"), Double.class, 0d);
-
-        if (!type.supports(scope)) {
-          throw new InvalidXMLException(
-              "VariableType " + type + " does not support scope: " + scope, variable);
-        }
-
-        VariableDefinition<?> varDef =
-            new VariableDefinition.Context<>(id, scope, def, type, type.build(factory, variable));
+      VariableParser parser = new VariableParser(factory);
+
+      for (Element variable : XMLUtils.flattenElements(doc.getRootElement(), "variables", null)) {
+        VariableDefinition<?> varDef = parser.parse(variable);
         factory.getFeatures().addFeature(variable, varDef);
         variables.add(varDef);
       }
 
       return new VariablesModule(variables.build());
     }
-
-    private Class<? extends Filterable<?>> parseScope(Element variable, VariableType type)
-        throws InvalidXMLException {
-      if (type.getDefaultScope() != null && variable.getAttribute("scope") == null) {
-        return type.getDefaultScope();
-      }
-      return Filterables.parse(Node.fromRequiredAttr(variable, "scope"));
-    }
   }
 }

core/src/main/java/tc/oc/pgm/variables/types/AbstractVariable.java

@@ -4,16 +4,15 @@
 import tc.oc.pgm.variables.Variable;
 import tc.oc.pgm.variables.VariableDefinition;
 
-public abstract class AbstractVariable<T extends Filterable<?>, D extends VariableDefinition<T>>
-    implements Variable<T> {
-  protected final D definition;
+public abstract class AbstractVariable<T extends Filterable<?>> implements Variable<T> {
+  protected final VariableDefinition<T> definition;
 
-  public AbstractVariable(D definition) {
+  public AbstractVariable(VariableDefinition<T> definition) {
     this.definition = definition;
   }
 
   @Override
-  public D getDefinition() {
+  public VariableDefinition<T> getDefinition() {
     return definition;
   }
 

core/src/main/java/tc/oc/pgm/variables/types/BlitzVariable.java

@@ -5,12 +5,12 @@
 import tc.oc.pgm.blitz.BlitzMatchModule;
 import tc.oc.pgm.variables.VariableDefinition;
 
-public class BlitzVariable extends AbstractVariable<MatchPlayer, VariableDefinition<MatchPlayer>> {
+public class BlitzVariable extends AbstractVariable<MatchPlayer> {
 
   private BlitzMatchModule bmm;
 
-  public BlitzVariable(VariableDefinition<?> definition) {
-    super((VariableDefinition<MatchPlayer>) definition);
+  public BlitzVariable(VariableDefinition<MatchPlayer> definition) {
+    super(definition);
   }
 
   @Override