Improve module errors, and add ModuleReporter

build.gradle

@@ -37,6 +37,7 @@ dependencies {
     compile 'ee.ellytr.command:EllyCommand:2.0.4'
     compile 'ee.ellytr.chat:EllyChat:1.2.6'
     compile 'org.jdom:jdom2:2.0.6'
+    compile 'org.jdom:jdom2-contrib:2.0.5'
     compile 'org.ow2.asm:asm-all:5.0.4'
 
     testCompile('junit:junit:4.12')
@@ -57,6 +58,7 @@ shadowJar {
     append('LICENSE')
     dependencies {
         include(dependency('org.jdom:jdom2:2.0.6'))
+        include(dependency('org.jdom:jdom2-contrib:2.0.5'))
         relocate 'org.jdom2', 'in.twizmwaz.cardinal.libs.jdom2'
         include(dependency('org.ow2.asm:asm-all:5.0.4'))
         relocate 'org.objectweb.asm', 'in.twizmwaz.cardinal.libs.asm'

gradle/wrapper/gradle-wrapper.properties

@@ -1,4 +1,4 @@
-#Tue Apr 26 23:37:45 EDT 2016
+#Thu Sep 22 02:07:06 CEST 2016
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME

src/main/java/in/twizmwaz/cardinal/module/ModuleError.java

@@ -26,14 +26,46 @@
 package in.twizmwaz.cardinal.module;
 
 import in.twizmwaz.cardinal.module.repository.LoadedMap;
+import lombok.AllArgsConstructor;
 import lombok.Data;
+import org.jdom2.Attribute;
+import org.jdom2.Element;
+import org.jdom2.contrib.input.LineNumberElement;
 
 @Data
+@AllArgsConstructor
 public final class ModuleError {
 
   private final Module module;
   private final LoadedMap map;
   private final String[] message;
   private final boolean critical;
 
+  public ModuleError(Module module, LoadedMap map, boolean critical, String... message) {
+    this(module, map, message, critical);
+  }
+
+  public ModuleError(Module module, LoadedMap map, Element element, String message, boolean critical) {
+    this(module, map, critical, getErrorLocation(element), message);
+  }
+
+  public ModuleError(Module module, LoadedMap map, Attribute attr, String message, boolean critical) {
+    this(module, map, critical, getErrorLocation(attr), message);
+  }
+
+  private static String getErrorLocation(Element element) {
+    String result = "'" + element.getName() + "' element";
+    if (element instanceof LineNumberElement) {
+      LineNumberElement lineElement = (LineNumberElement) element;
+      result += lineElement.getStartLine() == lineElement.getEndLine()
+          ? " on line " + lineElement.getStartLine()
+          : " from line " + lineElement.getStartLine() + " to " + lineElement.getEndLine();
+    }
+    return result;
+  }
+
+  private static String getErrorLocation(Attribute attribute) {
+    return "'" + attribute.getName() + "' attribute in " + getErrorLocation(attribute.getParent());
+  }
+
 }

src/main/java/in/twizmwaz/cardinal/module/ModuleReporter.java

@@ -0,0 +1,173 @@
+package in.twizmwaz.cardinal.module;
+
+import in.twizmwaz.cardinal.match.Match;
+import in.twizmwaz.cardinal.module.id.IdModule;
+import in.twizmwaz.cardinal.module.repository.LoadedMap;
+import in.twizmwaz.cardinal.util.ParseUtil;
+import in.twizmwaz.cardinal.util.document.XML;
+import lombok.Getter;
+import lombok.NonNull;
+import lombok.RequiredArgsConstructor;
+import org.jdom2.Attribute;
+import org.jdom2.Element;
+
+@RequiredArgsConstructor
+public class ModuleReporter {
+
+  private final Module module;
+  private final LoadedMap map;
+  @Getter
+  private final Match match;
+
+  public ModuleReporter(Module module, Match match) {
+    this(module, match.getMap(), match);
+  }
+
+  @Getter
+  private boolean canLoad = false;
+
+  public void reset() {
+    canLoad = true;
+  }
+
+  /**
+   * Gets an attribute value from an element. If it fails to get a value, en error will be thrown.
+   * @param element The element to get the attribute from.
+   * @param attr The attribute to get.
+   * @param type The class of the type to convert the attribute to.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getAttr(Element element, String attr, Class<T> type) {
+    return getAttr(element, attr, type, null, true);
+  }
+
+  /**
+   * Gets an attribute value from an element. Defaults to fallback if not found.
+   * @param element The element to get the attribute from.
+   * @param attr The attribute to get.
+   * @param type The class of the type to convert the attribute to.
+   * @param fallback The fallback in case it's null or an error is thrown, null can be used as fallback.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getAttr(Element element, String attr, Class<T> type, T fallback) {
+    return getAttr(element, attr, type, fallback, false);
+  }
+
+  private <T> T getAttr(Element element, String attr, Class<T> type, T fallback, boolean required) {
+    Attribute attribute = element.getAttribute(attr);
+    if (required && attribute == null) {
+      module.getErrors().add(new ModuleError(module, map, element, "Missing required attribute:'" + attr + "'", false));
+      return null;
+    }
+    T result = getAttribute(attribute, type);
+    if (required && result == null) {
+      canLoad = false;
+    }
+    return result != null ? result : fallback;
+  }
+
+  private <T> T getAttribute(Attribute attr, Class<T> type) {
+    try {
+      return XML.getAttribute(attr, type);
+    } catch (Exception e) {
+      module.getErrors().add(new ModuleError(module, map, attr, e.getMessage(), false));
+    }
+    return null;
+  }
+
+  /**
+   * Gets an attribute value from an element. If it fails to get a value, en error will be thrown.
+   * @param element The element to get the object from.
+   * @param type The class of the type to convert the element to.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getEl(Element element, Class<T> type) {
+    return getEl(element, type, null, true);
+  }
+
+  /**
+   * Gets an attribute value from an element.
+   * @param element The element to get the object from.
+   * @param type The class of the type to convert the element to.
+   * @param fallback The fallback in case it's null or an error is thrown, null can be used as fallback.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getEl(@NonNull Element element, Class<T> type, T fallback) {
+    return getEl(element, type, fallback, false);
+  }
+
+  private <T> T getEl(@NonNull Element element, Class<T> type, T fallback, boolean required) {
+    T result = getElement(element, type);
+    if (required && result == null) {
+      canLoad = false;
+    }
+    return result != null ? result : fallback;
+  }
+
+  private <T> T getElement(Element el, Class<T> type) {
+    try {
+      return XML.getElementObject(el, type);
+    } catch (Exception e) {
+      module.getErrors().add(new ModuleError(module, map, el, e.getMessage(), false));
+    }
+    return null;
+  }
+
+  /**
+   * Gets a property value from an element. It will try to get the object from the attribute,
+   * if null, from child element.
+   * @param element The element to get the property from.
+   * @param prop The property to get.
+   * @param type The class of the type to convert the property to.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getProp(Element element, String prop, Class<T> type) {
+    return getProp(element, prop, type, null, true);
+  }
+
+  /**
+   * Gets a property value from an element. It will try to get the object from the attribute,
+   * if missing, from child element.
+   * @param element The element to get the property from.
+   * @param prop The property to get.
+   * @param type The class of the type to convert the property to.
+   * @param fallback The fallback in case it's null or an error is thrown, null can be used as fallback.
+   * @param <T> The type to convert the attribute value to.
+   * @return An object of the correct type, or null if argument is missing or invalid.
+   */
+  public <T> T getProp(Element element, String prop, Class<T> type, T fallback) {
+    return getProp(element, prop, type, fallback, false);
+  }
+
+  private  <T> T getProp(Element element, String prop, Class<T> type, T fallback, boolean required) {
+    T result;
+    Attribute attribute = element.getAttribute(prop);
+    Element child = element.getChild(prop);
+    if (attribute == null && child == null && required) {
+      module.getErrors().add(new ModuleError(module, map, element, "Missing required property:'" + prop + "'", false));
+      return null;
+    }
+    if (attribute != null) {
+      result = getAttribute(attribute, type);
+    } else {
+      result = getElement(element, type);
+    }
+    if (required && result == null) {
+      canLoad = false;
+    }
+    return result != null ? result : fallback;
+  }
+
+  public void checkId(Attribute attribute, String id) {
+    if (!IdModule.get().canAdd(match, id)) {
+      module.getErrors().add(new ModuleError(module, map, attribute, "Invalid or duplicated ID specified", false));
+      canLoad = false;
+    }
+  }
+
+}

src/main/java/in/twizmwaz/cardinal/module/id/IdModule.java

@@ -96,6 +96,16 @@ public boolean add(Match match, String id, Object object, boolean force) {
     return false;
   }
 
+  /**
+   * Checks if the id could be added to the match
+   * @param match The match to try the object to.
+   * @param id The id to check.
+   * @return if the object isn't null and isn't duplicated.
+   */
+  public boolean canAdd(Match match, String id) {
+    return id != null && !ids.get(match).containsKey(id);
+  }
+
   /**
    * Get an object with the given id.
    * @param match The match the object belongs to.

src/main/java/in/twizmwaz/cardinal/module/objective/Objective.java

@@ -38,7 +38,6 @@ public abstract class Objective {
 
   @NonNull
   protected final Match match;
-  private final String id;
   private final boolean required;
   protected final boolean show;
 

src/main/java/in/twizmwaz/cardinal/module/objective/core/Core.java

@@ -83,7 +83,6 @@ public class Core extends Objective implements OwnedObjective, EntryUpdater {
 
   /**
    * @param match               The match the core is part of.
-   * @param id                  The core's ID, for usage in code and XML.
    * @param name                The core's name, for usage by the user.
    * @param required            Determines if this objective is required to win the match.
    * @param region              The region that contains this core.
@@ -96,10 +95,10 @@ public class Core extends Objective implements OwnedObjective, EntryUpdater {
    * @param proximityHorizontal Determines if only horizontal distance is considered when
    *                            calculating proximity.
    */
-  public Core(Match match, String id, String name, boolean required, Region region, int leak,
+  public Core(Match match, String name, boolean required, Region region, int leak,
               MaterialPattern material, Team owner, boolean modeChanges,
               boolean show, ProximityMetric proximityMetric, boolean proximityHorizontal) {
-    super(match, id, required, show);
+    super(match, required, show);
     this.name = name;
     this.leak = leak;
     this.material = material;

src/main/java/in/twizmwaz/cardinal/module/objective/core/CoreModule.java

@@ -170,7 +170,7 @@ public boolean loadMatch(Match match) {
       boolean proximityHorizontal = proximityHorizontalValue != null
           && Numbers.parseBoolean(proximityHorizontalValue);
 
-      Core core = new Core(match, id, name, required, region, leak, material, team, modeChanges, show,
+      Core core = new Core(match, name, required, region, leak, material, team, modeChanges, show,
           proximityMetric, proximityHorizontal);
       if (!IdModule.get().add(match, id, core)) {
         errors.add(new ModuleError(this, match.getMap(),

src/main/java/in/twizmwaz/cardinal/module/objective/destroyable/Destroyable.java

@@ -103,7 +103,6 @@ public class Destroyable extends Objective implements OwnedObjective, EntryUpdat
 
   /**
    * @param match               The match the destroyable belongs to.
-   * @param id                  This destroyable's ID.
    * @param name                This destroyable's name.
    * @param required            Determines if this objective is required to win the match.
    * @param region              The region that contains this destroyable.
@@ -119,12 +118,12 @@ public class Destroyable extends Objective implements OwnedObjective, EntryUpdat
    * @param proximityHorizontal Determines if only horizontal distance is considered when
    *                            calculating proximity.
    */
-  public Destroyable(Match match, String id, String name, boolean required, Region region,
+  public Destroyable(Match match, String name, boolean required, Region region,
                      MaterialPattern materials, Team owner,
                      double completion, boolean modeChanges, boolean showProgress,
                      boolean repairable, boolean sparks, boolean show,
                      ProximityMetric proximityMetric, boolean proximityHorizontal) {
-    super(match, id, required, show);
+    super(match, required, show);
     this.name = name;
     this.materials = materials;
     this.owner = owner;

src/main/java/in/twizmwaz/cardinal/module/objective/destroyable/DestroyableModule.java

@@ -164,7 +164,7 @@ public boolean loadMatch(Match match) {
         boolean proximityHorizontal = proximityHorizontalValue != null
             && Numbers.parseBoolean(proximityHorizontalValue);
 
-        Destroyable destroyable = new Destroyable(match, id, name, required, region, materials, owner,
+        Destroyable destroyable = new Destroyable(match, name, required, region, materials, owner,
             completion, modeChanges, showProgress, repairable, sparks, show, proximityMetric, proximityHorizontal);
         if (!IdModule.get().add(match, id, destroyable)) {
           errors.add(new ModuleError(this, match.getMap(),

src/main/java/in/twizmwaz/cardinal/module/objective/wool/Wool.java

@@ -83,9 +83,9 @@ public class Wool extends Objective implements Listener, EntryUpdater {
    * @param monumentProximityRule       The proximity rule that determines how to calculate proximity
    *                                    after picking up the wool.
    */
-  public Wool(Match match, String id, boolean required, Team team, DyeColor color, Region monument, boolean craftable,
+  public Wool(Match match, boolean required, Team team, DyeColor color, Region monument, boolean craftable,
               boolean show, Vector location, ProximityRule woolProximityRule, ProximityRule monumentProximityRule) {
-    super(match, id, required, show);
+    super(match, required, show);
     this.team = team;
     this.color = color;
     this.monument = monument;