Change how resource type works, allow proper app and library installation (#956)

* Change how app store types work, allow proper app and library installation

* Add node for resources in dependencies and docs

* Fix NPE without `type`

* Fix docs trying to change location of app type resource

* Separate libraries and apps and add trash for them

* Update docs

* Create trash directory if it doesn't exist

docs/scarpet/api/Overview.md

@@ -110,30 +110,46 @@ in a list and contain of map-like resources descriptors, looking like
    'resources' -> [
         {
             'source' -> 'https://raw.githubusercontent.com/gnembon/fabric-carpet/master/src/main/resources/assets/carpet/icon.png',
-            'type' -> 'url',
             'target' -> 'foo/photos.zip/foo/cm.png',
         },
         {
-            'source' -> 'survival/README.md',
+            'source' -> '/survival/README.md',
-            'type' -> 'store',
             'target' -> 'survival_readme.md',
             'shared' -> true,
         },
         {
-            'source' -> 'carpets.sc',
+            'source' -> 'circle.sc', // Relative path
-            'type' -> 'app',
+            'target' -> 'apps/circle.sc', // This won't install the app, use 'libraries' for that
-            'target' -> 'apps/flying_carpets.sc',
-            'shared' -> true,
         },
     ]
    ```
-   `source` and `type` indicate resource location: either an arbitrary url (type `'url'`), 
+   `source` indicates resource location: either an arbitrary url (starting with `http://` or `https://`), 
-   absolute location of a file in the app store (type `'store'`),
+   absolute location of a file in the app store (starting with a slash `/`),
-or a relative location in the same folder as the app in question (type `'app'`). 
+or a relative location in the same folder as the app in question (the relative location directly). 
-`'target'` points to the path in app data, or shared app data folder
+`'target'` points to the path in app data, or shared app data folder. If not specified it will place the app into the main data folder with the name it has.
 if `'shared'` is specified and `true`. When re-downloading the app, all resources will be re-downloaded as well. 
-Currently, app resources
+Currently, app resources are only downloaded when using `/carpet download` command.
-are only downloaded when using `/carpet download` command.
+*   `libraries` - list of libraries or apps to be downloaded when installing the app from the app store. It needs to be a list of map-like resource
+descriptors, like the above `resources` field.
+   ```
+   'libraries' -> [
+        {
+            'source' -> '/tutorial/carpets.sc'
+        },
+        {
+            'source' -> '/fundamentals/heap.sc',
+            'target' -> 'heap-lib.sc'
+        }
+    ]
+   ```
+    `source` indicates resource location and must point to a scarpet app or library. It can be either an arbitrary url (starting with `http://` 
+    or `https://`), absolute location of a file in the app store (starting with a slash `/`), or a relative location in the same folder as the app
+    in question (the relative location directly). 
+    `target` is an optional field indicating the new name of the app. If not specified it will place the app into the main data folder with the name it has.
+If the app has relative resources dependencies, Carpet will use the app's path for relatives if the app was loaded from the same app store, or none if the 
+app was loaded from an external url.
+If you need to `import()` from dependencies indicated in this block, make sure to have the `__config()` map before any import that references your
+remote dependencies, in order to allow them to be downloaded and initialized before the import is executed.
 *   `'arguments'` - defines custom argument types for legacy commands with `'legacy_command_type_support'` as well
 as for the custom commands defined with `'commands'`, see below.
 *   `'commands'` - defines custom commands for the app to be executed with `/<app>` command, see below.

docs/scarpet/language/FunctionsAndControlFlow.md

@@ -141,6 +141,10 @@ but if symbols are used directly in the module body rather than functions, it ma
 Returns full list of available symbols that could be imported from this module, which can be used to debug import 
 issues, and list contents of libraries.
 
+You can load and import functions from dependencies in a remote app store's source specified in your config's `libraries` block, but make sure
+to place your config _before_ the import in order to allow the remote dependency to be downloaded (currently, app resources are only downloaded
+when using the `/carpet download` command).
+
 ### `call(function, ? args ...)`
 
 calls a user defined function with specified arguments. It is equivalent to calling `function(args...)` directly 

src/main/java/carpet/commands/ScriptCommand.java

@@ -110,7 +110,7 @@ public class ScriptCommand
     }
 
     /**
-     * A method to suggest the available scarpet scripts based off of the current player input and {@link AppStoreManager#appStoreRoot}
+     * A method to suggest the available scarpet scripts based off of the current player input and {@link AppStoreManager#APP_STORE_ROOT}
      * variable.
      */
     private static CompletableFuture<Suggestions> suggestDownloadableApps(

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

@@ -70,7 +70,7 @@ import static net.minecraft.server.command.CommandManager.literal;
 public class CarpetScriptHost extends ScriptHost
 {
     private final CarpetScriptServer scriptServer;
-    ServerCommandSource responsibleSource;
+    public ServerCommandSource responsibleSource;
 
     private Tag globalState;
     private int saveTimeout;
@@ -328,6 +328,15 @@ public class CarpetScriptHost extends ScriptHost
                         AppStoreManager.addResource(this, storeSource, resource);
                     }
                 }
