Add support for recommended mod dependencies (#946)

packages/controller/src/ControlConnection.ts

@@ -675,6 +675,7 @@ export default class ControlConnection extends BaseConnection {
 		type ModPortalModType = InstanceType<typeof lib.ModPortalGetAllRequest.Response>["mods"][number];
 		type ModPortalReleaseType = NonNullable<ModPortalModType["releases"]>[number];
 		const dependencyRequirements = new Map<string, lib.ModVersionRange>();
+		const recommendedRequirements = new Map<string, lib.ModVersionRange>();
 		const optionalRequirements = new Map<string, lib.ModVersionRange>();
 		const candidateReleases = new Map<string, lib.ModInfo>();
 		const errors = new Map<string, lib.ModDependencyResolveErrors>();
@@ -739,16 +740,32 @@ export default class ControlConnection extends BaseConnection {
 				continue;
 			}
 
-			// Get the current version range (optionals are tracked in case they become required)
-			let versionRange = dependencyRequirements.get(mod.name);
-			if (!versionRange) {
-				versionRange = optionalRequirements.get(mod.name) ?? new lib.ModVersionRange();
-				if (mod.required) {
-					dependencyRequirements.set(mod.name, versionRange);
+			// Get the current version range (dependencies can be promoted to a stricter requirement)
+			const versionRange =
+				dependencyRequirements.get(mod.name)
+				?? recommendedRequirements.get(mod.name)
+				?? optionalRequirements.get(mod.name)
+				?? new lib.ModVersionRange();
+
+			if (mod.required) {
+				// Add to required, remove from recommended and optional
+				dependencyRequirements.set(mod.name, versionRange);
+				recommendedRequirements.delete(mod.name);
+				optionalRequirements.delete(mod.name);
+
+			} else if (mod.recommended) {
+				// If not required, add to recommended and remove from optional
+				if (!dependencyRequirements.has(mod.name)) {
+					recommendedRequirements.set(mod.name, versionRange);
 					optionalRequirements.delete(mod.name);
-				} else {
-					optionalRequirements.set(mod.name, versionRange);
 				}
+
+			} else if (
+				!dependencyRequirements.has(mod.name)
+				&& !recommendedRequirements.has(mod.name)
+			) {
+				// If not required or recommended, add to optional
+				optionalRequirements.set(mod.name, versionRange);
 			}
 
 			// Update the version range if needed
@@ -757,6 +774,7 @@ export default class ControlConnection extends BaseConnection {
 				if (!versionRange.valid) {
 					candidateReleases.delete(mod.name);
 					errors.set(mod.name, "unsatisfiable");
+					continue;
 				}
 			}
 
@@ -804,9 +822,9 @@ export default class ControlConnection extends BaseConnection {
 			}
 		}
 
-		// Remove incompatible dependencies if they aren't required by others
+		// Remove incompatible and unsatisfiable dependencies if they aren't required by others
 		for (const [modName, error] of errors) {
-			if (error === "incompatible" && !dependencyRequirements.has(modName)) {
+			if (!dependencyRequirements.has(modName) && (error === "incompatible" || error === "unsatisfiable")) {
 				errors.delete(modName);
 			}
 		}

packages/lib/src/data/ModInfo.ts

@@ -14,7 +14,7 @@ import {
 } from "./version";
 
 
-type ModDependencyType = "incompatible" | "optional" | "hidden" | "unordered" | "required";
+type ModDependencyType = "incompatible" | "optional" | "hidden" | "unordered" | "required" | "recommended";
 
 export type ModDependencyUnsatisfiedReason = "incompatible" | "missing_dependency" | "wrong_version";
 const UnsatisfiedSeverity: Record<ModDependencyUnsatisfiedReason, number> = {
@@ -39,6 +39,8 @@ export class ModDependency {
 				return "hidden";
 			case "~":
 				return "unordered";
+			case "+":
+				return "recommended";
 			case "":
 				return "required";
 			default:
@@ -86,6 +88,10 @@ export class ModDependency {
 		return this.type === "incompatible";
 	}
 
+	get recommended() {
+		return this.type === "recommended";
+	}
+
 	get required() {
 		return this.type === "unordered" || this.type === "required";
 	}

test/lib/data/ModInfo.js

@@ -217,23 +217,29 @@ describe("lib/data/ModInfo", function() {
 				const dependency = new ModDependency("! my-mod");
 				assert.equal(dependency.isSatisfied(mods), false);
 			});
-			it("should pass for optional / hidden being missing", function() {
+			it("should pass for optional / hidden / recommended being missing", function() {
 				const dependencyOptional = new ModDependency("? not-present");
 				assert.equal(dependencyOptional.isSatisfied(mods), true);
 				const dependencyHidden = new ModDependency("(?) not-present");
 				assert.equal(dependencyHidden.isSatisfied(mods), true);
+				const dependencyRecommended = new ModDependency("+ not-present");
+				assert.equal(dependencyRecommended.isSatisfied(mods), true);
 			});
-			it("should pass for optional / hidden being present", function() {
+			it("should pass for optional / hidden / recommended being present", function() {
 				const dependencyOptional = new ModDependency("? my-mod");
 				assert.equal(dependencyOptional.isSatisfied(mods), true);
 				const dependencyHidden = new ModDependency("(?) my-mod");
 				assert.equal(dependencyHidden.isSatisfied(mods), true);
+				const dependencyRecommended = new ModDependency("+ my-mod");
+				assert.equal(dependencyRecommended.isSatisfied(mods), true);
 			});
-			it("should fail for optional / hidden being present but wrong version", function() {
+			it("should fail for optional / hidden / recommended being present but wrong version", function() {
 				const dependencyOptional = new ModDependency("? my-mod > 2.0.0");
 				assert.equal(dependencyOptional.isSatisfied(mods), false);
 				const dependencyHidden = new ModDependency("(?) my-mod > 2.0.0");
 				assert.equal(dependencyHidden.isSatisfied(mods), false);
+				const dependencyRecommended = new ModDependency("+ my-mod > 2.0.0");
+				assert.equal(dependencyRecommended.isSatisfied(mods), false);
 			});
 			it("should pass for unordered / required being present", function() {
 				const dependencyOptional = new ModDependency("~ my-mod");
@@ -258,19 +264,29 @@ describe("lib/data/ModInfo", function() {
 			it("should return true iff the type is incompatible", function() {
 				const dependencyIncompatible = new ModDependency("! my-mod");
 				assert.equal(dependencyIncompatible.incompatible, true);
-				for (const test of ["my-mod", "? my-mod", "(?) my-mod", "~ my-mod"]) {
+				for (const test of ["my-mod", "? my-mod", "(?) my-mod", "~ my-mod", "+ my-mod"]) {
 					const dependency = new ModDependency(test);
 					assert.equal(dependency.incompatible, false, test);
 				}
 			});
 		});
+		describe("recommended", function() {
+			it("should return true iff the type is recommended", function() {
+				const dependencyIncompatible = new ModDependency("+ my-mod");
+				assert.equal(dependencyIncompatible.recommended, true);
+				for (const test of ["my-mod", "? my-mod", "(?) my-mod", "~ my-mod", "! my-mod"]) {
+					const dependency = new ModDependency(test);
+					assert.equal(dependency.recommended, false, test);
+				}
+			});
+		});
 		describe("required", function() {
 			it("should return true iff the dependency is required", function() {
 				for (const test of ["my-mod", "~ my-mod"]) {
 					const dependency = new ModDependency(test);
 					assert.equal(dependency.required, true, test);
 				}
-				for (const test of ["! my-mod", "? my-mod", "(?) my-mod"]) {
+				for (const test of ["! my-mod", "? my-mod", "(?) my-mod", "+ my-mod"]) {
 					const dependency = new ModDependency(test);
 					assert.equal(dependency.required, false, test);
 				}
@@ -282,7 +298,7 @@ describe("lib/data/ModInfo", function() {
 					const dependency = new ModDependency(test);
 					assert.equal(dependency.optional, true, test);
 				}
-				for (const test of ["! my-mod", "my-mod", "~ my-mod"]) {
+				for (const test of ["! my-mod", "my-mod", "~ my-mod", "+ my-mod"]) {
 					const dependency = new ModDependency(test);
 					assert.equal(dependency.optional, false, test);
 				}

test/messages/mod.test.js

@@ -187,14 +187,16 @@ describe("messages/mod", function() {
 			});
 		});
 		describe("handle", function() {
-			const factorioVersions = ["1.0", "1.1", "2.0"];
+			const factorioVersions = ["1.0", "1.1", "2.0", "2.1"];
 			it("resolves dependencies", async function() {
 				for (const factorioVersion of factorioVersions) {
-					setPortalModRelease("root", "1.0.0", factorioVersion, ["foo", "bar < 2.0.0", "? baz"]);
+					setPortalModRelease("root", "1.0.0", factorioVersion, ["foo", "bar < 2.0.0", "? baz", "+ qux"]);
 					setPortalModRelease("foo", "1.0.0", factorioVersion, ["foo-dep"]);
 					setPortalModRelease("foo-dep", "1.0.0", factorioVersion, []);
 					setPortalModRelease("bar", "1.0.0", factorioVersion, []);
 					setPortalModRelease("baz", "1.0.0", factorioVersion, []);
+					setPortalModRelease("qux", "1.0.0", factorioVersion, ["qux-dep"]);
+					setPortalModRelease("qux-dep", "1.0.0", factorioVersion, []);
 
 					setPortalModRelease("root-2", "1.0.0", factorioVersion, ["foo", "bar-2"]);
 					setPortalModRelease("bar-2", "1.0.0", factorioVersion, []);
@@ -213,7 +215,7 @@ describe("messages/mod", function() {
 					const depIds = new Set(result.dependencies.map(mod => mod.id));
 					assert.deepEqual(depIds, new Set([
 						"root_1.0.0", "foo_1.0.0", "foo-dep_1.0.0", "bar_1.0.0",
-						"root-2_1.0.0", "bar-2_1.0.0",
+						"root-2_1.0.0", "bar-2_1.0.0", "qux_1.0.0", "qux-dep_1.0.0",
 					]));
 				}
 			});
@@ -307,6 +309,24 @@ describe("messages/mod", function() {
 					assert.deepEqual(depIds, new Set(["root_1.0.0", "foo_1.0.0"]));
 				}
 			});
+			it("ignores missing optional dependencies", async function() {
+				for (const factorioVersion of factorioVersions) {
+					setPortalModRelease("root", "1.0.0", factorioVersion, ["? foo"]);
+
+					const result = await controlConnection.handleModDependencyResolveRequest(
+						new lib.ModDependencyResolveRequest([new ModDependency("root")], factorioVersion),
+					);
+
+					assert.deepEqual(result.errors, new Map([
+						["base", "notFound"],
+					]));
+
+					const depIds = new Set(result.dependencies.map(mod => mod.id));
+					assert.deepEqual(depIds, new Set([
+						"root_1.0.0",
+					]));
+				}
+			});
 			it("does not ignore optional dependencies when required by another", async function() {
 				for (const factorioVersion of factorioVersions) {
 					setPortalModRelease("root", "1.0.0", factorioVersion, ["foo", "? baz"]);
@@ -325,6 +345,44 @@ describe("messages/mod", function() {
 					assert.deepEqual(depIds, new Set(["root_1.0.0", "foo_1.0.0", "baz_1.0.0"]));
 				}
 			});
+			it("ignores missing recommended dependencies", async function() {
+				for (const factorioVersion of factorioVersions) {
+					setPortalModRelease("root", "1.0.0", factorioVersion, ["+ foo"]);
+
+					const result = await controlConnection.handleModDependencyResolveRequest(
+						new lib.ModDependencyResolveRequest([new ModDependency("root")], factorioVersion),
+					);
+
+					assert.deepEqual(result.errors, new Map([
+						["base", "notFound"],
+					]));
+
+					const depIds = new Set(result.dependencies.map(mod => mod.id));
+					assert.deepEqual(depIds, new Set([
+						"root_1.0.0",
+					]));
+				}
+			});
+			it("does not ignore recommended dependencies when required by another", async function() {
+				for (const factorioVersion of factorioVersions) {
+					setPortalModRelease("root", "1.0.0", factorioVersion, ["+ baz", "foo"]);
+					setPortalModRelease("foo", "1.0.0", factorioVersion, ["baz"]);
+					setPortalModRelease("baz", "1.0.0", factorioVersion, []);
+
+					const result = await controlConnection.handleModDependencyResolveRequest(
+						new lib.ModDependencyResolveRequest([new ModDependency("root")], factorioVersion),
+					);
+
+					assert.deepEqual(result.errors, new Map([
+						["base", "notFound"],
+					]));
+
+					const depIds = new Set(result.dependencies.map(mod => mod.id));
+					assert.deepEqual(depIds, new Set([
+						"root_1.0.0", "foo_1.0.0", "baz_1.0.0",
+					]));
+				}
+			});
 			it("prefers the locally installed version if matching", async function() {
 				controller.modStore.addMod(lib.ModInfo.fromJSON({ name: "foo", version: "1.0.0" }));
 				for (const factorioVersion of factorioVersions) {
@@ -426,7 +484,7 @@ describe("messages/mod", function() {
 					"pyalienlife", "pyalienlifegraphics", "pyalienlifegraphics2", "pyalienlifegraphics3",
 					"pycoalprocessing", "pycoalprocessinggraphics", "pyfusionenergy", "pyfusionenergygraphics",
 					"pypetroleumhandling", "pypetroleumhandlinggraphics", "pyrawores", "pyraworesgraphics",
-					"pyhightech", "pyhightechgraphics", "pyindustry", "pyindustrygraphics",
+					"pyhightech", "pyhightechgraphics", "pyindustry", "pyindustrygraphics", "enable-all-feature-flags",
 				]));
 			});
 		});