workshop phases leave optional mods out

src/renderer/src/extensions/collections/views/CollectionPageEdit/ModsEditPage.tsx

@@ -29,7 +29,13 @@ import type { IMod, IModRule } from "../../../mod_management/types/IMod";
 import { coerceToSemver } from "../../../mod_management/util/coerceToSemver";
 import { findModByRef } from "../../../mod_management/util/findModByRef";
 import renderModName, { renderModReference } from "../../../mod_management/util/modName";
-import { rulePhase } from "../../../mod_management/util/rulePhase";
+import {
+  assignablePhase,
+  changeRuleType,
+  highestRequiredPhase,
+  OPTIONAL_PHASE,
+  rulePhase,
+} from "../../../mod_management/util/rulePhase";
 import { isDependencyRule } from "../../../mod_management/util/testModReference";
 import { ADULT_CONTENT_URL, INSTRUCTIONS_PLACEHOLDER } from "../../constants";
 import type { ICollectionSourceInfo, SourceType } from "../../types/ICollection";
@@ -161,10 +167,8 @@ class ModsEditPage extends ComponentEx<IProps, IModsPageState> {
         const { entries } = this.state;
         instanceIds.forEach((id) => {
           if (entries[id]?.rule?.type === "recommends") {
-            const newRule = _.cloneDeep(entries[id].rule);
             onRemoveRule(entries[id].rule);
-            newRule.type = "requires";
-            onAddRule(newRule);
+            onAddRule(changeRuleType(entries[id].rule, "requires"));
           }
         });
       },
@@ -181,10 +185,8 @@ class ModsEditPage extends ComponentEx<IProps, IModsPageState> {
         const { entries } = this.state;
         instanceIds.forEach((id) => {
           if (entries[id].rule?.type === "requires") {
-            const newRule = _.cloneDeep(entries[id].rule);
             onRemoveRule(entries[id].rule);
-            newRule.type = "recommends";
-            onAddRule(newRule);
+            onAddRule(changeRuleType(entries[id].rule, "recommends"));
           }
         });
       },
