Use Fabric's newly exposed version APIs for app requirement parsing, expand it to allow wildcards and change load stopping to be a special exception (#1252)

src/main/java/carpet/script/CarpetScriptHost.java

@@ -13,6 +13,7 @@ import carpet.script.exception.ExpressionException;
 import carpet.script.exception.IntegrityException;
 import carpet.script.exception.InternalExpressionException;
 import carpet.script.exception.InvalidCallbackException;
+import carpet.script.exception.LoadException;
 import carpet.script.utils.AppStoreManager;
 import carpet.script.value.EntityValue;
 import carpet.script.value.FunctionValue;
@@ -37,6 +38,7 @@ import net.fabricmc.loader.api.ModContainer;
 import net.fabricmc.loader.api.SemanticVersion;
 import net.fabricmc.loader.api.Version;
 import net.fabricmc.loader.api.VersionParsingException;
+import net.fabricmc.loader.api.metadata.version.VersionPredicate;
 import net.minecraft.entity.Entity;
 import net.minecraft.entity.EntityType;
 import net.minecraft.nbt.NbtElement;
@@ -60,7 +62,6 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.TreeMap;
-import java.util.function.BiPredicate;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -119,8 +120,7 @@ public class CarpetScriptHost extends ScriptHost
                 String code = module.getCode();
                 if (code == null)
                 {
-                    Messenger.m(source, "r Unable to load "+module.getName()+" app - code not found");
-                    return null;
+                    throw new LoadException("Code not found");
                 }
                 host.setChatErrorSnooper(source);
                 CarpetExpression ex = new CarpetExpression(host.main, code, source, new BlockPos(0, 0, 0));
@@ -131,13 +131,12 @@ public class CarpetScriptHost extends ScriptHost
             catch (CarpetExpressionException e)
             {
                 host.handleErrorWithStack("Error while evaluating expression", e);
-                host.resetErrorSnooper();
-                return null;
+                throw new LoadException();
             }
