Add better validation for Planar Conquest readers

forge-core/src/main/java/forge/util/collect/FCollectionReader.java

@@ -12,13 +12,24 @@ public abstract class FCollectionReader<T> {
     }
 
     void readAll(FCollection<T> collection) {
-        for (final String line : FileUtil.readFile(file)) {
+        for (String line : FileUtil.readFile(file)) {
-            final T item = read(line);
+            line = line.trim();
+            if (line.isEmpty()) {
+                continue; //ignore blank or whitespace lines
+            }
+
+            T item = read(line);
             if (item != null) {
                 collection.add(item);
             }
         }
     }
 
+    protected void alertInvalidLine(String line, String message) {
+        System.err.println(message);
+        System.err.println(line);
+        System.err.println(file.getPath());
+    }
+
     protected abstract T read(String line);
 }

forge-core/src/main/java/forge/util/storage/StorageReaderFile.java

@@ -36,22 +36,10 @@ import java.util.TreeMap;
 public abstract class StorageReaderFile<T> extends StorageReaderBase<T> {
     protected final File file;
 
-    /**
-     * Instantiates a new storage reader file.
-     *
-     * @param pathname the pathname
-     * @param keySelector0 the key selector0
-     */
     public StorageReaderFile(final String pathname, final Function<? super T, String> keySelector0) {
         this(new File(pathname), keySelector0);
     }
 
-    /**
-     * Instantiates a new storage reader file.
-     *
-     * @param file0 the file0
-     * @param keySelector0 the key selector0
-     */
     public StorageReaderFile(final File file0, final Function<? super T, String> keySelector0) {
         super(keySelector0);
         file = file0;
@@ -62,23 +50,24 @@ public abstract class StorageReaderFile<T> extends StorageReaderBase<T> {
         return file.getPath();
     }
 
-    /* (non-Javadoc)
-     * @see forge.util.IItemReader#readAll()
-     */
     @Override
     public Map<String, T> readAll() {
         final Map<String, T> result = new TreeMap<String, T>();
 
         int idx = 0;
-        for (final String s : FileUtil.readFile(file)) {
+        for (String line : FileUtil.readFile(file)) {
-            if (!lineContainsObject(s)) {
+            line = line.trim();
+            if (line.isEmpty()) {
+                continue; //ignore blank or whitespace lines
+            }  
+
+            if (!lineContainsObject(line)) {
                 continue;
             }
 
-            final T item = read(s, idx);
+            T item = read(line, idx);
-            if (null == item) {
+            if (item == null) {
-                final String msg = "An object stored in " + file.getPath() + " failed to load.\nPlease submit this as a bug with the mentioned file attached.";
+                continue;
-                throw new RuntimeException(msg);
             }
 
             idx++;
@@ -92,31 +81,20 @@ public abstract class StorageReaderFile<T> extends StorageReaderBase<T> {
         return result;
     }
 
-    /**
-     * TODO: Write javadoc for this method.
-     * 
-     * @param line
-     *            the line
-     * @return the t
-     */
     protected abstract T read(String line, int idx);
 
-    /**
-     * Line contains object.
-     * 
-     * @param line
-     *            the line
-     * @return true, if successful
-     */
     protected boolean lineContainsObject(final String line) {
         return !StringUtils.isBlank(line) && !line.trim().startsWith("#");
     }
 
-    /* (non-Javadoc)
-     * @see forge.util.IItemReader#getItemKey(java.lang.Object)
-     */
     @Override
     public String getItemKey(final T item) {
         return keySelector.apply(item);
     }
+
+    protected void alertInvalidLine(String line, String message) {
+        System.err.println(message);
+        System.err.println(line);
+        System.err.println(file.getPath());
+    }
 }

forge-gui/src/main/java/forge/model/CardBlock.java

@@ -238,7 +238,7 @@ public final class CardBlock implements Comparable<CardBlock> {
          */
         @Override
         protected CardBlock read(String line, int i) {
-            final String[] sParts = TextUtil.splitWithParenthesis(line.trim(), ',', 3);
+            final String[] sParts = TextUtil.splitWithParenthesis(line, ',', 3);
             String name = sParts[0];
 
             String[] numbers = sParts[1].trim().split("/");

forge-gui/src/main/java/forge/planarconquest/ConquestEvent.java

@@ -103,11 +103,19 @@ public class ConquestEvent {
             String avatar = null;
             String description = null;
 
-            String[] pieces = line.trim().split("\\|");
+            String key, value;
+            String[] pieces = line.split("\\|");
             for (String piece : pieces) {
-                String[] kv = piece.split(":", 2);
+                int idx = piece.indexOf(':');
-                String key = kv[0].trim().toLowerCase();
+                if (idx != -1) {
-                String value = kv[1].trim();
+                    key = piece.substring(0, idx).trim().toLowerCase();
+                    value = piece.substring(idx + 1).trim();
+                }
+                else {
+                    alertInvalidLine(line, "Invalid event definition.");
+                    key = piece.trim().toLowerCase();
+                    value = "";
+                }
                 switch(key) {
                 case "name":
                     name = value;
@@ -133,6 +141,9 @@ public class ConquestEvent {
                 case "desc":
                     description = value;
                     break;
+                default:
+                    alertInvalidLine(line, "Invalid event definition.");
+                    break;
                 }
             }
             if (deck == null) {

forge-gui/src/main/java/forge/planarconquest/ConquestPlane.java

@@ -243,11 +243,19 @@ public class ConquestPlane {
             String description = null;
             boolean unreachable = false;
 
-            String[] pieces = line.trim().split("\\|");
+            String key, value;
+            String[] pieces = line.split("\\|");
             for (String piece : pieces) {
-                String[] kv = piece.split(":", 2);
+                int idx = piece.indexOf(':');
-                String key = kv[0].trim().toLowerCase();
+                if (idx != -1) {
-                String value = kv[1].trim();
+                    key = piece.substring(0, idx).trim().toLowerCase();
+                    value = piece.substring(idx + 1).trim();
+                }
+                else {
+                    alertInvalidLine(line, "Invalid plane definition.");
+                    key = piece.trim().toLowerCase();
+                    value = "";
+                }
                 switch(key) {
                 case "name":
                     name = value;
@@ -266,6 +274,9 @@ public class ConquestPlane {
                 case "desc":
                     description = value;
                     break;
+                default:
+                    alertInvalidLine(line, "Invalid plane definition.");
+                    break;
                 }
             }
             return new ConquestPlane(name, description, regionSize, unreachable);

forge-gui/src/main/java/forge/planarconquest/ConquestRegion.java

@@ -90,11 +90,19 @@ public class ConquestRegion {
             ColorSet colorSet = ColorSet.ALL_COLORS;
             Predicate<PaperCard> pred = Predicates.alwaysTrue();
 
-            String[] pieces = line.trim().split("\\|");
+            String key, value;
+            String[] pieces = line.split("\\|");
             for (String piece : pieces) {
-                String[] kv = piece.split(":", 2);
+                int idx = piece.indexOf(':');
-                String key = kv[0].trim().toLowerCase();
+                if (idx != -1) {
-                String value = kv[1].trim();
+                    key = piece.substring(0, idx).trim().toLowerCase();
+                    value = piece.substring(idx + 1).trim();
+                }
+                else {
+                    alertInvalidLine(line, "Invalid plane definition.");
+                    key = piece.trim().toLowerCase();
+                    value = "";
+                }
                 switch(key) {
                 case "name":
                     name = value;
@@ -129,6 +137,9 @@ public class ConquestRegion {
                         }
                     };
                     break;
+                default:
+                    alertInvalidLine(line, "Invalid region definition.");
+                    break;
                 }
             }
             return new ConquestRegion(plane, name, artCardName, colorSet, pred);

forge-gui/src/main/java/forge/quest/QuestWorld.java

@@ -18,7 +18,6 @@
 package forge.quest;
 
 import com.google.common.base.Function;
-
 import forge.deck.Deck;
 import forge.game.GameFormat;
 import forge.item.PaperCard;
@@ -131,28 +130,41 @@ public class QuestWorld implements Comparable<QuestWorld>{
             final List<String> sets = new ArrayList<String>();
             final List<String> bannedCards = new ArrayList<String>(); // if both empty, no format
 
-            // This is what you need to use here =>
+            String key, value;
-            // FileSection.parse(line, ":", "|");
+            String[] pieces = line.split("\\|");
-            
+            for (String piece : pieces) {
-            final String[] sParts = line.trim().split("\\|");
+                int idx = piece.indexOf(':');
-
+                if (idx != -1) {
-            for (final String sPart : sParts) {
+                    key = piece.substring(0, idx).trim().toLowerCase();
-                
+                    value = piece.substring(idx + 1).trim();
-                final String[] kv = sPart.split(":", 2);
+                }
-                final String key = kv[0].toLowerCase();
+                else {
-                if ("name".equals(key)) {
+                    alertInvalidLine(line, "Invalid plane definition.");
-                    useName = kv[1];
+                    key = piece.trim().toLowerCase();
-                } else if ("dir".equals(key)) {
+                    value = "";
-                    useDir = kv[1];
+                }
-                } else if ("sets".equals(key)) {
+                switch(key) {
-                    sets.addAll(Arrays.asList(kv[1].split(", ")));
+                case "name":
-                } else if ("banned".equals(key)) {
+                    useName = value;
-                    bannedCards.addAll(Arrays.asList(kv[1].split("; ")));
+                    break;
+                case "dir":
+                    useDir = value;
+                    break;
+                case "sets":
+                    sets.addAll(Arrays.asList(value.split(", ")));
+                    break;
+                case "banned":
+                    bannedCards.addAll(Arrays.asList(value.split("; ")));
+                    break;
+                default:
+                    alertInvalidLine(line, "Invalid quest world definition.");
+                    break;
                 }
             }
 
             if (useName == null) {
-                throw new RuntimeException("A Quest World  must have a name! Check worlds.txt file");
+                alertInvalidLine(line, "Quest world must have a name.");
+                return null;
             }
 
             if (!sets.isEmpty() || !bannedCards.isEmpty()) {