Cleanup plugin API:

- Remove QuiltPluginTask

This was never implemented properly, and I'm unsure if hiding CompletableFuture is even a good idea.
Plus, I haven't actually implemented any multithreading yet - and it will be better to add proper a multithreading API along with the implementation, not a theoretical one beforehand.

- Properly implement TentativeLoadOption.

This part is still untested, but it should work in theory? Either way it no longer requires plugins to deal with tasks.

- Remove GUI reference.

I'm unsure how we should implement plugin gui requests - they could just use the existing gui API instead of needing something specific to plugins.

Author: AlexIIL

src/main/java/org/quiltmc/loader/api/plugin/QuiltLoaderPlugin.java

@@ -163,20 +163,11 @@ default void finish(ModSolveResult result) {}
 	// Solving
 	// #######
 
-	/** Resolves a single {@link TentativeLoadOption} that was added via
-	 * {@link QuiltPluginContext#addTentativeOption(LoadOption)}. This is only called if the option was selected by the
-	 * solver - unselected options are not resolved.
-	 * <p>
-	 * Long-running operations should use {@link QuiltPluginContext#submit(java.util.concurrent.Callable)} to perform
-	 * those tasks in the future, and possibly on different threads. Operations that require use of the gui should use
-	 * {@link QuiltPluginContext#addGuiRequest()} instead, and call submit after that has been accepted or denied.
-	 * 
-	 * @return A {@link QuiltPluginTask} containing (or will contain) the {@link LoadOption} that will replace the
-	 *         {@link TentativeLoadOption} next cycle. */
-	default QuiltPluginTask<? extends LoadOption> resolve(TentativeLoadOption from) {
-		throw new IllegalStateException(
-			getClass() + " has added a TentativeLoadOption (" + from.getClass() + ") but can't resolve it!"
-		);
+	/** Called once before {@link TentativeLoadOption}s are resolved. This is intended to allow plugins to queue
+	 * downloads, or begin other long-running activities that would benefit from running simultaneously. This is only
+	 * called with load options are selected by the solver - unselected options are excluded from this list. */
+	default void preResolve(List<TentativeLoadOption> optionsToResolve) {
+		// Nothing to do by default
 	}
 
 	/** @return True if this plugin did something which will solve / change the error in future, and so loader won't ask

src/main/java/org/quiltmc/loader/api/plugin/QuiltPluginContext.java

@@ -98,43 +98,6 @@ public interface QuiltPluginContext {
 	 * stacktrace to the crash report. */
 	void haltLoading();
 
-	// ##############
-	// # Scheduling #
-	// ##############
-
-	/** Submits a task to be completed after plugin resolution, but before the current cycle ends. The task may be
-	 * executed on a different thread, depending on loaders config options.
-	 * <p>
-	 * This should only be called by {@link QuiltLoaderPlugin#resolve(TentativeLoadOption)},
-	 * {@link QuiltLoaderPlugin#finish(org.quiltmc.loader.api.plugin.solver.ModSolveResult)}, or by any tasks that are
-	 * passed to this function during their execution.
-	 * 
-	 * @return A {@link QuiltPluginTask} which will contain the result of the task, or the failure state if something
-	 *         went wrong. */
-	<V> QuiltPluginTask<V> submit(Callable<V> task);
-
-	/** Submits a task to be completed after plugin resolution, and additionally after the given tasks have completed,
-	 * but before the current cycle ends. The task may be executed on a different thread, depending on loaders config
-	 * options. Note that the task will still be executed, <em>even if the dependencies failed.</em> This is to allow
-	 * the task to handle errors directly.
-	 * 
-	 * @param deps The tasks that must complete before the given task can be executed.
-	 * @return A {@link QuiltPluginTask} which will contain the result of the task, or the failure state if something
-	 *         went wrong. */
-	<V> QuiltPluginTask<V> submitAfter(Callable<V> task, QuiltPluginTask<?>... deps);
-
-	// #######
-	// # Gui #
-	// #######
-
-	/** Used to ask the real user of something. Normally this will append something to the existing gui rather than
-	 * opening a new gui each time this is called.
-	 * <p>
-	 * TODO: Create all gui stuff! for now this just throws an {@link AbstractMethodError} */
-	default <V> QuiltPluginTask<V> addGuiRequest() {
-		throw new AbstractMethodError("// TODO: Add gui support!");
-	}
-
 	// ###########
 	// # Solving #
 	// ###########

src/main/java/org/quiltmc/loader/api/plugin/QuiltPluginManager.java

@@ -54,25 +54,6 @@ public interface QuiltPluginManager {
 	// Loading
 	// #######
 
-	/** Returns a task which will load the specified zip file and returns a path to the root of it's contents.
-	 * <p>
-	 * How the given zip is loaded depends on loaders config settings - in particular the zip could be extracted to a
-	 * temporary folder on the same filesystem as the original zip.
-	 * <p>
-	 * WARNING: if this method allocates a new {@link FileSystem} then that will be closed, <em>unless</em> at least one
-	 * of the {@link QuiltLoaderPlugin}s {@link QuiltPluginContext#lockZip(Path) locks} it, or if a chosen mod is loaded
-	 * from it.
-	 * <p>
-	 * The returned task may throw the following exceptions:
-	 * <ul>
-	 * <li>{@link IOException} if something went wrong while loading the file.</li>
-	 * <li>{@link NonZipException} if {@link FileSystems#newFileSystem(Path, ClassLoader)} throws a
-	 * {@link ProviderNotFoundException}.</li>
-	 * </ul>
-	 * 
-	 * @see #loadZipNow(Path) */
-	QuiltPluginTask<Path> loadZip(Path zip);
-
 	/** Loads the specified zip file and returns a path to the root of it's contents.
 	 * <p>
 	 * How the given zip is loaded depends on loaders config settings - in particular the zip could be extracted to a
@@ -84,8 +65,7 @@ public interface QuiltPluginManager {
 	 * 
 	 * @throws IOException if something went wrong while loading the file.
 	 * @throws NonZipException if {@link FileSystems#newFileSystem(Path, ClassLoader)} throws a
-	 *             {@link ProviderNotFoundException}.
-	 * @see #loadZip(Path) */
+	 *             {@link ProviderNotFoundException}. */
 	Path loadZipNow(Path zip) throws IOException, NonZipException;
 
 	/** Creates a new in-memory read-write file system. This can be used for mods that aren't loaded from zips.

src/main/java/org/quiltmc/loader/api/plugin/QuiltPluginTask.java

@@ -14,7 +14,5 @@
  * limitations under the License.
  */
 
-/**
- * The package for 
- */
+/** Deprecated package. Plugins should use {@link org.quiltmc.loader.api.gui.QuiltLoaderGui} instead. */
 package org.quiltmc.loader.api.plugin.gui;

src/main/java/org/quiltmc/loader/api/plugin/gui/package-info.java

@@ -16,14 +16,20 @@
 
 package org.quiltmc.loader.api.plugin.solver;
 
+import org.quiltmc.loader.api.plugin.QuiltLoaderPlugin;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternal;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternalType;
 
 /** {@link LoadOption}s can implement this if they must be processed before they can be used, if they are selected.
  * <p>
- * Unselected {@link TentativeLoadOption}s are left alone at the end of the cycle, and are not resolved.
- */
+ * Unselected {@link TentativeLoadOption}s are left alone at the end of the cycle, and are not resolved. */
 @QuiltLoaderInternal(QuiltLoaderInternalType.PLUGIN_API)
 public interface TentativeLoadOption {
 
+	/** Resolves this to a real {@link LoadOption}. This is only invoked after
+	 * {@link QuiltLoaderPlugin#preResolve(java.util.List)} has already been called.
+	 * 
+	 * @return a new {@link LoadOption} which can be loaded as-is. (It must not be another
+	 *         {@link TentativeLoadOption}) */
+	LoadOption resolve();
 }

src/main/java/org/quiltmc/loader/api/plugin/solver/TentativeLoadOption.java

@@ -18,15 +18,13 @@
 
 import java.nio.file.Path;
 import java.util.Collection;
-import java.util.concurrent.Callable;
 
 import org.quiltmc.loader.api.plugin.QuiltPluginContext;
 import org.quiltmc.loader.api.gui.QuiltDisplayedError;
 import org.quiltmc.loader.api.gui.QuiltLoaderText;
 import org.quiltmc.loader.api.gui.QuiltTreeNode;
 import org.quiltmc.loader.api.gui.QuiltTreeNode.SortOrder;
 import org.quiltmc.loader.api.plugin.QuiltPluginManager;
-import org.quiltmc.loader.api.plugin.QuiltPluginTask;
 import org.quiltmc.loader.api.plugin.gui.PluginGuiTreeNode;
 import org.quiltmc.loader.api.plugin.solver.LoadOption;
 import org.quiltmc.loader.api.plugin.solver.ModLoadOption;
@@ -102,16 +100,6 @@ public void haltLoading() {
 		manager.haltLoading(this);
 	}
 
-	@Override
-	public <V> QuiltPluginTask<V> submit(Callable<V> task) {
-		return manager.submit(this, task);
-	}
-
-	@Override
-	public <V> QuiltPluginTask<V> submitAfter(Callable<V> task, QuiltPluginTask<?>... deps) {
-		return manager.submitAfter(this, task, deps);
-	}
-
 	@Override
 	public RuleContext ruleContext() {
 		return ruleContext;

src/main/java/org/quiltmc/loader/impl/plugin/BasePluginContext.java

@@ -17,7 +17,6 @@
 package org.quiltmc.loader.impl.plugin;
 
 import org.quiltmc.loader.api.plugin.solver.ModSolveResult;
-import org.quiltmc.loader.api.plugin.solver.TentativeLoadOption;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternal;
 import org.quiltmc.loader.impl.util.QuiltLoaderInternalType;
 
@@ -31,9 +30,6 @@ enum PerCycleStep {
 	/** Indicates that we've called {@link BuiltinQuiltPlugin#beforeSolve()}, and now we're solving. */
 	SOLVE,
 
-	/** Indicates that solving has finished, and now we're resolving {@link TentativeLoadOption}s. */
-	POST_SOLVE_TENTATIVE,
-
 	/** Indicates that solving was successful, and we have a resulting {@link ModSolveResult} to end loading with. */
 	SUCCESS;
 }

src/main/java/org/quiltmc/loader/impl/plugin/PerCycleStep.java

@@ -73,7 +73,6 @@
 import org.quiltmc.loader.api.plugin.QuiltLoaderPlugin;
 import org.quiltmc.loader.api.plugin.QuiltPluginContext;
 import org.quiltmc.loader.api.plugin.QuiltPluginManager;
-import org.quiltmc.loader.api.plugin.QuiltPluginTask;
 import org.quiltmc.loader.api.plugin.gui.PluginGuiManager;
 import org.quiltmc.loader.api.plugin.gui.PluginGuiTreeNode;
 import org.quiltmc.loader.api.plugin.gui.PluginGuiTreeNode.WarningLevel;
@@ -238,18 +237,6 @@ private BuiltinPluginContext addBuiltinPlugin(BuiltinQuiltPlugin plugin, String
 	// Loading
 	// #######
 
-	@Override
-	public QuiltPluginTask<Path> loadZip(Path zip) {
-		if (config.singleThreadedLoading) {
-			try {
-				return QuiltPluginTask.createFinished(loadZipNow(zip));
-			} catch (IOException | NonZipException e) {
-				return QuiltPluginTask.createFailed(e);
-			}
-		}
-		return submit(null, () -> loadZipNow(zip));
-	}
-
 	/** Kept for backwards compatibility with the first versions of RGML-Quilt, as it invoked this using reflection. */
 	@Deprecated
 	private Path loadZip0(Path zip) throws IOException, NonZipException {
@@ -1356,7 +1343,7 @@ private ModSolveResultImpl runSingleCycle() throws ModResolutionException, Timeo
 						ModSolveResultImpl partialResult = getPartialSolution();
 
 						if (processTentatives(partialResult)) {
-							this.perCycleStep = step = PerCycleStep.POST_SOLVE_TENTATIVE;
+							this.perCycleStep = step = PerCycleStep.START;
 						} else {
 							this.perCycleStep = step = PerCycleStep.SUCCESS;
 							result = partialResult;
@@ -1371,10 +1358,6 @@ private ModSolveResultImpl runSingleCycle() throws ModResolutionException, Timeo
 						return null;
 					}
 				}
-				case POST_SOLVE_TENTATIVE: {
-					// TODO: Deal with tentative load options!
-					return null;
-				}
 				case SUCCESS: {
 
 					cleanup();
@@ -1585,9 +1568,29 @@ private boolean processTentatives(ModSolveResult partialResult) {
 			return false;
 		} else {
 
+			Map<QuiltPluginContext, List<TentativeLoadOption>> pluginsToOptions = new HashMap<>();
+
 			for (TentativeLoadOption option : tentatives) {
 				QuiltPluginContext pluginSrc = tentativeLoadOptions.get(option);
-				QuiltPluginTask<? extends LoadOption> resolution = pluginSrc.plugin().resolve(option);
+				pluginsToOptions.computeIfAbsent(pluginSrc, s -> new ArrayList<>()).add(option);
+			}
+
+			for (Map.Entry<QuiltPluginContext, List<TentativeLoadOption>> entry : pluginsToOptions.entrySet()) {
+				entry.getKey().plugin().preResolve(entry.getValue());
+			}
+
+			for (Map.Entry<QuiltPluginContext, List<TentativeLoadOption>> entry : pluginsToOptions.entrySet()) {
+				for (TentativeLoadOption option : entry.getValue()) {
+					LoadOption replacement = option.resolve();
+					if (replacement instanceof TentativeLoadOption) {
+						throw new IllegalStateException(
+							"The TentativeLoadOption " + option.getClass()
+								+ " resolved into another TentativeLoadOption " + replacement.getClass() + "!"
+						);
+					}
+					removeLoadOption((LoadOption) option);
+					addLoadOption(replacement, (BasePluginContext) entry.getKey());
+				}
 			}
 
 			return true;
@@ -1792,18 +1795,6 @@ private static void forceGcButBadly() {
 		System.gc();
 	}
 
-	// #########
-	// # Tasks #
-	// #########
-
-	<V> QuiltPluginTask<V> submit(BasePluginContext ctx, Callable<V> task) {
-		throw new AbstractMethodError("// TODO: Implement plugin tasks!");
-	}
-
-	<V> QuiltPluginTask<V> submitAfter(BasePluginContext ctx, Callable<V> task, QuiltPluginTask<?>... deps) {
-		throw new AbstractMethodError("// TODO: Implement plugin tasks!");
-	}
-
 	// ########
 	// # Mods #
 	// ########