@@ -322,12 +324,15 @@ class ModsEditPage extends ComponentEx<IProps, IModsPageState> {
     {
       icon: "sort-none",
       title: "Assign order",
+      condition: (instanceIds: string[]) =>
+        instanceIds.some((id) => assignablePhase(this.state.entries[id]?.rule) === undefined)
+          ? this.props.t("Optional mods install after all phases")
+          : true,
       subMenus: (instanceIds: string | string[]) => {
         const { t } = this.props;
         const ids = Array.isArray(instanceIds) ? instanceIds : [instanceIds];
-        const maxPhase = Object.values(this.state.entries).reduce(
-          (prev, entry) => Math.max(prev, rulePhase(entry.rule)),
-          0,
+        const maxPhase = highestRequiredPhase(
+          Object.values(this.state.entries).map((entry) => entry.rule),
         );
         return new Array(maxPhase + 1)
           .fill(0)
@@ -507,9 +512,7 @@ class ModsEditPage extends ComponentEx<IProps, IModsPageState> {
           ],
           onChangeValue: (source: IModEntry, value: any) => {
             this.props.onRemoveRule(source.rule);
-            const newRule = _.cloneDeep(source.rule);
-            newRule.type = value ? "requires" : "recommends";
-            this.props.onAddRule(newRule);
+            this.props.onAddRule(changeRuleType(source.rule, value ? "requires" : "recommends"));
           },
         },
         isSortable: true,
@@ -922,11 +925,16 @@ class ModsEditPage extends ComponentEx<IProps, IModsPageState> {
         isSortable: true,
         isDefaultVisible: false,
         groupName: (phase: any) =>
-          this.props.t("Phase {{phase}}", {
-            replace: { phase: (phase || 0).toString() },
-          }),
+          phase === OPTIONAL_PHASE
+            ? this.props.t("Optional")
+            : this.props.t("Phase {{phase}}", {
+                replace: { phase: (phase || 0).toString() },
+              }),
         isGroupable: true,
         calc: (mod) => rulePhase(mod.rule),
+        customRenderer: (entry: IModEntry, detailCell: boolean, t: TFunction) => (
+          <span>{assignablePhase(entry.rule) ?? t("Optional")}</span>
+        ),
         edit: {},
       },
     ];

src/renderer/src/extensions/mod_management/util/rulePhase.test.ts

@@ -1,7 +1,15 @@
 import { describe, it, expect } from "vitest";
 
+import { makeRule } from "../../../test-utils/builders";
 import type { IModRule } from "../types/IMod";
-import { rulePhase, collectionRulePhases, OPTIONAL_PHASE } from "./rulePhase";
+import {
+  rulePhase,
+  assignablePhase,
+  changeRuleType,
+  collectionRulePhases,
+  highestRequiredPhase,
+  OPTIONAL_PHASE,
+} from "./rulePhase";
 
 describe("rulePhase", () => {
   const baseRule = (over: Partial<IModRule>): IModRule => ({
@@ -77,3 +85,55 @@ describe("collectionRulePhases", () => {
     expect(collectionRulePhases([])).toEqual([]);
   });
 });
+
+describe("highestRequiredPhase", () => {
+  it("returns the highest phase among required rules, leaving optionals out", () => {
+    const rules = [
+      makeRule({ phase: 0 }),
+      makeRule({ phase: 2 }),
+      makeRule({ type: "recommends" }),
+    ];
+    expect(highestRequiredPhase(rules)).toBe(2);
+  });
+
+  it("returns 0 when every rule is optional", () => {
+    expect(highestRequiredPhase([makeRule({ type: "recommends" })])).toBe(0);
+  });
+});
+
+describe("assignablePhase", () => {
+  it("returns the authored phase of a required rule", () => {
+    expect(assignablePhase(makeRule({ phase: 2 }))).toBe(2);
+  });
+
+  it("has no phase for an optional rule", () => {
+    expect(assignablePhase(makeRule({ type: "recommends", phase: 2 }))).toBeUndefined();
+  });
+});
+
+describe("changeRuleType", () => {
+  it("drops the phase of a rule made optional", () => {
+    const changed = changeRuleType(makeRule({ phase: 3 }), "recommends");
+    expect(changed.type).toBe("recommends");
+    expect(changed.phase).toBeUndefined();
+  });
+
+  it("puts an optional made required in phase 0, whatever phase it carried", () => {
+    const optional = makeRule({ type: "recommends", phase: OPTIONAL_PHASE, extra: { phase: 4 } });
+    const changed = changeRuleType(optional, "requires");
+    expect(changed.type).toBe("requires");
+    expect(rulePhase(changed)).toBe(0);
+  });
+
+  it("keeps the phase when the type stays the same", () => {
+    const rule = makeRule({ phase: 3 });
+    expect(rulePhase(changeRuleType(rule, "requires"))).toBe(3);
+  });
+
+  it("keeps the rest of the rule", () => {
+    const rule = makeRule({ phase: 3, extra: { author: "someone", phase: 3 } });
+    const changed = changeRuleType(rule, "recommends");
+    expect(changed.reference).toEqual(rule.reference);
+    expect(changed.extra).toEqual({ author: "someone" });
+  });
+});

src/renderer/src/extensions/mod_management/util/rulePhase.ts

@@ -5,7 +5,7 @@
  */
 
 import type { IModRule } from "../types/IMod";
-import { isDependencyRule } from "./testModReference";
+import { isDependencyRule, isOptionalRule, isRequiredRule } from "./testModReference";
 
 /**
  * Install phase reserved for optional (recommends) members. It sorts after every authored
@@ -40,3 +40,27 @@ export function collectionRulePhases(rules: IModRule[]): number[] {
   (rules ?? []).filter(isDependencyRule).forEach((rule) => phases.add(rulePhase(rule)));
   return Array.from(phases).sort((a, b) => a - b);
 }
+
+/** Undefined for an optional, which installs after every phase. */
+export function assignablePhase(rule: IModRule | undefined): number | undefined {
+  return rule !== undefined && isOptionalRule(rule) ? undefined : rulePhase(rule);
+}
+
+/** A phase does not carry across a type change, so the copy drops it from both locations. */
+export function changeRuleType(rule: IModRule, type: "requires" | "recommends"): IModRule {
+  if (rule.type === type) {
+    return rule;
+  }
+  const { phase: _phase, extra, ...rest } = rule;
+  if (extra === undefined) {
+    return { ...rest, type };
+  }
+  const { phase: _legacyPhase, ...otherExtra } = extra;
+  return { ...rest, type, extra: otherExtra };
+}
+
+export function highestRequiredPhase(rules: IModRule[]): number {
+  return rules
+    .filter(isRequiredRule)
+    .reduce((highest, rule) => Math.max(highest, rulePhase(rule)), 0);
+}

src/renderer/src/util/collectionInstallSessionSelectors.test.ts

@@ -7,6 +7,7 @@
  */
 import { describe, expect, it } from "vitest";
 
+import { OPTIONAL_PHASE } from "../extensions/mod_management/util/rulePhase";
 import {
   makeInstallState,
   makeLookup,
@@ -20,10 +21,12 @@ import type {
   ICollectionModInstallInfo,
 } from "../types/collections/ICollectionInstallSession";
 import {
+  getCollectionCurrentPhase,
   getCollectionInstallProgress,
   getCollectionModByReference,
   getCollectionPhaseProgress,
   getCollectionStatusBreakdown,
+  getCollectionTotalPhases,
   getFailedOptionalMods,
   isCollectionPhaseComplete,
   isCollectionPhaseSettledSuccessfully,
@@ -43,7 +46,11 @@ function stateWith(entries: Entry[]) {
   for (const e of entries) {
     mods[e.ruleId] = makeModInstallInfo({
       // phase is read off the rule via rulePhase(); tag keeps each rule distinct
-      rule: makeRule({ phase: e.phase ?? 0, reference: { tag: e.ruleId } }),
+      rule: makeRule({
+        type: e.type ?? "requires",
+        phase: e.phase ?? 0,
+        reference: { tag: e.ruleId },
+      }),
       type: e.type ?? "requires",
       status: e.status ?? "pending",
     });
@@ -70,22 +77,25 @@ describe("getCollectionPhaseProgress", () => {
     ]);
 
     const phases = getCollectionPhaseProgress(state);
-    expect(phases.map((p) => p.phase)).toEqual([0, 1]);
+    expect(phases.map((p) => p.phase)).toEqual([0, 1, OPTIONAL_PHASE]);
 
-    const [p0, p1] = phases;
+    const [p0, p1, optionalPhase] = phases;
     // phase 0: one installed + one failed = both required terminal -> complete
     expect(p0.required).toBe(2);
     expect(p0.installed).toBe(1);
     expect(p0.failed).toBe(1);
     expect(p0.progress).toBe(100);
     expect(p0.isComplete).toBe(true);
 
-    // phase 1: one required still pending (the recommends one does not count) -> incomplete
+    // phase 1: one required still pending -> incomplete
     expect(p1.required).toBe(1);
-    expect(p1.optional).toBe(1);
     expect(p1.pending).toBe(1);
     expect(p1.progress).toBe(0);
     expect(p1.isComplete).toBe(false);
+
+    expect(optionalPhase.optional).toBe(1);
+    expect(optionalPhase.required).toBe(0);
+    expect(optionalPhase.isComplete).toBe(true);
   });
 
   it("treats a phase of only optional mods as complete", () => {
@@ -96,6 +106,36 @@ describe("getCollectionPhaseProgress", () => {
   });
 });
 
+describe("getCollectionTotalPhases", () => {
+  it("counts each authored phase plus one for the optional group", () => {
+    const state = stateWith([
+      { ruleId: "r1", phase: 0 },
+      { ruleId: "r2", phase: 2 },
+      { ruleId: "o1", type: "recommends" },
+    ]);
+    expect(getCollectionTotalPhases(state)).toBe(3);
+  });
+});
+
+describe("getCollectionCurrentPhase", () => {
+  it("returns the lowest phase with an outstanding required member", () => {
+    const state = stateWith([
+      { ruleId: "r1", phase: 0, status: "installed" },
+      { ruleId: "r2", phase: 2, status: "pending" },
+      { ruleId: "o1", type: "recommends", status: "pending" },
+    ]);
+    expect(getCollectionCurrentPhase(state)).toBe(2);
+  });
+
+  it("returns -1 once every required member is done, with optionals still pending", () => {
+    const state = stateWith([
+      { ruleId: "r1", phase: 0, status: "installed" },
+      { ruleId: "o1", type: "recommends", status: "pending" },
+    ]);
+    expect(getCollectionCurrentPhase(state)).toBe(-1);
+  });
+});
+
 describe("getCollectionStatusBreakdown", () => {
   it("splits counts across required, optional and total", () => {
     const state = stateWith([

src/renderer/src/util/collectionInstallSessionSelectors.ts

@@ -4,7 +4,7 @@ import { createSelector } from "reselect";
 
 import { activeDownloads } from "../extensions/download_management/selectors";
 import { modsForActiveGame } from "../extensions/mod_management/selectors";
-import { rulePhase } from "../extensions/mod_management/util/rulePhase";
+import { collectionRulePhases, rulePhase } from "../extensions/mod_management/util/rulePhase";
 import type { IModLookupInfo } from "../extensions/mod_management/util/testModReference";
 import testModReference, {
   isOptionalRule,
@@ -266,15 +266,19 @@ export const getCollectionModsForPhase = (
   return Object.values(mods).filter((mod) => rulePhase(mod.rule) === phase);
 };
 
+const getCollectionSessionPhases = (state: IState): number[] =>
+  collectionRulePhases(
+    Object.values(getCollectionActiveSessionMods(state))
+      .map((mod) => mod.rule)
+      .filter((rule) => rule !== undefined),
+  );
+
 /**
  * Get the total number of phases in the active session
- * @returns The highest phase number, or 0 if no active session
+ * @returns The number of distinct phases, optionals counting as one
  */
-export const getCollectionTotalPhases = (state: IState): number => {
-  const mods = getCollectionActiveSessionMods(state);
-  const phases = Object.values(mods).map((mod) => rulePhase(mod.rule));
-  return phases.length > 0 ? Math.max(...phases) + 1 : 0;
-};
+export const getCollectionTotalPhases = (state: IState): number =>
+  getCollectionSessionPhases(state).length;
 
 /**
  * Get installation progress statistics for the active session
@@ -506,19 +510,10 @@ export const isCollectionPhaseSettledSuccessfully = (state: IState, phase: numbe
 
 /**
  * Get the current phase being processed
- * @returns The lowest phase number with incomplete mods, or -1 if all complete
+ * @returns The lowest phase with incomplete required mods, or -1 once all required mods are done
  */
-export const getCollectionCurrentPhase = (state: IState): number => {
-  const totalPhases = getCollectionTotalPhases(state);
-
-  for (let phase = 0; phase < totalPhases; phase++) {
-    if (!isCollectionPhaseComplete(state, phase)) {
-      return phase;
-    }
-  }
-
-  return -1; // All phases complete
-};
+export const getCollectionCurrentPhase = (state: IState): number =>
+  getCollectionSessionPhases(state).find((phase) => !isCollectionPhaseComplete(state, phase)) ?? -1;
 
 /**
  * Get detailed phase progress information