+                Value libraries = config.get(new StringValue("libraries"));
+                if (libraries != null)
+                {
+                    if (!(libraries instanceof ListValue)) throw new InternalExpressionException("App libraries not defined as a list");
+                    for (Value library : ((ListValue) libraries).getItems())
+                    {
+                        AppStoreManager.addLibrary(this, storeSource, library);
+                    }
+                }
             }
             appConfig = config;
         }

src/main/java/carpet/script/utils/AppStoreManager.java

@@ -5,11 +5,11 @@ import carpet.script.CarpetScriptHost;
 import carpet.script.CarpetScriptServer;
 import carpet.script.exception.InternalExpressionException;
 import carpet.script.value.MapValue;
+import carpet.script.value.StringValue;
 import carpet.script.value.Value;
 import carpet.settings.ParsedRule;
 import carpet.settings.Validator;
 import carpet.utils.Messenger;
-import com.google.common.collect.ImmutableList;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
@@ -21,7 +21,7 @@ import net.minecraft.util.WorldSavePath;
 import org.apache.commons.lang3.tuple.Triple;
 
 import java.io.BufferedReader;
-import java.io.FileWriter;
+import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -32,7 +32,6 @@ import java.net.URL;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.charset.StandardCharsets;
-import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.Arrays;
 import java.util.Collections;
@@ -41,7 +40,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
-
 /**
  * A class used to save scarpet app store scripts to disk
  */
@@ -50,7 +48,7 @@ public class AppStoreManager
     /** A local copy of the scarpet repo's file structure, to avoid multiple queries to github.com while typing out the
      * {@code /script download} command and getting the suggestions.
      */
-    public static final StoreNode appStoreRoot = StoreNode.folder(null, "");
+    public static final StoreNode APP_STORE_ROOT = StoreNode.folder(null, "");
 
     /** This is the base link to the scarpet app repo from the github api.
      */
