Fix argument passing & add back commandline for legacy versions

- Game/jvm arguments are now passed one-at-a-time, never grouped
- Added support for legacy version manifests by adding in the missing
arguments
- Correctly pull arguments from old manifests
- Some slightly hacky handling for old library stanzas
- Short-circuit if library URL has empty path to prevent accidentally
downloading HTML pages as jars
- Add launcherShortname prop, passed to --versionType

launcher-builder/src/main/java/com/skcraft/launcher/builder/PackageBuilder.java

@@ -233,7 +233,11 @@ public class PackageBuilder {
     private boolean tryDownloadLibrary(Library library, Library.Artifact artifact, String baseUrl, File outputPath)
             throws IOException, InterruptedException {
         URL url = new URL(baseUrl);
-        File tempFile = File.createTempFile("launcherlib", null);
+
+        if (url.getPath().isEmpty() || url.getPath().equals("/")) {
+            // empty path, this is probably the first "is this a full URL" try.
+            return false;
+        }
 
         // Some repositories compress their files
         List<Compressor> compressors = BuilderUtils.getCompressors(baseUrl);
@@ -241,6 +245,8 @@ public class PackageBuilder {
             url = new URL(url, compressor.transformPathname(artifact.getPath()));
         }
 
+        File tempFile = File.createTempFile("launcherlib", null);
+
         try {
             log.info("Downloading library " + library.getName() + " from " + url + "...");
             HttpRequest.get(url).execute().expectResponseCode(200).saveContent(tempFile);

launcher-builder/src/main/java/com/skcraft/launcher/builder/loaders/ModernForgeLoaderProcessor.java

@@ -52,7 +52,7 @@ public class ModernForgeLoaderProcessor implements ILoaderProcessor {
 				}
 
 				// Copy tweak class arguments
-				List<GameArgument> gameArguments = info.getMinecraftArguments().getGameArguments();
+				List<GameArgument> gameArguments = info.getArguments().getGameArguments();
 				if (gameArguments != null) {
 					version.getArguments().getGameArguments().addAll(gameArguments);
 				}

launcher-builder/src/main/java/com/skcraft/launcher/builder/loaders/OldForgeLoaderProcessor.java

@@ -1,7 +1,6 @@
 package com.skcraft.launcher.builder.loaders;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.collect.Lists;
 import com.google.common.io.ByteStreams;
 import com.google.common.io.Closer;
 import com.google.common.io.Files;
@@ -12,7 +11,6 @@ import com.skcraft.launcher.model.minecraft.Library;
 import com.skcraft.launcher.model.minecraft.MinecraftArguments;
 import com.skcraft.launcher.model.minecraft.VersionManifest;
 import com.skcraft.launcher.model.modpack.Manifest;
-import com.skcraft.launcher.util.Environment;
 import lombok.extern.java.Log;
 
 import java.io.File;
@@ -44,7 +42,7 @@ public class OldForgeLoaderProcessor implements ILoaderProcessor {
 				VersionManifest version = manifest.getVersionManifest();
 
 				// Copy tweak class arguments
-				MinecraftArguments args = profile.getVersionInfo().getMinecraftArguments();
+				MinecraftArguments args = profile.getVersionInfo().getArguments();
 				if (args != null) {
 					Iterator<GameArgument> iter = args.getGameArguments().iterator();
 					while (iter.hasNext()) {
@@ -54,8 +52,9 @@ public class OldForgeLoaderProcessor implements ILoaderProcessor {
 									? cur.getValues().get(1)
 									: iter.next().getJoinedValue();
 
-							GameArgument tweakArg = new GameArgument(Lists.newArrayList("--tweakClass", tweakClass));
+							List<GameArgument> gameArgs = manifest.getVersionManifest().getArguments().getGameArguments();
-							manifest.getVersionManifest().getArguments().getGameArguments().add(tweakArg);
+							gameArgs.add(new GameArgument("--tweakClass"));
+							gameArgs.add(new GameArgument(tweakClass));
 
 							log.info(String.format("Adding tweak class '%s' to arguments", tweakClass));
 						}
@@ -87,11 +86,8 @@ public class OldForgeLoaderProcessor implements ILoaderProcessor {
 					ZipEntry libraryEntry = BuilderUtils.getZipEntry(jarFile, filePath);
 
 					if (libraryEntry != null) {
-						Library library = new Library();
-						library.setName(libraryPath);
-
 						File librariesDir = new File(baseDir, manifest.getLibrariesLocation());
-						File extractPath = new File(librariesDir, library.getPath(Environment.getInstance()));
+						File extractPath = new File(librariesDir, Library.mavenNameToPath(libraryPath));
 
 						Files.createParentDirs(extractPath);
 						ByteStreams.copy(closer.register(jarFile.getInputStream(libraryEntry)),

launcher/src/main/java/com/skcraft/launcher/launch/Runner.java

@@ -131,6 +131,8 @@ public class Runner implements Callable<Process>, ProgressObservable {
         }
 
         progress = new DefaultProgress(0.9, SharedLocale.tr("runner.collectingArgs"));
+        builder.classPath(getJarPath());
+        builder.setMainClass(versionManifest.getMainClass());
 
         addWindowArgs();
         addLibraries();
@@ -141,9 +143,6 @@ public class Runner implements Callable<Process>, ProgressObservable {
         addPlatformArgs();
         addLegacyArgs();
 
-        builder.classPath(getJarPath());
-        builder.setMainClass(versionManifest.getMainClass());
-
         callLaunchModifier();
 
         ProcessBuilder processBuilder = new ProcessBuilder(builder.buildCommand());
@@ -249,7 +248,6 @@ public class Runner implements Callable<Process>, ProgressObservable {
         List<String> flags = builder.getFlags();
         String rawJvmArgs = config.getJvmArgs();
         if (!Strings.isNullOrEmpty(rawJvmArgs)) {
-
             for (String arg : JavaProcessBuilder.splitArgs(rawJvmArgs)) {
                 flags.add(arg);
             }
@@ -258,8 +256,10 @@ public class Runner implements Callable<Process>, ProgressObservable {
         List<GameArgument> javaArguments = versionManifest.getArguments().getJvmArguments();
         StrSubstitutor substitutor = new StrSubstitutor(getCommandSubstitutions());
         for (GameArgument arg : javaArguments) {
-            if (arg.resolveRules(environment, featureList)) {
+            if (arg.shouldApply(environment, featureList)) {
-                flags.add(substitutor.replace(arg.getJoinedValue()));
+                for (String subArg : arg.getValues()) {
+                    flags.add(substitutor.replace(subArg));
+                }
             }
         }
     }
@@ -275,8 +275,10 @@ public class Runner implements Callable<Process>, ProgressObservable {
         List<GameArgument> rawArgs = versionManifest.getArguments().getGameArguments();
         StrSubstitutor substitutor = new StrSubstitutor(getCommandSubstitutions());
         for (GameArgument arg : rawArgs) {
-            if (arg.resolveRules(environment, featureList)) {
+            if (arg.shouldApply(environment, featureList)) {
-                args.add(substitutor.replace(arg.getJoinedValue()));
+                for (String subArg : arg.getValues()) {
+                    args.add(substitutor.replace(subArg));
+                }
             }
         }
     }
@@ -343,7 +345,32 @@ public class Runner implements Callable<Process>, ProgressObservable {
      * Add arguments to make legacy Minecraft work.
      */
     private void addLegacyArgs() {
-        builder.getFlags().add("-Dminecraft.applet.TargetDirectory=" + instance.getContentDir());
+        List<String> flags = builder.getFlags();
+
+        if (versionManifest.getMinimumLauncherVersion() < 21) {
+            // Add bits that the legacy manifests don't
+            flags.add("-Djava.library.path=" + extractDir.getAbsoluteFile());
+            flags.add("-cp");
+            flags.add(builder.buildClassPath());
+
+            if (featureList.hasFeature("has_custom_resolution")) {
+                List<String> args = builder.getArgs();
+                args.add("--width");
+                args.add(String.valueOf(config.getWindowWidth()));
+                args.add("--height");
+                args.add(String.valueOf(config.getWidowHeight()));
+            }
+
+            // Add old platform hacks that the new manifests already specify
+            if (getEnvironment().getPlatform() == Platform.WINDOWS) {
+                flags.add("-XX:HeapDumpPath=MojangTricksIntelDriversForPerformance_javaw.exe_minecraft.exe.heapdump");
+            }
+        }
+
+        if (versionManifest.getMinimumLauncherVersion() < 18) {
+            // TODO find out exactly what versions need this hack.
+            flags.add("-Dminecraft.applet.TargetDirectory=" + instance.getContentDir());
+        }
     }
 
     /**
@@ -356,6 +383,7 @@ public class Runner implements Callable<Process>, ProgressObservable {
         Map<String, String> map = new HashMap<String, String>();
 
         map.put("version_name", versionManifest.getId());
+        map.put("version_type", launcher.getProperties().getProperty("launcherShortname"));
 
         map.put("auth_access_token", session.getAccessToken());
         map.put("auth_session", session.getSessionToken());

launcher/src/main/java/com/skcraft/launcher/model/loader/BasicInstallProfile.java

@@ -28,6 +28,7 @@ public class BasicInstallProfile {
 	}
 
 	@Data
+	@JsonIgnoreProperties(ignoreUnknown = true)
 	public static class Legacy {
 		private String profileName;
 	}

launcher/src/main/java/com/skcraft/launcher/model/loader/VersionInfo.java

@@ -7,7 +7,6 @@
 package com.skcraft.launcher.model.loader;
 
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
-import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Splitter;
 import com.skcraft.launcher.model.minecraft.GameArgument;
 import com.skcraft.launcher.model.minecraft.Library;
@@ -21,15 +20,10 @@ import java.util.List;
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class VersionInfo {
     private String id;
-    @JsonProperty("arguments")
+    private MinecraftArguments arguments;
-    private MinecraftArguments minecraftArguments;
     private String mainClass;
     private List<Library> libraries;
 
-    public void setMinecraftArguments(MinecraftArguments arguments) {
-        this.minecraftArguments = arguments;
-    }
-
     public void setMinecraftArguments(String argumentString) {
         MinecraftArguments minecraftArguments = new MinecraftArguments();
         minecraftArguments.setGameArguments(new ArrayList<GameArgument>());
@@ -38,6 +32,6 @@ public class VersionInfo {
             minecraftArguments.getGameArguments().add(new GameArgument(arg));
         }
 
-        setMinecraftArguments(minecraftArguments);
+        setArguments(minecraftArguments);
     }
 }

launcher/src/main/java/com/skcraft/launcher/model/minecraft/FeatureList.java

@@ -22,6 +22,10 @@ public class FeatureList {
 		return true;
 	}
 
+	public boolean hasFeature(String key) {
+		return features.get(key) != null && features.get(key);
+	}
+
 	public static class Mutable extends FeatureList {
 		public void addFeature(String key, boolean value) {
 			features.put(key, value);

launcher/src/main/java/com/skcraft/launcher/model/minecraft/GameArgument.java

@@ -35,7 +35,7 @@ public class GameArgument {
 		return Joiner.on(' ').join(values);
 	}
 
-	public boolean resolveRules(Environment environment, FeatureList featureList) {
+	public boolean shouldApply(Environment environment, FeatureList featureList) {
 		if (getRules() == null) return true;
 
 		boolean result = false;

launcher/src/main/java/com/skcraft/launcher/model/minecraft/Library.java

@@ -121,6 +121,25 @@ public class Library {
         private Map<String, Artifact> classifiers;
     }
 
+    // Support for old Forge distributions with legacy library specs.
+    public void setUrl(String url) {
+        Artifact virtualArtifact = new Artifact();
+
+        virtualArtifact.setUrl(url);
+        virtualArtifact.setPath(mavenNameToPath(name));
+
+        Downloads downloads = new Downloads();
+        downloads.setArtifact(virtualArtifact);
+
+        setDownloads(downloads);
+    }
+
+    public void setServerreq(boolean value) {
+        if (value) {
+            setUrl("https://libraries.minecraft.net/"); // TODO do something better than this
+        }
+    }
+
     public static String mavenNameToPath(String mavenName) {
         List<String> split = Splitter.on(':').splitToList(mavenName);
         int size = split.size();

launcher/src/main/resources/com/skcraft/launcher/launcher.properties

@@ -6,6 +6,7 @@
 
 version=${project.version}
 agentName=Minecraft
+launcherShortname=SKCLauncher
 offlinePlayerName=Player
 
 versionManifestUrl=https://launchermeta.mojang.com/mc/game/version_manifest.json