Fix --neoform argument requirements (#52)

Remove --neoform-file again. Instead use the existing ArtifactManager methods that allow --neoforge / --neoform to be file paths.
neoforged · Jan 2, 2025 · 1dab034 · 1dab034
1 parent 3e6c450
commit 1dab034
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 37 deletions.
diff --git a/README.md b/README.md
@@ -49,9 +49,8 @@ This produces the Vanilla artifacts in build/
 | Option                              | Description                                                                                                                                                                                                                                             |
 |-------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `--dist` [required]                 | Which distribution type to generate artifacts for. NeoForm defines these and usually `client`, `server` and `joined` are available.                                                                                                                     |
-| `--neoforge=<gav>`                  | Pass the NeoForge artifact to use as `net.neoforged:neoforge:<version>`. When passing this, the NeoForm version is implied. It can still be overridden by passing `--neoform` as well.                                                                  |
-| `--neoform=<gav>`                   | Pass the NeoForm artifact to use as `net.neoforged:neoform:<version>@zip`.                                                                                                                                                                              |
-| `--neoform-file=<path>`             | As an alternative to `--neoform` you can also pass a path to the NeoForm data archive directly.                                                                                                                                                         |
+| `--neoforge=<gav>`                  | Pass the NeoForge artifact to use as `net.neoforged:neoforge:<version>`. When passing this, the NeoForm version is implied. It can still be overridden by passing `--neoform` as well. You can also pass a local file path.                             |
+| `--neoform=<gav>`                   | Pass the NeoForm artifact to use as `net.neoforged:neoform:<version>@zip`, or a path to a local file.                                                                                                                                                   |
 | `--write-result=<id>:<path>`        | This option can be passed multiple times. It tells NFRT to write a result of the execution graph to the given path, such as the recompiled Minecraft jar-file, or the sources. If you pass no such option, NFRT will print which results are available. |
 | `--access-transformer=<path>`       | Adds access transformers which will be applied to the source before recompiling it.                                                                                                                                                                     |
 | `--interface-injection-data=<path>` | Adds [interface injection data](https://github.com/neoforged/JavaSourceTransformer?tab=readme-ov-file#interface-injection) which will be applied to the source before recompiling it.                                                                   |
@@ -93,8 +92,8 @@ of potential Gradle plugins never having to actually read and parse the NeoForm
 | `--no-copy-launcher-assets`    | Disables copying of local Minecraft Launcher assets, if using the asset root directly is disabled.                                                                               |
 | `--no-use-launcher-asset-root` | Disables using a detected Minecraft Launcher installation directly to store the required assets.                                                                                 |
 | `--concurrent-downloads`       | Limits the maximum number of concurrent downloads. Default is 25.                                                                                                                |
-| `--write-properties`         | Writes a property file to the given path that contains the asset index id (`asset_index`) and asset root path (`assets_root`) suitable for passing to Minecraft.                 |
-| `--write-json`               | Writes a JSON file to the given path that contains the asset index id (`asset_index`) and asset root path (`assets`) suitable for passing to Minecraft via a Neoform entrypoint. |
+| `--write-properties`           | Writes a property file to the given path that contains the asset index id (`asset_index`) and asset root path (`assets_root`) suitable for passing to Minecraft.                 |
+| `--write-json`                 | Writes a JSON file to the given path that contains the asset index id (`asset_index`) and asset root path (`assets`) suitable for passing to Minecraft via a Neoform entrypoint. |
 
 ## Common Options
 
@@ -141,13 +140,15 @@ NFRT will continue to download the artifact remotely in this case.
 
 ### Mojang Launcher Manifest
 
-The full URL to the [Launcher version manifest](https://launchermeta.mojang.com/mc/game/version_manifest_v2.json) can be overridden using `--launcher-meta-uri`.
+The full URL to the [Launcher version manifest](https://launchermeta.mojang.com/mc/game/version_manifest_v2.json) can be
+overridden using `--launcher-meta-uri`.
 
 ### Output Settings
 
 For more verbose output, pass `--verbose`.
 
-To force the use of ANSI color on the console, pass `--color`, or `--no-color` to disable it. The `NO_COLOR` environment variable is also respected.
+To force the use of ANSI color on the console, pass `--color`, or `--no-color` to disable it. The `NO_COLOR` environment
+variable is also respected.
 
 The use of Emojis in console output can be toggled with `--emojis` and `--no-emojis`.
 

diff --git a/src/main/java/net/neoforged/neoform/runtime/cli/DownloadAssetsCommand.java b/src/main/java/net/neoforged/neoform/runtime/cli/DownloadAssetsCommand.java
@@ -1,13 +1,13 @@
 package net.neoforged.neoform.runtime.cli;
 
+import net.neoforged.neoform.runtime.artifacts.Artifact;
 import net.neoforged.neoform.runtime.artifacts.ArtifactManager;
 import net.neoforged.neoform.runtime.config.neoforge.NeoForgeConfig;
 import net.neoforged.neoform.runtime.config.neoform.NeoFormConfig;
 import net.neoforged.neoform.runtime.downloads.AssetDownloadResult;
 import net.neoforged.neoform.runtime.downloads.AssetDownloader;
 import net.neoforged.neoform.runtime.downloads.DownloadManager;
 import net.neoforged.neoform.runtime.downloads.DownloadsFailedException;
-import net.neoforged.neoform.runtime.utils.MavenCoordinate;
 import picocli.CommandLine;
 
 import java.io.IOException;
@@ -117,19 +117,18 @@ private String getMinecraftVersion(ArtifactManager artifactManager) throws IOExc
             return version.minecraftVersion;
         }
 
-        MavenCoordinate neoformArtifact;
+        Artifact neoFormArchive;
         if (version.neoformArtifact != null) {
-            neoformArtifact = MavenCoordinate.parse(version.neoformArtifact);
+            neoFormArchive = artifactManager.get(version.neoformArtifact);
         } else {
             // Pull from neoforge artifact then
             var neoforgeArtifact = artifactManager.get(version.neoforgeArtifact);
             try (var neoforgeZipFile = new JarFile(neoforgeArtifact.path().toFile())) {
                 var neoforgeConfig = NeoForgeConfig.from(neoforgeZipFile);
-                neoformArtifact = MavenCoordinate.parse(neoforgeConfig.neoformArtifact());
+                neoFormArchive = artifactManager.get(neoforgeConfig.neoformArtifact());
             }
         }
 
-        var neoFormArchive = artifactManager.get(neoformArtifact);
         try (var zipFile = new ZipFile(neoFormArchive.path().toFile())) {
             return NeoFormConfig.from(zipFile).minecraftVersion();
         }

diff --git a/src/main/java/net/neoforged/neoform/runtime/cli/RunNeoFormCommand.java b/src/main/java/net/neoforged/neoform/runtime/cli/RunNeoFormCommand.java
@@ -20,7 +20,6 @@
 import net.neoforged.neoform.runtime.utils.FileUtil;
 import net.neoforged.neoform.runtime.utils.HashingUtil;
 import net.neoforged.neoform.runtime.utils.Logger;
-import net.neoforged.neoform.runtime.utils.MavenCoordinate;
 import picocli.CommandLine;
 
 import java.io.IOException;
@@ -69,19 +68,12 @@ public class RunNeoFormCommand extends NeoFormEngineCommand {
     String parchmentConflictPrefix;
 
     static class SourceArtifacts {
-        @CommandLine.ArgGroup(multiplicity = "1")
-        NeoFormArtifact neoform;
+        @CommandLine.Option(names = "--neoform")
+        String neoform;
         @CommandLine.Option(names = "--neoforge")
         String neoforge;
     }
 
-    static class NeoFormArtifact {
-        @CommandLine.Option(names = "--neoform")
-        String artifact;
-        @CommandLine.Option(names = "--neoform-file")
-        Path file;
-    }
-
     @Override
     protected void runWithNeoFormEngine(NeoFormEngine engine, List<AutoCloseable> closables) throws IOException, InterruptedException {
         var artifactManager = engine.getArtifactManager();
@@ -93,14 +85,11 @@ protected void runWithNeoFormEngine(NeoFormEngine engine, List<AutoCloseable> cl
 
             // Allow it to be overridden with local or remote data
             Path neoformArtifact;
-            if (sourceArtifacts.neoform.file != null) {
-                LOG.println("Overriding NeoForm version " + neoforgeConfig.neoformArtifact() + " with NeoForm file " + sourceArtifacts.neoform.file);
-                neoformArtifact = sourceArtifacts.neoform.file;
-            } else if (sourceArtifacts.neoform.artifact != null) {
-                LOG.println("Overriding NeoForm version " + neoforgeConfig.neoformArtifact() + " with CLI argument " + sourceArtifacts.neoform.artifact);
-                neoformArtifact = artifactManager.get(MavenCoordinate.parse(sourceArtifacts.neoform.artifact)).path();
+            if (sourceArtifacts.neoform != null) {
+                LOG.println("Overriding NeoForm version " + neoforgeConfig.neoformArtifact() + " with CLI argument " + sourceArtifacts.neoform);
+                neoformArtifact = artifactManager.get(sourceArtifacts.neoform).path();
             } else {
-                neoformArtifact = artifactManager.get(MavenCoordinate.parse(neoforgeConfig.neoformArtifact())).path();
+                neoformArtifact = artifactManager.get(neoforgeConfig.neoformArtifact()).path();
             }
 
             engine.loadNeoFormData(neoformArtifact, dist);
@@ -181,12 +170,7 @@ protected void runWithNeoFormEngine(NeoFormEngine engine, List<AutoCloseable> cl
 
             createSourcesAndCompiledWithNeoForge(engine.getGraph(), compiledWithNeoForgeOutput, sourcesWithNeoForgeOutput);
         } else {
-            Path neoFormDataPath;
-            if (sourceArtifacts.neoform.file != null) {
-                neoFormDataPath = sourceArtifacts.neoform.file;
-            } else {
-                neoFormDataPath = artifactManager.get(MavenCoordinate.parse(sourceArtifacts.neoform.artifact)).path();
-            }
+            var neoFormDataPath = artifactManager.get(sourceArtifacts.neoform).path();
 
             engine.loadNeoFormData(neoFormDataPath, dist);
         }
@@ -349,7 +333,7 @@ private static ExecutionNode getOrAddTransformSourcesNode(NeoFormEngine engine)
                 return transformNode;
             } else {
                 throw new IllegalStateException("Node transformSources has a different action type than expected. Expected: "
-                                                + ApplySourceTransformAction.class + " but got " + transformNode.action().getClass());
+                        + ApplySourceTransformAction.class + " but got " + transformNode.action().getClass());
             }
         }
 

diff --git a/src/main/java/net/neoforged/neoform/runtime/config/neoforge/NeoForgeConfig.java b/src/main/java/net/neoforged/neoform/runtime/config/neoforge/NeoForgeConfig.java
@@ -33,7 +33,7 @@ public static NeoForgeConfig from(ZipFile zipFile) throws IOException {
         byte[] configContent;
         var configEntry = zipFile.getEntry("config.json");
         if (configEntry == null || configEntry.isDirectory()) {
-            throw new IOException("NeoForm config file config.json not found in " + zipFile.getName());
+            throw new IOException("NeoForge config file config.json not found in " + zipFile.getName());
         }
 
         try (var in = zipFile.getInputStream(configEntry)) {