From 4aaca88a5d0e9e839dc261c3618f24eeaaa28540 Mon Sep 17 00:00:00 2001 From: IDCs Date: Mon, 12 Oct 2026 10:58:15 +0300 Subject: [PATCH] Merge pull request #24485 from Nexus-Mods/fix/laz-1009 workshop phases leave optional mods out --- .../views/CollectionPageEdit/ModsEditPage.tsx | 40 +++++++----- .../mod_management/util/rulePhase.test.ts | 62 ++++++++++++++++++- .../mod_management/util/rulePhase.ts | 26 +++++++- .../collectionInstallSessionSelectors.test.ts | 50 +++++++++++++-- .../util/collectionInstallSessionSelectors.ts | 33 +++++----- 5 files changed, 169 insertions(+), 42 deletions(-) diff --git a/src/renderer/src/extensions/collections/views/CollectionPageEdit/ModsEditPage.tsx b/src/renderer/src/extensions/collections/views/CollectionPageEdit/ModsEditPage.tsx index 719657758d..3c1ef34ca4 100644 --- a/src/renderer/src/extensions/collections/views/CollectionPageEdit/ModsEditPage.tsx +++ b/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 { 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 { 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 { { 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 { ], 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 { 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) => ( + {assignablePhase(entry.rule) ?? t("Optional")} + ), edit: {}, }, ]; diff --git a/src/renderer/src/extensions/mod_management/util/rulePhase.test.ts b/src/renderer/src/extensions/mod_management/util/rulePhase.test.ts index 4017b89502..1a7511cbd7 100644 --- a/src/renderer/src/extensions/mod_management/util/rulePhase.test.ts +++ b/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 => ({ @@ -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" }); + }); +}); diff --git a/src/renderer/src/extensions/mod_management/util/rulePhase.ts b/src/renderer/src/extensions/mod_management/util/rulePhase.ts index a6f3b7973b..e89dd26720 100644 --- a/src/renderer/src/extensions/mod_management/util/rulePhase.ts +++ b/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); +} diff --git a/src/renderer/src/util/collectionInstallSessionSelectors.test.ts b/src/renderer/src/util/collectionInstallSessionSelectors.test.ts index 051389a852..f2fd3b116a 100644 --- a/src/renderer/src/util/collectionInstallSessionSelectors.test.ts +++ b/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,9 +77,9 @@ 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); @@ -80,12 +87,15 @@ describe("getCollectionPhaseProgress", () => { 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([ diff --git a/src/renderer/src/util/collectionInstallSessionSelectors.ts b/src/renderer/src/util/collectionInstallSessionSelectors.ts index 3c77cd98c8..2687d2a5e4 100644 --- a/src/renderer/src/util/collectionInstallSessionSelectors.ts +++ b/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