@@ -61,8 +59,8 @@ public class AppStoreManager
     {
         @Override public String validate(ServerCommandSource source, ParsedRule<String> currentRule, String newValue, String string)
         {
-            appStoreRoot.sealed = false;
+            APP_STORE_ROOT.sealed = false;
-            appStoreRoot.children = new HashMap<>();
+            APP_STORE_ROOT.children = new HashMap<>();
             if (newValue.equalsIgnoreCase("none"))
             {
                 scarpetRepoLink = null;
@@ -115,7 +113,7 @@ public class AppStoreManager
         }
         private StringBuilder createPrePath()
         {
-            return this == appStoreRoot ? new StringBuilder() : parent.createPrePath().append(pathElement());
+            return this == APP_STORE_ROOT ? new StringBuilder() : parent.createPrePath().append(pathElement());
         }
         private StoreNode(StoreNode parent, String name)
         {
@@ -212,7 +210,7 @@ public class AppStoreManager
     public static List<String> suggestionsFromPath(String currentPath) throws IOException
     {
         String[] path = currentPath.split("/");
-        StoreNode appKiosk = appStoreRoot;
+        StoreNode appKiosk = APP_STORE_ROOT;
         for(String pathElement : path)
         {
             if (appKiosk.cannotContinueFor(pathElement)) break;
@@ -237,6 +235,11 @@ public class AppStoreManager
     public static int downloadScript(ServerCommandSource source, String path)
     {
         Triple<String, String, StoreNode> nodeInfo = getFileNode(path);
+        return downloadScript(source, path, nodeInfo, false);
+    }
+
+    private static int downloadScript(ServerCommandSource source, String path, Triple<String, String, StoreNode> nodeInfo, boolean useTrash)
+    {
         String code;
         try
         {
@@ -246,7 +249,7 @@ public class AppStoreManager
         {
             throw new CommandException(Messenger.c("rb Failed to obtain app file content: "+e.getMessage()));
         }
-        if (!saveScriptToFile(source, path, nodeInfo.getLeft(), code, false)) return 0;
+        if (!saveScriptToFile(source, path, nodeInfo.getLeft(), code, false, useTrash)) return 0;
         boolean success = CarpetServer.scriptServer.addScriptHost(source, nodeInfo.getLeft().replaceFirst("\\.sc$", ""), null, true, false, false, nodeInfo.getRight());
         return success?1:0;
     }
@@ -258,7 +261,7 @@ public class AppStoreManager
      */
     public static Triple<String, String, StoreNode> getFileNode(String appPath)
     {
-        return getFileNodeFrom(appStoreRoot, appPath);
+        return getFileNodeFrom(APP_STORE_ROOT, appPath);
     }
 
     public static Triple<String, String, StoreNode> getFileNodeFrom(StoreNode start, String appPath)
@@ -280,32 +283,46 @@ public class AppStoreManager
     }
 
 
-    public static boolean saveScriptToFile(ServerCommandSource source, String path, String appFileName, String code, boolean globalSavePath){
+    public static boolean saveScriptToFile(ServerCommandSource source, String path, String appFileName, String code, boolean globalSavePath, boolean useTrash)
-        String  scriptPath;
+    {
-        String location;
+        Path scriptLocation;
-        if(globalSavePath && !source.getMinecraftServer().isDedicated())
+        if (globalSavePath && !source.getMinecraftServer().isDedicated()) // never happens, this is always called with globalSavePath being false
         { //cos config folder only is in clients
-            scriptPath = FabricLoader.getInstance().getConfigDir().resolve("carpet/scripts/appstore").toAbsolutePath()+"/"+path;
+            scriptLocation = FabricLoader.getInstance().getConfigDir().resolve("carpet/scripts/appstore").toAbsolutePath().resolve(path);
-            location = "global script config folder";
         }
         else
         {
-            scriptPath = source.getMinecraftServer().getSavePath(WorldSavePath.ROOT).resolve("scripts").toAbsolutePath()+"/"+appFileName;
+            scriptLocation = source.getMinecraftServer().getSavePath(WorldSavePath.ROOT).resolve("scripts").toAbsolutePath().resolve(appFileName);
-            location = "world scripts folder";
         }
         try
         {
-            Path scriptLocation = Paths.get(scriptPath);
             Files.createDirectories(scriptLocation.getParent());
-            if(Files.exists(scriptLocation))
+            if (Files.exists(scriptLocation))
-                Messenger.m(source, String.format("gi Note: replaced existing app '%s'", appFileName));
+            {
-            FileWriter fileWriter = new FileWriter(scriptPath);
+                if (useTrash)
-            fileWriter.write(code);
+                {
-            fileWriter.close();
+                    Files.createDirectories(scriptLocation.getParent().resolve("trash"));
+                    Path trashPath = scriptLocation.getParent().resolve("trash").resolve(path);
+                    int i = 0;
+                    while (Files.exists(trashPath))
+                    {
+                        String[] nameAndExtension = appFileName.split("\\.");
+                        String newFileName = String.format(nameAndExtension[0] + "%02d." + nameAndExtension[1],i);
+                        trashPath = trashPath.getParent().resolve(newFileName);
+                        i++;
+                    }
+                    Files.move(scriptLocation, trashPath);
+                }
+                Messenger.m(source, String.format("gi Note: replaced existing app '%s'"+(useTrash ? " (old moved to /trash folder)" : ""), appFileName));
+            }
+            BufferedWriter writer = Files.newBufferedWriter(scriptLocation);
+            writer.write(code);
+            writer.close();
         }
         catch (IOException e)
         {
-            Messenger.m(source, "r Error in downloading app: "+e.getMessage());
+            Messenger.m(source, "r Error while downloading app: " + e.toString());
+            CarpetScriptServer.LOG.warn("Error while downloading app", e);
             return false;
         }
         return true;
@@ -348,42 +365,32 @@ public class AppStoreManager
             Files.copy(in, destination, StandardCopyOption.REPLACE_EXISTING);
         }
     }
+    
+    private static String getFullContentUrl(String original, StoreNode storeSource)
+    {
+        if (original.matches("^https?://.*$")) // We've got a full url here: Just use it
+            return original;
+        if (original.charAt(0) == '/') // We've got an absolute path: Use app store root
+            return getFileNode(original.substring(1)).getMiddle();
+        return getFileNodeFrom(storeSource, original).getMiddle(); // Relative path: Use download location
+    }
 
     public static void addResource(CarpetScriptHost carpetScriptHost, StoreNode storeSource, Value resource)
     {
         if (!(resource instanceof MapValue))
             throw new InternalExpressionException("This is not a valid resource map: "+resource.getString());
         Map<String, Value> resourceMap = ((MapValue) resource).getMap().entrySet().stream().collect(Collectors.toMap(e -> e.getKey().getString(), Map.Entry::getValue));
-        for (String key: ImmutableList.of("source", "target", "type"))
+        if (!resourceMap.containsKey("source")) throw new InternalExpressionException("Missing 'source' field in resource descriptor: "+resource.getString());
-            if (!resourceMap.containsKey(key)) throw new InternalExpressionException("Missing '"+key+"' field in resource descriptor: "+resource.getString());
         String source = resourceMap.get("source").getString();
-        String target = resourceMap.get("target").getString();
+        String contentUrl = getFullContentUrl(source, storeSource);
+        String target = resourceMap.computeIfAbsent("target", k -> new StringValue(contentUrl.substring(contentUrl.lastIndexOf('/') + 1))).getString();
         boolean shared = resourceMap.getOrDefault("shared", Value.FALSE).getBoolean();
-        String type = resourceMap.get("type").getString();
+
-        String contentUrl;
-        if (type.equalsIgnoreCase("url"))
-        {
-            contentUrl = source;
-        }
-        else if (type.equalsIgnoreCase("store"))
-        {
-            Triple<String, String, StoreNode> nodeInfo = getFileNode(source);
-            contentUrl = nodeInfo.getMiddle();
-        }
-        else if (type.equalsIgnoreCase("app"))
-        {
-            Triple<String, String, StoreNode> nodeInfo = getFileNodeFrom(storeSource, source);
-            contentUrl = nodeInfo.getMiddle();
-        }
-        else
-        {
-            throw new InternalExpressionException("Incorrect source type, expecting 'url', 'store' or 'app', got "+type);
-        }
         if (!carpetScriptHost.applyActionForResource(target, shared, p -> {
             try {
                 writeUrlToFile(contentUrl, p);
             } catch (IOException e) {
-                throw new InternalExpressionException("Unable to write resource "+target+": "+e.getMessage());
+                throw new InternalExpressionException("Unable to write resource "+target+": "+e.toString());
             }
         }))
         {
@@ -391,4 +398,56 @@ public class AppStoreManager
         }
         CarpetScriptServer.LOG.info("Downloaded resource "+target+" from "+contentUrl);
     }
+
+    /**
+     * Gets a new StoreNode for an app's dependency with proper relativeness. Will be null if it comes from an external URL
+     * @param originalSource The StoreNode from the container's app
+     * @param sourceString The string the app specified as source
+     * @param contentUrl The full content URL, from {@link #getFullContentUrl(String, StoreNode)}
+     * @return A {@link StoreNode} that can be used in an app that came from the provided source
+     */
+    private static StoreNode getNewStoreNode(StoreNode originalSource, String sourceString, String contentUrl)
+    {
+        StoreNode next = originalSource;
+        if (sourceString == contentUrl) // External URL (check getFullUrlContent)
+            return null;
+        if (sourceString.charAt(0) == '/') // Absolute URL
+        {
+            next = APP_STORE_ROOT;
+            sourceString = sourceString.substring(1);
+        }
+        String[] dirs = sourceString.split("/");
+        try
+        {
+            for (int i = 0; i < dirs.length - 1; i++)
+                next = next.drillDown(dirs[i]);
+        }
+        catch (IOException e)
+        {
+            return null; // Should never happen, but let's not give a potentially incorrect node just in case
+        }
+        return next;
+    }
+
+    public static void addLibrary(CarpetScriptHost carpetScriptHost, StoreNode storeSource, Value library) {
+        if (!(library instanceof MapValue))
+            throw new InternalExpressionException("This is not a valid library map: "+library.getString());
+        Map<String, String> libraryMap = ((MapValue) library).getMap().entrySet().stream().collect(Collectors.toMap(e -> e.getKey().getString(), e -> e.getValue().getString()));
+        String source = libraryMap.get("source");
+        String contentUrl = getFullContentUrl(source, storeSource);
+        String target = libraryMap.computeIfAbsent("target", k -> contentUrl.substring(contentUrl.lastIndexOf('/') + 1));
+        if (!(contentUrl.endsWith(".sc") || contentUrl.endsWith(".scl")))
+            throw new InternalExpressionException("App resource type must download a scarpet app or library.");
+        if (target.indexOf('/') != -1)
+            throw new InternalExpressionException("App resource tried to leave script reserved space");
+        try
+        {
+            downloadScript(carpetScriptHost.responsibleSource, target, Triple.of(target, contentUrl, getNewStoreNode(storeSource, source, contentUrl)), true);
+        }
+        catch (CommandException e)
+        {
+            throw new InternalExpressionException("Error when installing app dependencies: " + e.toString());
+        }
+        CarpetScriptServer.LOG.info("Downloaded app "+target+" from "+contentUrl);
+    }
 }