[BREAKING] Move several methods from MinecraftVersion to GenericVersion.

This breaks testing for UNKNOWN_VERSION, if that is used externally.
Access methods are added for testing for unknown versions.

PR mentioning access methods:
94c4da3267

NCPCompatBukkit/src/main/java/fr/neatmonster/nocheatplus/compat/cbreflect/MCAccessCBReflect.java

@@ -9,6 +9,7 @@ import fr.neatmonster.nocheatplus.compat.bukkit.BlockCacheBukkit;
 import fr.neatmonster.nocheatplus.compat.bukkit.MCAccessBukkitBase;
 import fr.neatmonster.nocheatplus.compat.cbreflect.reflect.ReflectHelper;
 import fr.neatmonster.nocheatplus.compat.cbreflect.reflect.ReflectHelper.ReflectFailureException;
+import fr.neatmonster.nocheatplus.compat.versions.GenericVersion;
 import fr.neatmonster.nocheatplus.compat.versions.ServerVersion;
 import fr.neatmonster.nocheatplus.logging.Streams;
 import fr.neatmonster.nocheatplus.utilities.BlockCache;
@@ -27,22 +28,22 @@ public class MCAccessCBReflect extends MCAccessBukkitBase {
         helper = new ReflectHelper();
         // Version Envelope tests (1.4.5-R1.0 ... 1.8.x is considered to be ok).
         final String mcVersion = ServerVersion.getMinecraftVersion();
-        if (mcVersion == ServerVersion.UNKNOWN_VERSION) {
+        if (mcVersion == GenericVersion.UNKNOWN_VERSION) {
             NCPAPIProvider.getNoCheatPlusAPI().getLogManager().warning(Streams.INIT, "The Minecraft version could not be detected, Compat-CB-Reflect might or might not work.");
             this.knownSupportedVersion = false;
         }
-        else if (ServerVersion.compareVersions(mcVersion, "1.4.5") < 0) {
+        else if (GenericVersion.compareVersions(mcVersion, "1.4.5") < 0) {
             NCPAPIProvider.getNoCheatPlusAPI().getLogManager().warning(Streams.INIT, "The Minecraft version seems to be older than what Compat-CB-Reflect can support.");
             this.knownSupportedVersion = false;
         }
-        else if (ServerVersion.compareVersions(mcVersion, "1.9") >= 0) {
+        else if (GenericVersion.compareVersions(mcVersion, "1.9") >= 0) {
             this.knownSupportedVersion = false;
             NCPAPIProvider.getNoCheatPlusAPI().getLogManager().warning(Streams.INIT, "The Minecraft version seems to be more recent than the one Compat-CB-Reflect has been built with - this might work, but there could be incompatibilities.");
         } else {
             this.knownSupportedVersion = true;
         }
         // Fall damage / event. TODO: Tests between 1.8 and 1.7.2. How about spigot vs. CB?
-        if (mcVersion == ServerVersion.UNKNOWN_VERSION || ServerVersion.compareVersions(mcVersion, "1.8") < 0) {
+        if (mcVersion == GenericVersion.UNKNOWN_VERSION || GenericVersion.compareVersions(mcVersion, "1.8") < 0) {
             dealFallDamageFiresAnEvent = false;
         } else {
             // Assume higher versions to fire an event.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/compat/versions/Bugs.java

@@ -32,14 +32,14 @@ public class Bugs {
         }
         
         // First move exploit (classic CraftBukkit or Spigot before 1.7.5). 
-        if (mcVersion == ServerVersion.UNKNOWN_VERSION) {
+        if (mcVersion == GenericVersion.UNKNOWN_VERSION) {
             // Assume something where it's not an issue.
             enforceLocation = false;
         }
-        else if (ServerVersion.compareVersions(mcVersion, "1.8") >= 0) {
+        else if (GenericVersion.compareVersions(mcVersion, "1.8") >= 0) {
             // Assume Spigot + fixed.
             enforceLocation = false;
-        } else if (serverVersion.indexOf("spigot") >= 0 && ServerVersion.compareVersions(mcVersion, "1.7.5") >= 0) {
+        } else if (serverVersion.indexOf("spigot") >= 0 && GenericVersion.compareVersions(mcVersion, "1.7.5") >= 0) {
             // Fixed in Spigot just before 1.7.5.
             enforceLocation = false;
         } else if (serverVersion.indexOf("craftbukkit") != 0){

NCPCore/src/main/java/fr/neatmonster/nocheatplus/compat/versions/BukkitVersion.java

@@ -4,10 +4,12 @@ import org.bukkit.Bukkit;
 import org.bukkit.Server;
 
 /**
- * Bukkit-specific static access API, adding initialization methods. Note that Bukkit.getVersion() should be used to get the version of Bukkit (...).
+ * Bukkit-specific static access API, adding initialization methods. After init,
+ * all the methods in ServerVersion can be used.
  * <hr/>
  * Taken from the TrustCore plugin.
- * @author mc_dev
+ * 
+ * @author asofold
  *
  */
 public class BukkitVersion {
@@ -25,6 +27,7 @@ public class BukkitVersion {
         }
         // Initialize server version.
         final Server server = Bukkit.getServer();
+        // Note that Bukkit.getVersion() should be used to get the version of Bukkit (...)
         ServerVersion.setMinecraftVersion(ServerVersion.parseMinecraftVersion(server.getBukkitVersion(), server.getVersion()));
 
         // Test availability/reliability of certain features.
@@ -35,16 +38,16 @@ public class BukkitVersion {
         String mcVersion = ServerVersion.getMinecraftVersion();
         int cmp = -1;
         try {
-            cmp = ServerVersion.compareVersions(mcVersion, "1.7.5");
+            cmp = GenericVersion.compareVersions(mcVersion, "1.7.5");
         } catch (IllegalArgumentException e) {
-            mcVersion = ServerVersion.UNKNOWN_VERSION; // Fake but somewhat true :p.
+            mcVersion = GenericVersion.UNKNOWN_VERSION; // Fake but somewhat true :p.
         }
-        if (ServerVersion.UNKNOWN_VERSION.equals(mcVersion)) {
+        if (GenericVersion.UNKNOWN_VERSION.equals(mcVersion)) {
             // TODO: Might test for features.
         } else if (cmp == 1) {
             uuidOnline = true;
             uuidOffline = true;
-        } else if (ServerVersion.compareVersions(mcVersion, "1.7") == 1) {
+        } else if (GenericVersion.compareVersions(mcVersion, "1.7") == 1) {
             // 1.7.x
             uuidOnline = true; // TODO: Test, if REALLY.
             uuidOffline = false;
@@ -58,6 +61,7 @@ public class BukkitVersion {
 
     /**
      * Test if Player.getUniqueId be used for online players.
+     * 
      * @return
      */
     public static boolean uniqueIdOnline() {
@@ -69,10 +73,12 @@ public class BukkitVersion {
     }
 
     /**
-     * Test if OfflinePlayer.getUniqueId can be used for offline payers [provided it exists :p].<br>
+     * Test if OfflinePlayer.getUniqueId can be used for offline payers
+     * [provided it exists :p].<br>
      * Not sure this makes sense :p.
      * <hr>
      * IMPORTANT NOTE: DO NOT MIX UP WITH OFFLINE MODE!<br>
+     * 
      * @return
      */
     public static boolean uniqueIdOffline() {
@@ -85,6 +91,7 @@ public class BukkitVersion {
 
     /**
      * Delegating to ServerVersion.getMinecraftVersion.
+     * 
      * @return
      */
     public static String getMinecraftVersion() {

NCPCore/src/main/java/fr/neatmonster/nocheatplus/compat/versions/GenericVersion.java

@@ -0,0 +1,144 @@
+package fr.neatmonster.nocheatplus.compat.versions;
+
+/**
+ * Some generic version parsing and comparison utility methods.
+ * @author asofold
+ *
+ */
+public class GenericVersion {
+
+    /**
+     * The unknown version, meant to work with '=='. All internal version
+     * parsing will return this for unknown versions.
+     */
+    public static final String UNKNOWN_VERSION = "unknown";
+
+    /**
+     * Access method meant to stay, test vs. UNKNOWN_VERSION with equals.
+     * 
+     * @param version
+     * @return
+     */
+    public static boolean isVersionUnknown(String version) {
+        return UNKNOWN_VERSION.equals(version);
+    }
+
+    /**
+     * Split a version tag consisting of integers and dots between the numbers
+     * into an int array. Not fail safe, may throw NumberFormatException.
+     * 
+     * @param version
+     * @return
+     */
+    public static int[] versionToInt(String version) {
+        String[] split = version.split("\\.");
+        int[] num = new int[split.length];
+        for (int i = 0; i < split.length; i++) {
+            num[i] = Integer.parseInt(split[i]);
+        }
+        return num;
+    }
+
+    /**
+     * Simple x.y.z versions. Returns 0 on equality, -1 if version 1 is smaller
+     * than version 2, 1 if version 1 is greater than version 2.
+     * 
+     * @param version1
+     *            Can be unknown.
+     * @param version2
+     *            Must not be unknown.
+     * @return
+     */
+    public static int compareVersions(String version1, String version2) {
+        if (version2.equals(UNKNOWN_VERSION)) {
+            throw new IllegalArgumentException("version2 must not be 'unknown'.");
+        } else if (version1.equals(UNKNOWN_VERSION)) {
+            // Assume smaller than any given version.
+            return -1;
+        }
+        if (version1.equals(version2)) {
+            return 0;
+        }
+        try {
+            int[] v1Int = versionToInt(version1);
+            int[] v2Int = versionToInt(version2);
+            for (int i = 0; i < Math.min(v1Int.length, v2Int.length); i++) {
+                if (v1Int[i] < v2Int[i]) {
+                    return -1;
+                } else if (v1Int[i] > v2Int[i]) {
+                    return 1;
+                }
+            }
+            // Support sub-sub-sub-...-....-marines.
+            if (v1Int.length < v2Int.length) {
+                return -1;
+            }
+            else if (v1Int.length > v2Int.length) {
+                return 1;
+            }
+        } catch (NumberFormatException e) {}
+
+        // Equality was tested above, so it would seem.
+        throw new IllegalArgumentException("Bad version input.");
+    }
+
+    /**
+     * Exact case, no trim.
+     * 
+     * @param input
+     * @param prefix
+     * @param suffix
+     * @return
+     */
+    protected static String parseVersionDelimiters(String input, String prefix, String suffix) {
+        int preIndex = prefix.isEmpty() ? 0 : input.indexOf(prefix);
+        if (preIndex != -1) {
+            String candidate = input.substring(preIndex + prefix.length());
+            int postIndex = suffix.isEmpty() ? candidate.length() : candidate.indexOf(suffix);
+            if (postIndex != -1) {
+                return GenericVersion.collectVersion(candidate.substring(0, postIndex).trim(), 0);
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Collect a version of the type X.Y.Z with X, Y, Z being numbers. Demands
+     * at least one number, but allows an arbitrary amount of sections X....Y.
+     * Rigid character check, probably not fit for snapshots.
+     * 
+     * @param input
+     * @param beginIndex
+     * @return null if not successful.
+     */
+    protected static String collectVersion(String input, int beginIndex) {
+        StringBuilder buffer = new StringBuilder(128);
+        // Rigid scan by character.
+        boolean numberFound = false;
+        char[] chars = input.toCharArray();
+        for (int i = beginIndex; i < input.length(); i++) {
+            char c = chars[i];
+            if (c == '.') {
+                if (numberFound) {
+                    // Reset, expecting a number to follow.
+                    numberFound = false;
+                } else {
+                    //  Failure.
+                    return null;
+                }
+            } else if (!Character.isDigit(c)) {
+                // Failure.
+                return null;
+            } else {
+                numberFound = true;
+            }
+            buffer.append(c);
+        }
+        if (numberFound) {
+            return buffer.toString();
+        } else {
+            return null;
+        }
+    }
+
+}

NCPCore/src/main/java/fr/neatmonster/nocheatplus/compat/versions/ServerVersion.java

@@ -12,14 +12,19 @@ package fr.neatmonster.nocheatplus.compat.versions;
  */
 public class ServerVersion {
 
-    public static final String UNKNOWN_VERSION = "unknown";
+    private static String minecraftVersion = GenericVersion.UNKNOWN_VERSION; 
-
-    private static String minecraftVersion = UNKNOWN_VERSION; 
 
     private static final String[][] versionTagsMatch = {
         {"1.7.2-r", "1.7.2"}, // Example. Probably this method will just be removed.
     };
 
+    /**
+     * Test if the set Minecraft version is unknown.
+     */
+    public static boolean isMinecraftVersionUnknown() {
+        return GenericVersion.isVersionUnknown(minecraftVersion);
+    }
+
     /**
      * Attempt to return the Minecraft version for a given server version
      * string.
@@ -32,7 +37,7 @@ public class ServerVersion {
         for (String serverVersion : versionCandidates) {
             serverVersion = serverVersion.trim();
             for (String minecraftVersion : new String[]{
-                    collectVersion(serverVersion, 0),
+                    GenericVersion.collectVersion(serverVersion, 0),
                     parseMinecraftVersionGeneric(serverVersion),
                     parseMinecraftVersionTokens(serverVersion)
             }) {
@@ -52,7 +57,7 @@ public class ServerVersion {
      * @return
      */
     private static boolean validateMinecraftVersion(String minecraftVersion) {
-        return collectVersion(minecraftVersion, 0) != null;
+        return GenericVersion.collectVersion(minecraftVersion, 0) != null;
     }
 
     /**
@@ -71,45 +76,6 @@ public class ServerVersion {
         return null;
     }
 
-    /**
-     * Collect a version of the type X.Y.Z with X, Y, Z being numbers. Demands
-     * at least one number, but allows an arbitrary amount of sections X....Y.
-     * Rigid character check, probably not fit for snapshots.
-     * 
-     * @param input
-     * @param beginIndex
-     * @return null if not successful.
-     */
-    private static String collectVersion(String input, int beginIndex) {
-        StringBuilder buffer = new StringBuilder(128);
-        // Rigid scan by character.
-        boolean numberFound = false;
-        char[] chars = input.toCharArray();
-        for (int i = beginIndex; i < input.length(); i++) {
-            char c = chars[i];
-            if (c == '.') {
-                if (numberFound) {
-                    // Reset, expecting a number to follow.
-                    numberFound = false;
-                } else {
-                    //  Failure.
-                    return null;
-                }
-            } else if (!Character.isDigit(c)) {
-                // Failure.
-                return null;
-            } else {
-                numberFound = true;
-            }
-            buffer.append(c);
-        }
-        if (numberFound) {
-            return buffer.toString();
-        } else {
-            return null;
-        }
-    }
-
     /**
      * 
      * @param serverVersion
@@ -118,10 +84,10 @@ public class ServerVersion {
     private static String parseMinecraftVersionGeneric(String serverVersion) {
         String lcServerVersion = serverVersion.trim().toLowerCase();
         for (String candidate : new String[] {
-                parseVersionDelimiters(lcServerVersion, "(mc:", ")"),
+                GenericVersion.parseVersionDelimiters(lcServerVersion, "(mc:", ")"),
-                parseVersionDelimiters(lcServerVersion, "mcpc-plus-", "-"),
+                GenericVersion.parseVersionDelimiters(lcServerVersion, "mcpc-plus-", "-"),
-                parseVersionDelimiters(lcServerVersion, "git-bukkit-", "-r"),
+                GenericVersion.parseVersionDelimiters(lcServerVersion, "git-bukkit-", "-r"),
-                parseVersionDelimiters(lcServerVersion, "", "-r"),
+                GenericVersion.parseVersionDelimiters(lcServerVersion, "", "-r"),
                 // TODO: Other server mods + custom builds !?.
         }) {
             if (candidate != null) {
@@ -131,26 +97,6 @@ public class ServerVersion {
         return null;
     }
 
-    /**
-     * Exact case, no trim.
-     * 
-     * @param input
-     * @param prefix
-     * @param suffix
-     * @return
-     */
-    private static String parseVersionDelimiters(String input, String prefix, String suffix) {
-        int preIndex = prefix.isEmpty() ? 0 : input.indexOf(prefix);
-        if (preIndex != -1) {
-            String candidate = input.substring(preIndex + prefix.length());
-            int postIndex = suffix.isEmpty() ? candidate.length() : candidate.indexOf(suffix);
-            if (postIndex != -1) {
-                return collectVersion(candidate.substring(0, postIndex).trim(), 0);
-            }
-        }
-        return null;
-    }
-
     /**
      * Sets lower-case Minecraft version - or UNKNOWN_VERSION, if null or empty.
      * 
@@ -160,11 +106,11 @@ public class ServerVersion {
      */
     public static String setMinecraftVersion(String version) {
         if (version == null) {
-            minecraftVersion = UNKNOWN_VERSION;
+            minecraftVersion = GenericVersion.UNKNOWN_VERSION;
         } else {
             version = version.trim().toLowerCase();
             if (version.isEmpty()) {
-                minecraftVersion = UNKNOWN_VERSION;
+                minecraftVersion = GenericVersion.UNKNOWN_VERSION;
             } else {
                 minecraftVersion = version;
             }
@@ -180,49 +126,6 @@ public class ServerVersion {
         return minecraftVersion;
     }
 
-    /**
-     * Simple x.y.z versions. Returns 0 on equality, -1 if version 1 is smaller
-     * than version 2, 1 if version 1 is greater than version 2.
-     * 
-     * @param version1
-     *            Can be unknown.
-     * @param version2
-     *            Must not be unknown.
-     * @return
-     */
-    public static int compareVersions(String version1, String version2) {
-        if (version2.equals(UNKNOWN_VERSION)) {
-            throw new IllegalArgumentException("version2 must not be 'unknown'.");
-        } else if (version1.equals(UNKNOWN_VERSION)) {
-            // Assume smaller than any given version.
-            return -1;
-        }
-        if (version1.equals(version2)) {
-            return 0;
-        }
-        try {
-            int[] v1Int = versionToInt(version1);
-            int[] v2Int = versionToInt(version2);
-            for (int i = 0; i < Math.min(v1Int.length, v2Int.length); i++) {
-                if (v1Int[i] < v2Int[i]) {
-                    return -1;
-                } else if (v1Int[i] > v2Int[i]) {
-                    return 1;
-                }
-            }
-            // Support sub-sub-sub-...-....-marines.
-            if (v1Int.length < v2Int.length) {
-                return -1;
-            }
-            else if (v1Int.length > v2Int.length) {
-                return 1;
-            }
-        } catch (NumberFormatException e) {}
-
-        // Equality was tested above, so it would seem.
-        throw new IllegalArgumentException("Bad version input.");
-    }
-
     /**
      * Convenience for compareVersions(getMinecraftVersion(), version).
      * 
@@ -232,7 +135,7 @@ public class ServerVersion {
      *         Minecraft version is higher.
      */
     public static int compareMinecraftVersion(String toVersion) {
-        return compareVersions(getMinecraftVersion(), toVersion);
+        return GenericVersion.compareVersions(getMinecraftVersion(), toVersion);
     }
 
     /**
@@ -251,35 +154,26 @@ public class ServerVersion {
     public static boolean isMinecraftVersionBetween(String versionLow, boolean includeLow, String versionHigh, boolean includeHigh) {
         final String minecraftVersion = getMinecraftVersion();
         if (includeLow) {
-            if (compareVersions(minecraftVersion, versionLow) == -1) {
+            if (GenericVersion.compareVersions(minecraftVersion, versionLow) == -1) {
                 return false;
             }
         } else {
-            if (compareVersions(minecraftVersion, versionLow) <= 0) {
+            if (GenericVersion.compareVersions(minecraftVersion, versionLow) <= 0) {
                 return false;
             }
         }
         if (includeHigh) {
-            if (compareVersions(minecraftVersion, versionHigh) == 1) {
+            if (GenericVersion.compareVersions(minecraftVersion, versionHigh) == 1) {
                 return false;
             }
         } else {
-            if (compareVersions(minecraftVersion, versionHigh) >= 0) {
+            if (GenericVersion.compareVersions(minecraftVersion, versionHigh) >= 0) {
                 return false;
             }
         }
         return true;
     }
 
-    public static int[] versionToInt(String version) {
-        String[] split = version.split("\\.");
-        int[] num = new int[split.length];
-        for (int i = 0; i < split.length; i++) {
-            num[i] = Integer.parseInt(split[i]);
-        }
-        return num;
-    }
-
     /**
      * Select a value based on the Minecraft version.
      * 
@@ -299,10 +193,10 @@ public class ServerVersion {
      */
     public static <V> V select(final String cmpVersion, final V valueLT, final V valueEQ, final V valueGT, final V valueUnknown) {
         final String mcVersion = ServerVersion.getMinecraftVersion();
-        if (mcVersion == ServerVersion.UNKNOWN_VERSION) {
+        if (mcVersion == GenericVersion.UNKNOWN_VERSION) {
             return valueUnknown;
         } else {
-            final int cmp = ServerVersion.compareVersions(mcVersion, cmpVersion);
+            final int cmp = GenericVersion.compareVersions(mcVersion, cmpVersion);
             if (cmp == 0) {
                 return valueEQ;
             } else if (cmp < 0) {

NCPCore/src/main/java/fr/neatmonster/nocheatplus/players/DataManager.java

@@ -30,6 +30,7 @@ import fr.neatmonster.nocheatplus.checks.access.ICheckConfig;
 import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.checks.combined.CombinedData;
 import fr.neatmonster.nocheatplus.compat.BridgeMisc;
+import fr.neatmonster.nocheatplus.compat.versions.GenericVersion;
 import fr.neatmonster.nocheatplus.compat.versions.ServerVersion;
 import fr.neatmonster.nocheatplus.components.ComponentRegistry;
 import fr.neatmonster.nocheatplus.components.ComponentWithName;
@@ -119,7 +120,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
     public DataManager() {
         instance = this;
         final String version = ServerVersion.getMinecraftVersion();
-        if (ServerVersion.compareVersions(version, "1.8") >= 0 || version.equals("1.7.10") && Bukkit.getServer().getVersion().toLowerCase().indexOf("spigot") != -1) {
+        if (GenericVersion.compareVersions(version, "1.8") >= 0 || version.equals("1.7.10") && Bukkit.getServer().getVersion().toLowerCase().indexOf("spigot") != -1) {
             // Safe to assume Spigot, don't store Player instances.
             playerMap = new PlayerMap(false);
         } else {

NCPPlugin/src/main/java/fr/neatmonster/nocheatplus/NoCheatPlus.java

@@ -52,6 +52,7 @@ import fr.neatmonster.nocheatplus.compat.MCAccess;
 import fr.neatmonster.nocheatplus.compat.MCAccessConfig;
 import fr.neatmonster.nocheatplus.compat.MCAccessFactory;
 import fr.neatmonster.nocheatplus.compat.versions.BukkitVersion;
+import fr.neatmonster.nocheatplus.compat.versions.GenericVersion;
 import fr.neatmonster.nocheatplus.compat.versions.ServerVersion;
 import fr.neatmonster.nocheatplus.components.ComponentRegistry;
 import fr.neatmonster.nocheatplus.components.ComponentWithName;
@@ -772,7 +773,7 @@ public class NoCheatPlus extends JavaPlugin implements NoCheatPlusAPI {
         if (NCPAPIProvider.getNoCheatPlusAPI() == null) {
             NCPAPIProvider.setNoCheatPlusAPI(this);
         }
-        if (ServerVersion.getMinecraftVersion() == ServerVersion.UNKNOWN_VERSION) {
+        if (ServerVersion.getMinecraftVersion() == GenericVersion.UNKNOWN_VERSION) {
             BukkitVersion.init();
         }
         if (!ConfigManager.isInitialized()) {