-            catch (ArithmeticException ae)
+            catch (ArithmeticException ae) // is this branch ever reached? Seems like arithmetic exceptions are converted to CEEs earlier
             {
                 host.handleErrorWithStack("Math doesn't compute", ae);
-                return null;
+                throw new LoadException();
             }
             catch (StackOverflowError soe)
             {
@@ -314,7 +313,7 @@ public class CarpetScriptHost extends ScriptHost
             {
                 Value reqResult = callNow((FunctionValue) loadRequirements, Collections.emptyList());
                 if (reqResult.getBoolean()) // != false or null
-                    throw new InternalExpressionException(reqResult.getString());
+                    throw new LoadException(reqResult.getString());
             }
             else
             {
@@ -422,20 +421,6 @@ public class CarpetScriptHost extends ScriptHost
         return addLegacyCommand(notifier);
     }
     
-    // Prefixes based on FLoader's SemanticVersionPredicateParser's class prefixes under Apache 2.0, since that's not public API
-    private static final Map<String, BiPredicate<SemanticVersion, SemanticVersion>> VERSION_PREFIXES = new HashMap<String, BiPredicate<SemanticVersion, SemanticVersion>>() {{
-            put(">=", (target, source) -> source.compareTo(target) >= 0);
-            put("<=", (target, source) -> source.compareTo(target) <= 0);
-            put(">", (target, source) -> source.compareTo(target) > 0);
-            put("<", (target, source) -> source.compareTo(target) < 0);
-            put("=", (target, source) -> source.compareTo(target) == 0);
-            put("~", (target, source) -> source.compareTo(target) >= 0
-                    && source.getVersionComponent(0) == target.getVersionComponent(0)
-                    && source.getVersionComponent(1) == target.getVersionComponent(1));
-            put("^", (target, source) -> source.compareTo(target) >= 0
-                    && source.getVersionComponent(0) == target.getVersionComponent(0));
-    }};
-    
     public void checkModVersionRequirements(Value reqs) {
         if (reqs == null)
             return;
@@ -446,58 +431,25 @@ public class CarpetScriptHost extends ScriptHost
         Map<Value, Value> requirements = ((MapValue)reqs).getMap();
         for (Entry<Value, Value> requirement : requirements.entrySet())
         {
-            boolean successful = false;
             String requiredModId = requirement.getKey().getString();
-            String requirementString = requirement.getValue().getString();
-            
+            String stringPredicate = requirement.getValue().getString();
+            VersionPredicate predicate;
+            try {
+                predicate = VersionPredicate.parse(stringPredicate);
+            } catch (VersionParsingException e) {
+                throw new InternalExpressionException("Failed to parse version conditions for '" + requiredModId + "' in 'requires': " + e.getMessage());
+            }
+
             ModContainer mod = FabricLoader.getInstance().getModContainer(requiredModId).orElse(null);
             if (mod != null)
             {
-                if (requirementString.equals("*"))
-                    continue;
                 Version presentVersion = mod.getMetadata().getVersion();
-                BiPredicate<SemanticVersion, SemanticVersion> tester = null;
-                String remainingRequirementString = requirementString;
-                for (Entry<String, BiPredicate<SemanticVersion, SemanticVersion>> s : VERSION_PREFIXES.entrySet())
-                {
-                    if (requirementString.startsWith(s.getKey()))
-                    {
-                        tester = s.getValue();
-                        remainingRequirementString = requirementString.substring(s.getKey().length());
-                    }
-                }
-                if (tester == null)
-                    tester = VERSION_PREFIXES.get("=");
-                if (presentVersion instanceof SemanticVersion)
-                {
-                    SemanticVersion requiredVersion = null;
-                    try {
-                        requiredVersion = SemanticVersion.parse(remainingRequirementString);
-                    }
-                    catch (VersionParsingException e)
-                    {
-                        throw new InternalExpressionException("Failed to parse semantic version for '" + requiredModId + "' in 'requires': " + e.getMessage());
-                    }
-                    if (tester.test(requiredVersion, (SemanticVersion) presentVersion))
-                        successful = true;
-                }
-                else
-                {
-                    if (FabricLoader.getInstance().isDevelopmentEnvironment())
-                        successful = true; // Autoversioning breaks semver in dev (loads as ${version})
-                    else
-                    {
-                        if (!(tester == VERSION_PREFIXES.get("=")))
-                            throw new InternalExpressionException("Mod '"+requiredModId+"' doesn't use semversion, thus can only check version equality");
-                        if (presentVersion.getFriendlyString().equals(remainingRequirementString))
-                            successful = true;
-                    }
+                if (predicate.test(presentVersion) || (FabricLoader.getInstance().isDevelopmentEnvironment() && !(presentVersion instanceof SemanticVersion)))
+                { // in a dev env, mod version is usually replaced with ${version}, and that isn't semantic
+                    continue;
                 }
             }
-            if (!successful)
-            {
-                throw new InternalExpressionException(getName()+" requires " + requiredModId + " version " + requirementString + " in order to load");
-            }
+            throw new LoadException(String.format("%s requires a version of mod '%s' matching '%s', which is missing!", getName(), requiredModId, stringPredicate));
         }
     }
 

src/main/java/carpet/script/CarpetScriptServer.java

@@ -13,6 +13,7 @@ import carpet.CarpetServer;
 import carpet.script.bundled.FileModule;
 import carpet.script.bundled.Module;
 import carpet.script.exception.ExpressionException;
+import carpet.script.exception.LoadException;
 import carpet.script.language.Arithmetic;
 import carpet.script.language.ControlFlow;
 import carpet.script.language.DataStructures;
@@ -256,10 +257,14 @@ public class CarpetScriptServer
             Messenger.m(source, "r Failed to add "+name+" app");
             return false;
         }
-        CarpetScriptHost newHost = CarpetScriptHost.create(this, module, perPlayer, source, commandValidator, isRuleApp, installer);
-        if (newHost == null)
+        CarpetScriptHost newHost;
+        try
         {
-            Messenger.m(source, "r Failed to add "+name+" app");
+            newHost = CarpetScriptHost.create(this, module, perPlayer, source, commandValidator, isRuleApp, installer);
+        }
+        catch (LoadException e)
+        {
+            Messenger.m(source, "r Failed to add " + name + " app" + (e.getMessage() == null ? "" : ": " + e.getMessage()));
             return false;
         }
         // config needs to be read as we load the app since some events use that info

src/main/java/carpet/script/exception/LoadException.java

@@ -0,0 +1,19 @@
+package carpet.script.exception;
+
+/**
+ * A Scarpet exception that indicates that load of the app has failed.
+ * 
+ * Goes up the stack to the point of app load and gets caught there, preventing the app from loading with
+ * the given message.
+ *
+ */
+public class LoadException extends RuntimeException implements ResolvedException {
+    
+    public LoadException() {
+        super();
+    }
+    
+    public LoadException(String message) {
+        super(message);
+    }
+}