diff --git a/src/domain/mods/importModpack.ts b/src/domain/mods/importModpack.ts index c2e2d907..7bb8d760 100644 --- a/src/domain/mods/importModpack.ts +++ b/src/domain/mods/importModpack.ts @@ -21,6 +21,33 @@ import type { InstalledModCopy, InstallModFailure, InstallModResult, ModReleaseT export interface ModpackEntry { modid: string version: string + /** + * Display name of the copy the pack was exported from, when the manifest carries one. Older packs + * do not, and nothing here may depend on it: it is a label of last resort, never an identifier. + */ + name?: string +} + +/** + * The manifest reader (`src/ipc/handlers/modsHandlers.ts`) refuses a mod name longer than this. + * The writer below must never emit one, so the two stay in agreement without being copy-pasted. + */ +export const MAX_MODPACK_MOD_NAME_LENGTH = 256 + +/** + * Cuts a mod's display name down to the length the manifest reader accepts. + * + * The name rides along for display only: it is never an identifier, and it must never be the + * reason an export fails. A modinfo.json name can run up to 4096 characters, well past the + * reader's cap, so anything over the cap is cut down here rather than left to the reader to + * reject. The cut lands on a UTF-16 code unit boundary that never splits a surrogate pair, so a + * name ending on an astral character (an emoji, say) keeps or drops it whole rather than leaving + * a lone surrogate behind. + */ +export function clampModpackModName(name: string): string { + if (name.length <= MAX_MODPACK_MOD_NAME_LENGTH) return name + const cut = name.slice(0, MAX_MODPACK_MOD_NAME_LENGTH) + return /[\uD800-\uDBFF]$/.test(cut) ? cut.slice(0, -1) : cut } /** A mod already in the installation's Mods folder, copied out of wherever it lives. */ @@ -54,6 +81,11 @@ export type ModpackSkipReason = | "not-on-moddb" /** The page exists but publishes no release at all. */ | "no-release" + /** + * The ModDB lookup itself did not answer: a transport failure, not a 404. Unlike + * "not-on-moddb", nothing here says the mod does not exist, so the row must not say so either. + */ + | "lookup-failed" /** An entry the import will install, with the release it settled on. */ export interface ModpackInstallItem { @@ -103,6 +135,12 @@ export interface ModpackPlanInput { gameVersion: string /** ModDB detail per modid, for every entry {@link modpackEntriesToResolve} asked for. */ details: ReadonlyMap + /** + * Modids whose lookup did not answer at all, transport failure rather than a clean miss. Absent + * from `details` the same way a genuine 404 is, but the caller has to tell the two apart to + * avoid calling a mod a fork when the database was simply unreachable. + */ + failedModids?: ReadonlySet } function installedFor(installed: readonly InstalledModSnapshot[], modid: string): InstalledModSnapshot | undefined { @@ -145,6 +183,71 @@ export function modpackDowngrades(entries: readonly ModpackEntry[], installed: r }) } +/** + * The best name a row of the import table can put on one manifest entry. + * + * Three sources, in falling order of trust: the name the ModDB answered with, the name the exporting + * launcher read off the local modinfo.json, and the modid. The middle one is what makes an entry the + * ModDB cannot resolve readable at all, and it is why the export carries it. + * + * `resolvedName` is read off a plan item, which names an unresolvable entry after its own modid, so + * a resolved name equal to the modid counts as no name and falls through to the local one. + * + * @param entry The manifest entry, with the local name when the pack carries one. + * @param resolvedName The ModDB name, when the lookup answered. + */ +export function modpackRowLabel(entry: ModpackEntry, resolvedName?: string): string { + const resolved = resolvedName?.trim() + if (resolved !== undefined && resolved.length > 0 && resolved !== entry.modid) return resolved + + const local = entry.name?.trim() + return local !== undefined && local.length > 0 ? local : entry.modid +} + +/** What one row of the import table says will happen to that mod, before anything happens. */ +export type ModpackRowStatusKind = + /** Not installed at all: the pack adds it. */ + | "new" + /** Installed at an older version than the release the import picked. */ + | "update" + /** Installed at a newer version than the release the import picked. */ + | "downgrade" + /** + * Installed at the version the import would put there, and still replaced: the copy on disk is + * turned off, or its version string does not match what the manifest asked for. A copy edited by + * hand lands here, and the row has to say so before the edit is overwritten. + */ + | "replace" + | ModpackSkipReason + +/** One row's plan, with the versions its wording needs. */ +export interface ModpackRowStatus { + kind: ModpackRowStatusKind + /** Version installed now, or null when the mod is new to the installation. */ + fromVersion: string | null + /** Version the import would leave behind, or null when it will not install anything. */ + toVersion: string | null +} + +/** + * Reads one plan item as the sentence its row shows. + * + * Every branch here is already decided by {@link planModpackImport}; this only tells the three ways + * of replacing an installed copy apart, which is the difference between "Update from 1.9.0 to + * 2.0.0" and a silent overwrite of a copy the player edited themselves. + */ +export function modpackRowStatus(item: ModpackPlanItem): ModpackRowStatus { + if (item.decision === "skip") { + return { kind: item.reason, fromVersion: item.fromVersion, toVersion: item.reason === "already-present" ? item.fromVersion : null } + } + + const toVersion = item.release.modversion + if (item.fromVersion === null) return { kind: "new", fromVersion: null, toVersion } + if (item.downgrade) return { kind: "downgrade", fromVersion: item.fromVersion, toVersion } + + return { kind: compareVersions(toVersion, item.fromVersion) > 0 ? "update" : "replace", fromVersion: item.fromVersion, toVersion } +} + /** * Picks the release to install for one entry. * @@ -181,7 +284,10 @@ function planEntry(entry: ModpackEntry, input: ModpackPlanInput): ModpackPlanIte } const detail = input.details.get(entry.modid) - if (!detail) return { decision: "skip", modid: entry.modid, requestedVersion: entry.version, name: entry.modid, reason: "not-on-moddb", fromVersion } + if (!detail) { + const reason = input.failedModids?.has(entry.modid) ? "lookup-failed" : "not-on-moddb" + return { decision: "skip", modid: entry.modid, requestedVersion: entry.version, name: entry.modid, reason, fromVersion } + } const release = pickRelease(detail.releases, entry.version, input.gameVersion) if (!release) { diff --git a/src/global.d.ts b/src/global.d.ts index 1af1a8ef..39e57639 100644 --- a/src/global.d.ts +++ b/src/global.d.ts @@ -283,6 +283,12 @@ declare global { type ModpackModEntryType = { modid: string version: string + /** + * Display name read off the installed mod's modinfo.json when the pack was + * exported. Absent from every pack exported before #379, so no reader may + * require it. + */ + name?: string } type ModpackManifestType = { diff --git a/src/ipc/handlers/modsHandlers.ts b/src/ipc/handlers/modsHandlers.ts index 7ad7a5f6..9d10773f 100644 --- a/src/ipc/handlers/modsHandlers.ts +++ b/src/ipc/handlers/modsHandlers.ts @@ -12,6 +12,7 @@ import { isJpegBytes, isPngBytes } from "@domain/backgrounds" import { getErrorMessage, logMessage } from "@src/utils/logManager" import { renameModArchiveTo, scanInstalledMods } from "@domain/mods/scanInstalled" import type { ScannedMod } from "@domain/mods/scanInstalled" +import { MAX_MODPACK_MOD_NAME_LENGTH } from "@domain/mods/importModpack" const MAX_MODPACK_ENTRIES = 2_000 @@ -59,7 +60,11 @@ function parseModpackManifest(value: unknown): ModpackManifestType { gameVersion: assertString(value.gameVersion, "modpack game version", 128), mods: value.mods.map((entry) => { if (!isRecord(entry)) throw new TypeError("Invalid modpack entry") - return { modid: assertString(entry.modid, "mod id", 256), version: assertString(entry.version, "mod version", 128) } + const parsed = { modid: assertString(entry.modid, "mod id", 256), version: assertString(entry.version, "mod version", 128) } + // Every pack exported before #379 carries modid and version only, so the name is read when it + // is there and never required. A name of the wrong type is still a refusal, like every other + // field: the manifest comes off disk and this is the only place its shape is checked. + return entry.name === undefined ? parsed : { ...parsed, name: assertString(entry.name, "mod name", MAX_MODPACK_MOD_NAME_LENGTH) } }) } } diff --git a/src/ipc/handlers/netHandlers.ts b/src/ipc/handlers/netHandlers.ts index e35abe17..e4fd7bd1 100644 --- a/src/ipc/handlers/netHandlers.ts +++ b/src/ipc/handlers/netHandlers.ts @@ -1,7 +1,8 @@ -import { ipcMain } from "electron" +import { app, ipcMain } from "electron" import { IPC_CHANNELS } from "../ipcChannels" import { readCatalogCache, writeCatalogCache } from "@src/ipc/catalogCache" +import { ConcurrencyLimiter } from "@src/ipc/concurrencyLimiter" import { assertTrustedIpcSender } from "@src/ipc/ipcSecurity" import { requestBoundedBuffer, requestBoundedText } from "@src/ipc/network" import { assertAllowedApiUrl, assertAllowedDownloadUrl, getApiUrlMaxBytes, MAX_MODDB_LISTING_RESPONSE_BYTES } from "@src/ipc/validation" @@ -17,9 +18,42 @@ function isModCatalogUrl(url: URL): boolean { return url.hostname === MOD_CATALOG_HOSTNAME && url.pathname === MOD_CATALOG_PATHNAME } +/** + * Nothing capped how many QUERY_URL calls the renderer could have in flight at once, so a + * burst of mod-detail lookups went straight out as one request per mod with no ceiling. + * The modpack import is the loudest caller: it resolves every entry the folder does not + * already satisfy, and it now does that when the manifest loads instead of when Import is + * clicked, so a 200-mod pack opened 200 sockets at the mod database before the player had + * decided anything. Manage Mods does the same shape of thing, one detail per installed mod. + * + * The bound belongs here rather than in the popup because every caller of this channel + * shares the one remote host, and only the main process sees all of them at once. A queued + * call looks to the renderer exactly like a slow one, a promise that has not settled, which + * is what the popup's "Checking the mod database..." state already renders. + * + * 6 is the per-host connection cap Chrome and Firefox have used for years, so a burst from + * the launcher never puts more on the mod database, which is community-run on modest + * hosting, than an ordinary visit to one of its pages does. It also keeps a big pack quick: + * 200 lookups in lanes of 6 is 34 rounds, a few seconds at a normal response time, rather + * than the minutes a smaller bound would cost. + * + * Queueing cannot trip a request's own timeout: requestBoundedText starts its wall clock + * inside the task, which the limiter does not run until a slot is free. + */ +const QUERY_URL_CONCURRENCY_LIMIT = 6 + +const queryConcurrency = new ConcurrencyLimiter(QUERY_URL_CONCURRENCY_LIMIT) + +// Same reason pathsHandlers.ts shuts its limiters down here: before-quit can preventDefault +// (the config flush in main/index.ts), so a queued lookup could otherwise still be handed a +// slot and start a fresh request while the app is on its way out. +app.on("before-quit", () => { + queryConcurrency.shutdown() +}) + /** * Validates and fetches a bounded API response, applying the per-rule ceiling (see - * API_URL_RULES). The mods-catalog endpoint additionally serves its last good disk-cached + * API_URL_RULES) and taking a slot in {@link queryConcurrency} for the request itself. The mods-catalog endpoint additionally serves its last good disk-cached * response, with a logged warning, when the fresh fetch fails for any reason (network * down, ceiling tripped, non-2xx status). Every other endpoint fails as before. */ @@ -29,7 +63,7 @@ export async function queryUrl(url: unknown): Promise { const isCatalog = isModCatalogUrl(safeUrl) try { - const text = await requestBoundedText(safeUrl, { maxBytes }) + const text = await queryConcurrency.run(() => requestBoundedText(safeUrl, { maxBytes })) if (isCatalog) { await writeCatalogCache(safeUrl, text).catch((cacheErr: unknown) => { logMessage("debug", `[back] [ipc] [ipc/handlers/netHandlers.ts] [QUERY_URL] Failed to write mod catalog cache: ${getErrorMessage(cacheErr)}`) diff --git a/src/renderer/src/features/mods/adapters/importModpack.ts b/src/renderer/src/features/mods/adapters/importModpack.ts index 3465b6c8..d6aca018 100644 --- a/src/renderer/src/features/mods/adapters/importModpack.ts +++ b/src/renderer/src/features/mods/adapters/importModpack.ts @@ -1,5 +1,23 @@ +import { clampModpackModName } from "@domain/mods/importModpack" import type { InstalledModSnapshot, ModpackImportEntryReport, ModpackModDetail } from "@domain/mods/importModpack" +/** + * Builds the manifest an export writes. + * + * The local display name rides along with the modid because the two diverge often (`tradie` is + * Traders Expansion, `sandwich` is Sammiches), and the importing launcher has nothing else to call a + * mod the ModDB cannot resolve. Nothing reads it as an identifier: the modid stays the key, and the + * name is clamped rather than passed through verbatim, so one mod with an oversized modinfo.json + * name can never fail the whole export. + */ +export function toModpackManifest(installation: InstallationType, installedMods: readonly InstalledModType[]): ModpackManifestType { + return { + name: installation.name, + gameVersion: installation.version, + mods: installedMods.map((mod) => ({ modid: mod.modid, version: mod.version, name: clampModpackModName(mod.name) })) + } +} + /** Copies an installed mod into the plain shape the planner reads. */ export function toInstalledModSnapshot(mod: InstalledModType): InstalledModSnapshot { return { modid: mod.modid, name: mod.name, version: mod.version, path: mod.path, enabled: mod.enabled, assetid: mod._mod?.assetid } diff --git a/src/renderer/src/features/mods/components/ImportModpackPopup.tsx b/src/renderer/src/features/mods/components/ImportModpackPopup.tsx index 7a32ec56..83d2ca93 100644 --- a/src/renderer/src/features/mods/components/ImportModpackPopup.tsx +++ b/src/renderer/src/features/mods/components/ImportModpackPopup.tsx @@ -5,8 +5,8 @@ import { PiCheckCircleDuotone, PiProhibitInsetDuotone, PiDownloadDuotone, PiMinu import { FiLoader } from "react-icons/fi" import clsx from "clsx" -import { executeModpackImport, modpackDowngrades, modpackEntriesToResolve, planModpackImport } from "@domain/mods/importModpack" -import type { ModpackEntryStatus, ModpackModDetail } from "@domain/mods/importModpack" +import { executeModpackImport, modpackDowngrades, modpackEntriesToResolve, modpackRowLabel, modpackRowStatus, planModpackImport } from "@domain/mods/importModpack" +import type { ModpackEntryStatus, ModpackModDetail, ModpackPlanItem, ModpackRowStatus, ModpackRowStatusKind } from "@domain/mods/importModpack" import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" import { toInstalledModSnapshot, toModChangeSummaryEntry, toModpackModDetail } from "@renderer/features/mods/adapters/importModpack" import { useInstallMod } from "../hooks/useInstallMod" @@ -18,8 +18,11 @@ import { NormalButton } from "@renderer/components/ui/Buttons" import { FormButton } from "@renderer/components/ui/FormComponents" import ModChangeSummaryPopup from "./ModChangeSummaryPopup" -/** What one row of the table shows: the two states of an entry in flight, then whatever it settled on. */ -type ModStatus = "pending" | "downloading" | ModpackEntryStatus +/** + * What one row of the table shows: what the plan intends, the two states of an entry in flight, then + * whatever it settled on. + */ +type ModStatus = "pending" | "downloading" | ModpackEntryStatus | ModpackRowStatusKind function ImportModpackPopup({ isOpen, @@ -47,11 +50,74 @@ function ImportModpackPopup({ const [importing, setImporting] = useState(false) const [summaryEntries, setSummaryEntries] = useState([]) const [showSummary, setShowSummary] = useState(false) + const [details, setDetails] = useState | null>(null) + const [failedModids, setFailedModids] = useState>(new Set()) + // Bumped by the retry action to re-run the resolution effect below on the same manifest, the + // same way a dependency change would. + const [retryCount, setRetryCount] = useState(0) + + const installed = useMemo(() => installedMods.map(toInstalledModSnapshot), [installedMods]) const downgradedMods = useMemo(() => { if (!manifest) return [] - return modpackDowngrades(manifest.mods, installedMods.map(toInstalledModSnapshot)) - }, [manifest, installedMods]) + return modpackDowngrades(manifest.mods, installed) + }, [manifest, installed]) + + /** + * The lookups run when the manifest opens rather than when Import is clicked. + * + * They cost the same either way, the import needs them regardless, and running them first is what + * lets the table name a mod and say what will happen to it before the player commits to anything. + * Only the entries the folder does not already satisfy are asked about, as before. + */ + useEffect(() => { + if (!manifest) return + + let cancelled = false + setDetails(null) + setFailedModids(new Set()) + + void (async (): Promise => { + const toResolve = modpackEntriesToResolve(manifest.mods, installed) + const fetched = await Promise.all(toResolve.map(async (entry) => [entry.modid, await queryMod({ modid: entry.modid })] as const)) + if (cancelled) return + + const resolved = new Map() + const failed = new Set() + for (const [modid, outcome] of fetched) { + if (outcome.status === "found") resolved.set(modid, toModpackModDetail(outcome.mod)) + else if (outcome.status === "failed") failed.add(modid) + } + setDetails(resolved) + setFailedModids(failed) + })() + + return (): void => { + cancelled = true + } + }, [manifest, installed, queryMod, retryCount]) + + const retryLookups = (): void => setRetryCount((n) => n + 1) + + const plan = useMemo(() => { + if (!manifest || !details) return null + return planModpackImport({ entries: manifest.mods, installed, gameVersion: installation.version, details, failedModids }) + }, [manifest, details, installed, installation.version, failedModids]) + + const planByModid = useMemo(() => new Map((plan?.items ?? []).map((item): [string, ModpackPlanItem] => [item.modid, item])), [plan]) + + const notOnModDbCount = useMemo(() => (plan?.items ?? []).filter((item) => item.decision === "skip" && item.reason === "not-on-moddb").length, [plan]) + + const lookupFailedCount = useMemo(() => (plan?.items ?? []).filter((item) => item.decision === "skip" && item.reason === "lookup-failed").length, [plan]) + + // True once the lookups have come back and every single one of them failed: nothing here was + // ever a clean 404, so the table would be one long, wrong "not on the mod database" list. The + // manifest is shown instead as unreachable rather than as a plan nobody can trust. + const allLookupsFailed = useMemo(() => { + if (!manifest || !details) return false + const toResolve = modpackEntriesToResolve(manifest.mods, installed) + return toResolve.length > 0 && toResolve.every((entry) => failedModids.has(entry.modid)) + }, [manifest, installed, details, failedModids]) const completedCount = useMemo(() => { return Object.values(modStatuses).filter((s) => s !== "pending" && s !== "downloading").length @@ -73,7 +139,7 @@ function ImportModpackPopup({ } async function handleImport(): Promise { - if (!manifest) return + if (!manifest || !plan) return // The same precondition every sibling flow has. Importing a pack writes to the Mods folder just // as an update does, and it was the one write that ran straight through a backup. @@ -81,20 +147,6 @@ function ImportModpackPopup({ setImporting(true) - const installed = installedMods.map(toInstalledModSnapshot) - - // Only the entries the folder does not already satisfy cost a lookup, which is what the old - // interleaved loop achieved by checking the folder before it queried. - const toResolve = modpackEntriesToResolve(manifest.mods, installed) - const fetched = await Promise.all(toResolve.map(async (entry) => [entry.modid, await queryMod({ modid: entry.modid })] as const)) - - const details = new Map() - for (const [modid, mod] of fetched) { - if (mod) details.set(modid, toModpackModDetail(mod)) - } - - const plan = planModpackImport({ entries: manifest.mods, installed, gameVersion: installation.version, details }) - const collected: ModChangeSummaryEntry[] = [] await executeModpackImport( @@ -187,61 +239,99 @@ function ImportModpackPopup({ )} - - - - {t("generic.name")} - {t("generic.version")} - {t("generic.status")} - - - - - {[...manifest.mods] - .sort((a, b) => a.modid.localeCompare(b.modid)) - .map((mod) => { - const status = modStatuses[mod.modid] || "pending" - return ( - - {mod.modid} - {mod.version} - - - - {statusLabel(status, t)} - - - - ) - })} - - - - {importing && ( -
-
- {t("features.mods.importModpackProgress", { completed: completedCount, total: totalCount })} - {Math.round(progressPct)}% -
-
-
-
+ {allLookupsFailed ? ( +
+ +

{t("features.mods.importModpackLookupUnreachable")}

+ + {t("features.mods.importModpackRetry")} +
+ ) : ( + <> + + + + {t("generic.name")} + {t("generic.version")} + {t("generic.status")} + + + + + {[...manifest.mods] + .sort((a, b) => a.modid.localeCompare(b.modid)) + .map((mod) => { + const live = modStatuses[mod.modid] || "pending" + const item = planByModid.get(mod.modid) + const label = modpackRowLabel(mod, item?.name) + // The plan owns the row until the import starts moving it: once an entry is + // downloading or settled, what happened outranks what was going to happen. + const planned = live === "pending" && item ? modpackRowStatus(item) : undefined + const status: ModStatus = planned?.kind ?? live + return ( + + +

{label}

+ {label !== mod.modid &&

{mod.modid}

} +
+ {mod.version} + + + + {statusLabel(status, t, planned)} + + +
+ ) + })} +
+
+ + {notOnModDbCount > 0 &&

{t("features.mods.importModpackNotOnModDbNote", { count: notOnModDbCount })}

} + + {lookupFailedCount > 0 && ( +
+ {t("features.mods.importModpackLookupFailedNote", { count: lookupFailedCount })} + + {t("features.mods.importModpackRetry")} + +
+ )} + + {importing && ( +
+
+ {t("features.mods.importModpackProgress", { completed: completedCount, total: totalCount })} + {Math.round(progressPct)}% +
+
+
+
+
+ )} + +
+ {!importing ? ( + + {plan ? : } +

{plan ? t("features.mods.importModpackButton") : t("features.mods.importModpackChecking")}

+
+ ) : ( + {}}> + +

{t("features.mods.importModpackImporting")}

+
+ )} +
+ )} - -
- {!importing ? ( - - -

{t("features.mods.importModpackButton")}

-
- ) : ( - {}}> - -

{t("features.mods.importModpackImporting")}

-
- )} -
)} @@ -249,18 +339,23 @@ function ImportModpackPopup({ ) } -function StatusIcon({ status }: Readonly<{ status: ModStatus }>): JSX.Element { +function StatusIcon({ status, className }: Readonly<{ status: ModStatus; className?: string }>): JSX.Element { switch (status) { case "installed": - return + return case "already-present": - return + return case "downloading": - return + return + case "downgrade": + case "replace": + return + case "new": + case "update": case "pending": - return + return default: - return + return } } @@ -271,7 +366,14 @@ function statusColor(status: ModStatus): string { case "already-present": return "text-zinc-400" case "downloading": + case "update": return "text-blue-400" + case "downgrade": + case "replace": + return "text-orange-300" + // Kept last in its group and on its own line: tests/text-contrast.test.ts reads the colour of a + // pending row straight out of this switch. + case "new": case "pending": return "text-zinc-400" default: @@ -279,8 +381,18 @@ function statusColor(status: ModStatus): string { } } -function statusLabel(status: ModStatus, t: (key: string) => string): string { +function statusLabel(status: ModStatus, t: (key: string, options?: Record) => string, planned?: ModpackRowStatus): string { + const versions = { from: planned?.fromVersion ?? "", to: planned?.toVersion ?? "" } + switch (status) { + case "new": + return t("features.mods.importModpackStatusNew") + case "update": + return t("features.mods.importModpackStatusUpdate", versions) + case "downgrade": + return t("features.mods.importModpackStatusDowngrade", versions) + case "replace": + return t("features.mods.importModpackStatusReplace", versions) case "installed": return t("features.mods.importModpackStatusDone") case "already-present": @@ -289,6 +401,8 @@ function statusLabel(status: ModStatus, t: (key: string) => string): string { return t("features.mods.importModpackStatusDownloading") case "not-on-moddb": return t("features.mods.importModpackNotFound") + case "lookup-failed": + return t("features.mods.importModpackLookupFailed") case "no-release": return t("features.mods.importModpackNoRelease") case "old-version-delete-failed": diff --git a/src/renderer/src/features/mods/hooks/useExportModpack.ts b/src/renderer/src/features/mods/hooks/useExportModpack.ts index ee84884f..fda106b9 100644 --- a/src/renderer/src/features/mods/hooks/useExportModpack.ts +++ b/src/renderer/src/features/mods/hooks/useExportModpack.ts @@ -2,22 +2,14 @@ import { useTranslation } from "react-i18next" import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" import { exportModpackArchive } from "@renderer/features/moddb/adapters/modsManager" +import { toModpackManifest } from "@renderer/features/mods/adapters/importModpack" export function useExportModpack(): ({ installedMods, installation }: { installedMods: InstalledModType[]; installation: InstallationType }) => Promise { const { t } = useTranslation() const { addNotification } = useNotificationsContext() async function exportModpack({ installedMods, installation }: { installedMods: InstalledModType[]; installation: InstallationType }): Promise { - const manifest: ModpackManifestType = { - name: installation.name, - gameVersion: installation.version, - mods: installedMods.map((mod) => ({ - modid: mod.modid, - version: mod.version - })) - } - - const result = await exportModpackArchive(manifest) + const result = await exportModpackArchive(toModpackManifest(installation, installedMods)) if (result.success) { addNotification(t("features.mods.exportModpackSuccess"), "success") diff --git a/src/renderer/src/features/mods/hooks/useGetCompleteInstalledMods.ts b/src/renderer/src/features/mods/hooks/useGetCompleteInstalledMods.ts index a3a9e0e8..2ad6ee9c 100644 --- a/src/renderer/src/features/mods/hooks/useGetCompleteInstalledMods.ts +++ b/src/renderer/src/features/mods/hooks/useGetCompleteInstalledMods.ts @@ -43,7 +43,10 @@ export function useGetCompleteInstalledMods(): ({ path, version, onFinish }: { p const pending = modDetails.get(key) if (pending) return pending - const request = queryMod({ modid }) + // Not-found and lookup-failed both leave this scan with no detail for the mod, which is all + // it has ever distinguished (a plain compatibility/update pass, not the modpack import + // table this outcome type exists for). + const request = queryMod({ modid }).then((outcome) => (outcome.status === "found" ? outcome.mod : undefined)) modDetails.set(key, request) return request } diff --git a/src/renderer/src/features/mods/hooks/useModReleaseCatalog.ts b/src/renderer/src/features/mods/hooks/useModReleaseCatalog.ts index 7a8993ff..8a35e85a 100644 --- a/src/renderer/src/features/mods/hooks/useModReleaseCatalog.ts +++ b/src/renderer/src/features/mods/hooks/useModReleaseCatalog.ts @@ -14,12 +14,11 @@ export type ModReleaseCatalogState = { /** * Queries one mod's detail (its release list) and reports whether that query worked. * - * useQueryMod answers `undefined` both when the ModDB call throws and when the payload does not - * parse, and it logs either one. That is enough for the callers that merge the answer into a - * bigger list, but not for a screen whose whole content is that answer: without a failure flag - * they cannot tell "still loading" from "never coming", which is how the install popup ended up - * spinning forever whenever the ModDB was slow or down. `failed` is that flag, and `retry` is the - * way out of it, the same shape useGameVersionCatalog gives the version list. + * useQueryMod tells a clean 404 apart from a lookup that never answered, but this screen has + * nothing more useful to say about either one than "it did not work": without a failure flag of + * its own it cannot tell "still loading" from "never coming", which is how the install popup ended + * up spinning forever whenever the ModDB was slow or down. `failed` is that flag, and `retry` is + * the way out of it, the same shape useGameVersionCatalog gives the version list. */ export function useModReleaseCatalog(modid: number | string | null): ModReleaseCatalogState { const queryMod = useQueryMod() @@ -44,11 +43,11 @@ export function useModReleaseCatalog(modid: number | string | null): ModReleaseC setLoading(true) setFailed(false) ;(async (): Promise => { - const found = await queryMod({ modid }) + const outcome = await queryMod({ modid }) if (cancelled) return - setMod(found ?? null) + setMod(outcome.status === "found" ? outcome.mod : null) setLoading(false) - setFailed(found === undefined) + setFailed(outcome.status !== "found") })() return (): void => { diff --git a/src/renderer/src/features/mods/hooks/useQueryMod.ts b/src/renderer/src/features/mods/hooks/useQueryMod.ts index e49999df..4f529fa2 100644 --- a/src/renderer/src/features/mods/hooks/useQueryMod.ts +++ b/src/renderer/src/features/mods/hooks/useQueryMod.ts @@ -4,7 +4,18 @@ import { parseModDetailResponse } from "@domain/mods/moddb" import { queryModDb } from "@renderer/features/moddb/adapters/moddb" import { logMods } from "@renderer/features/moddb/adapters/log" -export function useQueryMod(): ({ modid, onFinish }: { modid: number | string; onFinish?: () => void }) => Promise { +/** + * What a mod detail lookup came back with. + * + * "not-found" and "failed" both leave the caller with no mod, but they are not the same thing: a + * clean 404 means the ModDB has spoken and the id is not on it, while "failed" means the lookup + * never got an answer at all (a thrown network error, a timeout, or a response that did not parse + * as a v1 envelope). A caller that folds the two together ends up telling the player a mod is a + * fork or a private build when the real story is that the database could not be reached. + */ +export type QueryModOutcome = { status: "found"; mod: DownloadableModType } | { status: "not-found" } | { status: "failed" } + +export function useQueryMod(): ({ modid, onFinish }: { modid: number | string; onFinish?: () => void }) => Promise { /** * Makes a query and returns the mod with the passed Mod ID. * @@ -15,22 +26,25 @@ export function useQueryMod(): ({ modid, onFinish }: { modid: number | string; o * @param {object} props * @param {string} [props.modid] Mod ID string to query it. * @param {() => void} [props.onFinish] Optional function that will be called just before returning the mod. - * @returns {Promise} + * @returns {Promise} */ - return useCallback(async function queryMod({ modid, onFinish }: { modid: number | string; onFinish?: () => void }): Promise { + return useCallback(async function queryMod({ modid, onFinish }: { modid: number | string; onFinish?: () => void }): Promise { try { const res = await queryModDb(`/mod/${modid}`) const parsed = parseModDetailResponse(res) if (onFinish) onFinish() - if (!parsed.ok) return + // Only a genuine 404 counts as "not found". Every other way the envelope can fail to check + // out, an unrecognised statuscode or a payload that does not parse, is not an answer this + // caller can trust either way, so it is reported the same as a lookup that never came back. + if (!parsed.ok) return parsed.reason === "api-error" && parsed.statusCode === "404" ? { status: "not-found" } : { status: "failed" } - return parsed.payload as unknown as DownloadableModType + return { status: "found", mod: parsed.payload as unknown as DownloadableModType } } catch (err) { logMods("error", `[front] [mods] [features/mods/hooks/useQueryMod.ts] [useQueryMod > queryMod] Error fetching ${modid} mod versions.`) logMods("debug", `[front] [mods] [features/mods/hooks/useQueryMod.ts] [useQueryMod > queryMod] Error fetching ${modid} mod versions: ${err}`) - return + return { status: "failed" } } }, []) } diff --git a/src/renderer/src/locales/en-US.json b/src/renderer/src/locales/en-US.json index 2ab46758..2c70624a 100644 --- a/src/renderer/src/locales/en-US.json +++ b/src/renderer/src/locales/en-US.json @@ -263,8 +263,18 @@ "importModpackVersionWarning": "This modpack was made for game version {{packVersion}}, but this Installation uses version {{installVersion}}. Some Mods may not be compatible.", "importModpackInvalidFile": "Invalid modpack file!", "importModpackImporting": "Importing...", + "importModpackChecking": "Checking the mod database...", "importModpackManualRequired": "Manual install required", - "importModpackNotFound": "Not found on ModDB", + "importModpackNotFound": "Not on the mod database", + "importModpackNotOnModDbNote": "{{count}} mod(s) are not on the mod database: no listing there declares the mod id in any of its releases. Those are most likely forks or private builds, and have to be installed by hand.", + "importModpackLookupFailed": "Couldn't reach the mod database", + "importModpackLookupFailedNote": "{{count}} mod(s) could not be checked: the mod database could not be reached.", + "importModpackLookupUnreachable": "The mod database could not be reached, so none of these mods could be checked yet.", + "importModpackRetry": "Try again", + "importModpackStatusNew": "New install", + "importModpackStatusUpdate": "Update from {{from}} to {{to}}", + "importModpackStatusDowngrade": "Downgrade from {{from}} to {{to}}", + "importModpackStatusReplace": "The installed copy ({{from}}) will be replaced by {{to}}", "importModpackNoRelease": "No compatible release", "importModpackOldVersionStuck": "Old version still on disk", "importModpackStatusPending": "Pending", diff --git a/tests/domain/mods/importModpack.test.ts b/tests/domain/mods/importModpack.test.ts index c6c7a957..4775676f 100644 --- a/tests/domain/mods/importModpack.test.ts +++ b/tests/domain/mods/importModpack.test.ts @@ -1,7 +1,16 @@ import assert from "node:assert/strict" import { describe, it } from "vitest" -import { executeModpackImport, modpackDowngrades, modpackEntriesToResolve, planModpackImport } from "../../../src/domain/mods/importModpack" +import { + clampModpackModName, + executeModpackImport, + MAX_MODPACK_MOD_NAME_LENGTH, + modpackDowngrades, + modpackEntriesToResolve, + modpackRowLabel, + modpackRowStatus, + planModpackImport +} from "../../../src/domain/mods/importModpack" import type { InstalledModSnapshot, ModpackEntry, ModpackImportEntryReport, ModpackInstallItem, ModpackModDetail, ModpackPlanItem, ModpackRelease } from "../../../src/domain/mods/importModpack" import type { InstallModResult } from "../../../src/domain/mods/install" @@ -19,8 +28,8 @@ function installedCopy(overrides: Partial = {}): Installed return { modid: "carryon", name: "Carry On", version: "1.9.0", path: "/installations/main/Mods/carryon-1.9.0.zip", enabled: true, assetid: 4711, ...overrides } } -function plan(entries: ModpackEntry[], installed: InstalledModSnapshot[], details: Array<[string, ModpackModDetail]>): ModpackPlanItem[] { - return planModpackImport({ entries, installed, gameVersion: GAME_VERSION, details: new Map(details) }).items +function plan(entries: ModpackEntry[], installed: InstalledModSnapshot[], details: Array<[string, ModpackModDetail]>, failedModids?: readonly string[]): ModpackPlanItem[] { + return planModpackImport({ entries, installed, gameVersion: GAME_VERSION, details: new Map(details), failedModids: failedModids && new Set(failedModids) }).items } function onlyItem(items: ModpackPlanItem[]): ModpackPlanItem { @@ -169,6 +178,17 @@ describe("planModpackImport decisions", () => { assert.deepEqual(item, { decision: "skip", modid: "ghostmod", requestedVersion: "1.0.0", name: "ghostmod", reason: "not-on-moddb", fromVersion: null }) }) + // #384: a modid absent from `details` is not always a clean 404. When the caller has told the + // plan that this particular lookup never answered, the row must say the database was + // unreachable, not that the mod is a fork or a private build. + it("tells a lookup that failed apart from one that genuinely found nothing, for the same absent detail", () => { + const failed = onlyItem(plan([{ modid: "ghostmod", version: "1.0.0" }], [], [], ["ghostmod"])) + assert.deepEqual(failed, { decision: "skip", modid: "ghostmod", requestedVersion: "1.0.0", name: "ghostmod", reason: "lookup-failed", fromVersion: null }) + + const notFound = onlyItem(plan([{ modid: "ghostmod", version: "1.0.0" }], [], [])) + assert.equal(notFound.decision === "skip" && notFound.reason, "not-on-moddb") + }) + it("reports a page that publishes no release at all", () => { const item = onlyItem(plan([{ modid: "carryon", version: "2.0.1" }], [], [["carryon", detail([])]])) @@ -335,3 +355,122 @@ describe("executeModpackImport", () => { assert.deepEqual(report, { entries: [], installed: 0, failed: 0 }) }) }) + +describe("modpackRowLabel", () => { + it("prefers the name the ModDB answered with", () => { + assert.equal(modpackRowLabel({ modid: "tradie", version: "1.4.0", name: "Traders Expansion (local build)" }, "Traders Expansion"), "Traders Expansion") + }) + + it("falls back to the name the pack was exported with when nothing resolved (#379)", () => { + assert.equal(modpackRowLabel({ modid: "alloycalculatorstuzzichino", version: "1.0.4", name: "Alloy Calculator" }, undefined), "Alloy Calculator") + }) + + // A skipped plan item names itself after its own modid when no ModDB page answered, so a resolved + // name equal to the modid is not a name at all. + it("reads a resolved name equal to the modid as no name and takes the local one", () => { + assert.equal(modpackRowLabel({ modid: "animationslib", version: "1.2.0", name: "Animations Library" }, "animationslib"), "Animations Library") + }) + + it("falls back to the modid for a pack exported before names were written", () => { + assert.equal(modpackRowLabel({ modid: "waterwheelriverflowfix", version: "1.0.0" }, undefined), "waterwheelriverflowfix") + }) + + it("treats a blank local name as no name", () => { + assert.equal(modpackRowLabel({ modid: "sandwich", version: "2.1.0", name: " " }, undefined), "sandwich") + }) + + it("takes the ModDB name over a modid even when the pack carries no local name", () => { + assert.equal(modpackRowLabel({ modid: "hqzlights", version: "1.1.0" }, "Braziers"), "Braziers") + }) +}) + +describe("modpackRowStatus", () => { + function statusOf(entries: ModpackEntry[], installed: InstalledModSnapshot[], details: Array<[string, ModpackModDetail]>, failedModids?: readonly string[]): ReturnType { + return modpackRowStatus(onlyItem(plan(entries, installed, details, failedModids))) + } + + it("calls a mod the installation does not have a new install", () => { + const status = statusOf([{ modid: "carryon", version: "2.0.1" }], [], [["carryon", detail([release("2.0.1", ["v1.20.4"])])]]) + + assert.deepEqual(status, { kind: "new", fromVersion: null, toVersion: "2.0.1" }) + }) + + it("calls a newer release over an older copy an update, and carries both versions", () => { + const status = statusOf([{ modid: "carryon", version: "2.0.1" }], [installedCopy()], [["carryon", detail([release("2.0.1", ["v1.20.4"])])]]) + + assert.deepEqual(status, { kind: "update", fromVersion: "1.9.0", toVersion: "2.0.1" }) + }) + + it("calls an older release over a newer copy a downgrade, and carries both versions", () => { + const status = statusOf([{ modid: "carryon", version: "1.5.0" }], [installedCopy()], [["carryon", detail([release("1.5.0", ["v1.20.4"])])]]) + + assert.deepEqual(status, { kind: "downgrade", fromVersion: "1.9.0", toVersion: "1.5.0" }) + }) + + // #287: a pack is a playable set, so a copy the player turned off is reinstalled enabled. The row + // has to say the copy on disk is going away rather than call it a fresh install. + it("warns that a disabled copy at the very same version is still replaced", () => { + const status = statusOf([{ modid: "carryon", version: "1.9.0" }], [installedCopy({ enabled: false })], [["carryon", detail([release("1.9.0", ["v1.20.4"])])]]) + + assert.deepEqual(status, { kind: "replace", fromVersion: "1.9.0", toVersion: "1.9.0" }) + }) + + // The hand-edited copy from the #379 report: its version string was changed locally, so nothing on + // the ModDB matches it and the release that is picked lands on the same version it already has. + it("warns that a copy whose version the manifest does not match is replaced", () => { + const status = statusOf([{ modid: "carryon", version: "1.9.0-mine" }], [installedCopy()], [["carryon", detail([release("1.9.0", ["v1.20.4"])])]]) + + assert.deepEqual(status, { kind: "replace", fromVersion: "1.9.0", toVersion: "1.9.0" }) + }) + + it("says a mod already sitting at the requested version stays where it is", () => { + const status = statusOf([{ modid: "carryon", version: "1.9.0" }], [installedCopy()], []) + + assert.deepEqual(status, { kind: "already-present", fromVersion: "1.9.0", toVersion: "1.9.0" }) + }) + + it("says nothing will be installed for a modid no listing declares", () => { + const status = statusOf([{ modid: "alloycalculatorstuzzichino", version: "1.0.4" }], [], []) + + assert.deepEqual(status, { kind: "not-on-moddb", fromVersion: null, toVersion: null }) + }) + + it("says the lookup could not be checked, for a modid whose query failed rather than answered 404", () => { + const status = statusOf([{ modid: "alloycalculatorstuzzichino", version: "1.0.4" }], [], [], ["alloycalculatorstuzzichino"]) + + assert.deepEqual(status, { kind: "lookup-failed", fromVersion: null, toVersion: null }) + }) + + it("says nothing will be installed for a page that publishes no release, and still names the copy on disk", () => { + const status = statusOf([{ modid: "carryon", version: "2.0.1" }], [installedCopy()], [["carryon", detail([])]]) + + assert.deepEqual(status, { kind: "no-release", fromVersion: "1.9.0", toVersion: null }) + }) +}) + +describe("clampModpackModName", () => { + it("leaves a name at or under the cap untouched", () => { + assert.equal(clampModpackModName("Traders Expansion"), "Traders Expansion") + const atCap = "a".repeat(MAX_MODPACK_MOD_NAME_LENGTH) + assert.equal(clampModpackModName(atCap), atCap) + }) + + it("cuts a name over the cap down to exactly the cap the manifest reader accepts", () => { + const long = "a".repeat(MAX_MODPACK_MOD_NAME_LENGTH + 50) + const clamped = clampModpackModName(long) + + assert.equal(clamped.length, MAX_MODPACK_MOD_NAME_LENGTH) + assert.equal(clamped, "a".repeat(MAX_MODPACK_MOD_NAME_LENGTH)) + }) + + // A cut that lands mid-surrogate-pair leaves a lone high surrogate at the end of the string, + // which is not a character at all. The whole astral character is dropped instead, one code unit + // short of the cap, rather than shipping half of it. + it("never splits a surrogate pair sitting right on the cut", () => { + const straddling = "a".repeat(MAX_MODPACK_MOD_NAME_LENGTH - 1) + "🎮" + "bbbb" + const clamped = clampModpackModName(straddling) + + assert.equal(clamped, "a".repeat(MAX_MODPACK_MOD_NAME_LENGTH - 1)) + assert.equal(clamped.length, MAX_MODPACK_MOD_NAME_LENGTH - 1) + }) +}) diff --git a/tests/ipc/mod-catalog-cache.test.ts b/tests/ipc/mod-catalog-cache.test.ts index 76ee6a6e..4b58952a 100644 --- a/tests/ipc/mod-catalog-cache.test.ts +++ b/tests/ipc/mod-catalog-cache.test.ts @@ -22,7 +22,9 @@ const mockState = vi.hoisted(() => ({ vi.mock("electron", () => ({ app: { getPath: (name: string): string => (name === "userData" ? mockState.userDataDir : tmpdir()), - isPackaged: true + isPackaged: true, + // netHandlers.ts registers its limiter shutdown on before-quit; nothing here quits. + on: (): void => {} }, ipcMain: { handle: vi.fn() }, net: { request: (options: unknown): FakeRequest => mockState.requestHandler(options) } diff --git a/tests/ipc/modsHandlers.test.ts b/tests/ipc/modsHandlers.test.ts index 236fdc48..fcc5a6ef 100644 --- a/tests/ipc/modsHandlers.test.ts +++ b/tests/ipc/modsHandlers.test.ts @@ -230,6 +230,35 @@ describe("EXPORT_MODPACK", () => { assert.equal(vi.mocked(writeJsonAtomic).mock.calls.length, 1) assert.deepEqual(vi.mocked(writeJsonAtomic).mock.calls[0]?.[2], { spaces: 2 }) }) + + // #384: the writer (toModpackManifest) clamps a mod name to 256 characters before it ever + // reaches this handler, so the reader's own cap has to accept a name at exactly that length or + // the two would disagree again the moment either one moved. + it("accepts and exports a mod name at exactly the 256-character cap the writer can emit", async () => { + const exportDirectory = join(temporaryRoot, "exports-long-name") + mkdirSync(exportDirectory, { recursive: true }) + const targetFile = join(exportDirectory, "Long Name Pack.json") + vi.mocked(dialog.showSaveDialog).mockResolvedValueOnce({ canceled: false, filePath: targetFile }) + + const longName = "A".repeat(256) + const manifest: ModpackManifestType = { name: "Long Name Pack", gameVersion: "1.20.0", mods: [{ modid: "a", version: "1.0.0", name: longName }] } + + const event = await createTrustedEvent() + const result = await exportModpackHandler()(event, manifest) + assert.deepEqual(result, { success: true, path: targetFile }) + + const { readFileSync } = await import("node:fs") + assert.deepEqual(JSON.parse(readFileSync(targetFile, "utf-8")), manifest) + }) + + it("still refuses a mod name one character over that cap, so raising it silently could not go unnoticed", async () => { + const event = await createTrustedEvent() + const manifest = { name: "Pack", gameVersion: "1.20.0", mods: [{ modid: "a", version: "1.0.0", name: "A".repeat(257) }] } + const result = await exportModpackHandler()(event, manifest as unknown as ModpackManifestType) + + assert.deepEqual(result, { success: false }) + assert.equal(vi.mocked(dialog.showSaveDialog).mock.calls.length, 0) + }) }) describe("IMPORT_MODPACK", () => { @@ -297,6 +326,51 @@ describe("IMPORT_MODPACK", () => { assert.deepEqual(result, { success: false, error: "Error reading modpack file." }) }) + it("imports a manifest that carries a display name per mod (#379)", async () => { + const importDirectory = join(temporaryRoot, "imports") + mkdirSync(importDirectory, { recursive: true }) + const namedFile = join(importDirectory, "named.json") + const named: ModpackManifestType = { name: "My Modpack", gameVersion: "1.20.0", mods: [{ modid: "tradie", version: "1.4.0", name: "Traders Expansion" }] } + writeFileSync(namedFile, JSON.stringify(named), "utf-8") + + vi.mocked(dialog.showOpenDialog).mockResolvedValueOnce({ canceled: false, filePaths: [namedFile] }) + + const event = await createTrustedEvent() + const result = await importModpackHandler()(event) + assert.deepEqual(result, { success: true, manifest: named }) + }) + + it("refuses a manifest whose mod name is not a string", async () => { + const importDirectory = join(temporaryRoot, "imports") + mkdirSync(importDirectory, { recursive: true }) + const badFile = join(importDirectory, "bad-name.json") + writeFileSync(badFile, JSON.stringify({ name: "Pack", gameVersion: "1.20.0", mods: [{ modid: "tradie", version: "1.4.0", name: 7 }] }), "utf-8") + + vi.mocked(dialog.showOpenDialog).mockResolvedValueOnce({ canceled: false, filePaths: [badFile] }) + + const event = await createTrustedEvent() + const result = await importModpackHandler()(event) + assert.deepEqual(result, { success: false, error: "Error reading modpack file." }) + }) + + // #384: the round trip the fix promises. A name clamped to the writer's cap on export is exactly + // the shape the reader accepts, so the file that export produced imports cleanly too. + it("imports a manifest whose mod name sits at the 256-character cap the export clamp emits", async () => { + const importDirectory = join(temporaryRoot, "imports-long-name") + mkdirSync(importDirectory, { recursive: true }) + const longNameFile = join(importDirectory, "long-name.json") + const longName = "A".repeat(256) + const manifest: ModpackManifestType = { name: "Long Name Pack", gameVersion: "1.20.0", mods: [{ modid: "a", version: "1.0.0", name: longName }] } + writeFileSync(longNameFile, JSON.stringify(manifest), "utf-8") + + vi.mocked(dialog.showOpenDialog).mockResolvedValueOnce({ canceled: false, filePaths: [longNameFile] }) + + const event = await createTrustedEvent() + const result = await importModpackHandler()(event) + assert.deepEqual(result, { success: true, manifest }) + }) + + // Every pack exported before #379 has modid and version only. The reader has to keep taking them. it("imports a valid modpack manifest", async () => { const importDirectory = join(temporaryRoot, "imports") mkdirSync(importDirectory, { recursive: true }) diff --git a/tests/ipc/netHandlersDispatch.test.ts b/tests/ipc/netHandlersDispatch.test.ts index daf28abf..0c3753e6 100644 --- a/tests/ipc/netHandlersDispatch.test.ts +++ b/tests/ipc/netHandlersDispatch.test.ts @@ -37,7 +37,9 @@ const mockState = vi.hoisted(() => ({ vi.mock("electron", () => ({ app: { getPath: (name: string): string => (name === "userData" ? mockState.userDataDir : tmpdir()), - isPackaged: true + isPackaged: true, + // netHandlers.ts registers its limiter shutdown on before-quit; nothing here quits. + on: (): void => {} }, ipcMain: { handle: (channel: string, listener: (event: IpcMainInvokeEvent, ...args: never[]) => unknown): void => { @@ -58,7 +60,11 @@ class FakeResponse extends EventEmitter { } } -type RequestScenario = { kind: "success"; body: string } | { kind: "request-error"; message: string } +type RequestScenario = + | { kind: "success"; body: string } + | { kind: "request-error"; message: string } + /** Answers nothing until the test calls back the `finish` it is handed, so requests can be held open and overlap. */ + | { kind: "held"; body: string; onStart: (finish: () => void) => void } class FakeRequest extends EventEmitter { aborted = false @@ -78,6 +84,16 @@ class FakeRequest extends EventEmitter { end(): void { const scenario = this.scenario + if (scenario.kind === "held") { + scenario.onStart(() => { + const response = new FakeResponse({}, 200) + this.emit("response", response) + response.emit("data", Buffer.from(scenario.body, "utf8")) + response.emit("end") + }) + return + } + if (scenario.kind === "request-error") { this.emit("error", new Error(scenario.message)) return @@ -143,3 +159,65 @@ describe("QUERY_URL ipcMain.handle wrapper", () => { await assert.rejects(() => handler(event, "https://example.com/api/mods"), /URL is not allowed/) }) }) + +/** + * The bound on how many requests QUERY_URL lets out at once (#384). + * + * The modpack import resolves one mod-detail lookup per entry the folder does not already + * satisfy, and since #384 it does that when the manifest loads rather than when Import is + * clicked, so a 200-mod pack used to fire 200 requests at the mod database off a file + * chooser. Manage Mods fans out the same way over installed mods. + * + * This drives the real handler with the transport stubbed, holding every request open until + * the peak has been observed, so what it measures is the handler's own ceiling and not a + * timing accident. The expected peak is written out as a literal on purpose: reading + * QUERY_URL_CONCURRENCY_LIMIT back out of the module would make the test agree with whatever + * the constant happened to say. + */ +describe("QUERY_URL concurrency bound", () => { + it("never has more than 6 requests in flight, and still answers all 40", async () => { + const handler = getIpcHandler(IPC_CHANNELS.NET_MANAGER.QUERY_URL) + + let inFlight = 0 + let peak = 0 + const finishers: Array<() => void> = [] + + mockState.requestHandler = (): FakeRequest => + new FakeRequest({ + kind: "held", + body: '["tag-a"]', + onStart: (finish): void => { + inFlight++ + peak = Math.max(peak, inFlight) + finishers.push(() => { + inFlight-- + finish() + }) + } + }) + + const event = await createTrustedEvent() + const pending = Array.from({ length: 40 }, () => handler(event, TAGS_URL)) + + // Let every call that can start, start. Each held request parks after onStart, so once the + // queue stops growing the number parked is exactly the ceiling under test. A fixed drain + // rather than a wait for six, so a bound lower than expected fails on the count below + // instead of hanging until the suite timeout. + for (let tick = 0; tick < 50; tick++) await new Promise((resolve) => setImmediate(resolve)) + + assert.equal(peak, 6) + + // Release them one at a time, so a limiter that handed out extra slots on a release would + // show up as a peak above the bound rather than as a slower run. + while (finishers.length > 0) { + finishers.shift()?.() + await new Promise((resolve) => setImmediate(resolve)) + } + + const answers = await Promise.all(pending) + assert.equal(answers.length, 40) + assert.ok(answers.every((text) => text === '["tag-a"]')) + assert.equal(peak, 6) + assert.equal(inFlight, 0) + }) +}) diff --git a/tests/renderer-dom/importModpackPopup.test.tsx b/tests/renderer-dom/importModpackPopup.test.tsx new file mode 100644 index 00000000..b6648e0f --- /dev/null +++ b/tests/renderer-dom/importModpackPopup.test.tsx @@ -0,0 +1,341 @@ +import { describe, expect, it } from "vitest" +import { act, fireEvent, screen, waitFor, within } from "@testing-library/react" + +import { TaskProvider } from "@renderer/contexts/TaskManagerContext" +import ImportModpackPopup from "@renderer/features/mods/components/ImportModpackPopup" + +import { installMockWindowApi } from "./helpers/windowApi" +import { renderWithProviders } from "./helpers/render" + +/** + * The import table before anything is clicked (#379). + * + * NekoJess read a 200-row table of modids and could not tell what her pack held, nor what the import + * was about to do to her folder. Both answers exist in the plan already, so this mounts the popup + * over a manifest with one row of every kind and reads the table without pressing Import. + */ + +const GAME_VERSION = "1.20.4" + +function detailResponse(name: string, modversions: string[]): string { + return JSON.stringify({ + statuscode: "200", + mod: { + modid: 1, + assetid: 100, + name, + tags: [], + releases: modversions.map((modversion, index) => ({ + releaseid: index, + mainfile: `https://mods.vintagestory.at/download?v=${modversion}`, + filename: `mod-${modversion}.zip`, + fileid: index + 1, + modidstr: "mod", + modversion, + tags: [`v${GAME_VERSION}`] + })) + } + }) +} + +const MODDB: Record = { + tradie: detailResponse("Traders Expansion", ["1.4.0"]), + carryon: detailResponse("Carry On", ["2.0.0", "1.9.0"]), + primitivesurvival: detailResponse("Primitive Survival", ["3.6.0"]), + hqzlights: detailResponse("Braziers", ["1.1.0"]), + ghostmod: detailResponse("Ghost Mod", []), + alloycalculatorstuzzichino: JSON.stringify({ statuscode: "404" }), + animationslib: JSON.stringify({ statuscode: "404" }) +} + +function installation(): InstallationType { + return { + id: "main", + name: "Main", + icon: "", + path: "/installations/main", + version: GAME_VERSION, + startParams: "", + backupsLimit: 3, + backupsAuto: false, + compressionLevel: 6, + backups: [], + lastTimePlayed: 0, + totalTimePlayed: 0, + mesaGlThread: false, + envVars: "" + } +} + +function installedMod(name: string, modid: string, version: string, enabled = true): InstalledModType { + return { name, modid, version, path: `/installations/main/Mods/${modid}-${version}.zip`, enabled } +} + +const MANIFEST: ModpackManifestType = { + name: "NekoJess pack", + gameVersion: GAME_VERSION, + mods: [ + { modid: "tradie", version: "1.4.0", name: "Traders Expansion" }, + { modid: "carryon", version: "2.0.0", name: "Carry On" }, + { modid: "primitivesurvival", version: "3.6.0", name: "Primitive Survival" }, + { modid: "sandwich", version: "2.1.0", name: "Sammiches" }, + { modid: "hqzlights", version: "1.1.0", name: "Braziers" }, + { modid: "alloycalculatorstuzzichino", version: "1.0.4", name: "Alloy Calculator" }, + // No name at all: a pack exported before the name was written. + { modid: "animationslib", version: "1.2.0" }, + { modid: "ghostmod", version: "1.0.0", name: "Ghost Mod" } + ] +} + +const INSTALLED = [ + installedMod("Carry On", "carryon", "1.9.0"), + installedMod("Primitive Survival", "primitivesurvival", "3.7.0"), + installedMod("Sammiches", "sandwich", "2.1.0"), + installedMod("Braziers", "hqzlights", "1.1.0", false) +] + +function mountPopup(): void { + installMockWindowApi({ + netManager: { + queryURL: async (url: string) => { + const modid = url.split("/mod/")[1] ?? "" + const response = MODDB[modid] + if (!response) throw new Error(`The popup queried an unexpected mod: ${modid}`) + return response + } + } + }) + + renderWithProviders( + + {}} installation={installation()} installedMods={INSTALLED} onFinish={(): void => {}} /> + + ) +} + +async function rowFor(label: string): Promise { + const row = (await screen.findByText(label)).closest("li") + if (!row) throw new Error(`No table row found for "${label}".`) + return row +} + +describe("ImportModpackPopup, before Import is clicked", () => { + it("labels a resolved row with the mod database name and keeps the modid as a second line", async () => { + mountPopup() + + const row = await rowFor("Traders Expansion") + expect(within(row).getByText("tradie")).toBeTruthy() + expect(within(row).getByText("New install")).toBeTruthy() + }) + + it("says an update, from the installed version to the one the pack asks for", async () => { + mountPopup() + + expect(within(await rowFor("Carry On")).getByText("Update from 1.9.0 to 2.0.0")).toBeTruthy() + }) + + it("says a downgrade, both versions named", async () => { + mountPopup() + + expect(within(await rowFor("Primitive Survival")).getByText("Downgrade from 3.7.0 to 3.6.0")).toBeTruthy() + }) + + it("says a mod already sitting at the pack's version needs nothing, without ever asking the mod database", async () => { + mountPopup() + + expect(within(await rowFor("Sammiches")).getByText("Already installed")).toBeTruthy() + }) + + it("warns that a copy the pack does not accept, here a disabled one, will be replaced (#379)", async () => { + mountPopup() + + expect(within(await rowFor("Braziers")).getByText("The installed copy (1.1.0) will be replaced by 1.1.0")).toBeTruthy() + }) + + it("names an unresolvable mod by the name the pack was exported with, not by its modid", async () => { + mountPopup() + + const row = await rowFor("Alloy Calculator") + expect(within(row).getByText("alloycalculatorstuzzichino")).toBeTruthy() + expect(within(row).getByText("Not on the mod database")).toBeTruthy() + }) + + it("explains what was checked for the mods the database does not declare", async () => { + mountPopup() + + await rowFor("Alloy Calculator") + expect( + screen.getByText( + "2 mod(s) are not on the mod database: no listing there declares the mod id in any of its releases. Those are most likely forks or private builds, and have to be installed by hand." + ) + ).toBeTruthy() + }) + + it("falls back to the bare modid for a pack exported before names were written, with no second line", async () => { + mountPopup() + + const row = await rowFor("animationslib") + expect(within(row).getAllByText("animationslib").length).toBe(1) + }) + + it("says a mod whose page publishes no release at all cannot be installed", async () => { + mountPopup() + + expect(within(await rowFor("Ghost Mod")).getByText("No compatible release")).toBeTruthy() + }) + + it("keeps the rows as plain list items, so the second line does not turn one into a control", async () => { + mountPopup() + + const row = await rowFor("Traders Expansion") + expect(row.tagName).toBe("LI") + expect(within(row).queryByRole("button")).toBeNull() + }) +}) + +/** + * Closing the popup while the lookups are still out (#384). + * + * The lookups now start when the manifest loads, and the main process holds them to six at a + * time, so a big pack spends real seconds resolving and a player can easily close the popup + * and open another pack before the first batch has come back. The effect's cancelled flag is + * what keeps that first batch from landing on the second pack's table. + * + * The stale batch is released last on purpose. A resolution that comes back before the fresh + * one would be overwritten anyway; the one that has to be dropped is the one that arrives + * after the popup has already been re-opened on something else. + */ +describe("ImportModpackPopup, closed mid-lookup", () => { + const FIRST: ModpackManifestType = { name: "First pack", gameVersion: GAME_VERSION, mods: [{ modid: "tradie", version: "1.4.0", name: "Traders Expansion" }] } + const SECOND: ModpackManifestType = { name: "Second pack", gameVersion: GAME_VERSION, mods: [{ modid: "carryon", version: "2.0.0", name: "Carry On" }] } + + it("drops a lookup that comes back after the popup was closed and re-opened on another pack", async () => { + const held = new Map void>() + + installMockWindowApi({ + netManager: { + queryURL: (url: string) => + new Promise((resolve) => { + held.set(url.split("/mod/")[1] ?? "", resolve) + }) + } + }) + + function popup(manifest: ModpackManifestType | null): JSX.Element { + return ( + + {}} installation={installation()} installedMods={[]} onFinish={(): void => {}} /> + + ) + } + + const { rerender } = renderWithProviders(popup(FIRST)) + await waitFor(() => expect(held.has("tradie")).toBe(true)) + + rerender(popup(null)) + rerender(popup(SECOND)) + await waitFor(() => expect(held.has("carryon")).toBe(true)) + + await act(async () => { + held.get("carryon")?.(detailResponse("Carry On", ["2.0.0"])) + }) + expect(within(await rowFor("Carry On")).getByText("New install")).toBeTruthy() + + await act(async () => { + held.get("tradie")?.(detailResponse("Traders Expansion", ["1.4.0"])) + }) + + expect(screen.queryByText("Traders Expansion")).toBeNull() + expect(within(await rowFor("Carry On")).getByText("New install")).toBeTruthy() + }) +}) + +/** + * The mod database being unreachable (#384). + * + * useQueryMod used to fold a thrown network error into the same "nothing answered" bucket as a + * clean 404, so an outage read as every unresolved mod being a fork the player has to install by + * hand. These tests pin the fix: a lookup that never answered is told apart from one that did, + * shown as its own row status with a way to try again, and a total outage replaces the table + * rather than drowning it in false "not on the mod database" rows. + */ +describe("ImportModpackPopup, when the mod database cannot be reached", () => { + const MIXED: ModpackManifestType = { + name: "Mixed pack", + gameVersion: GAME_VERSION, + mods: [ + { modid: "tradie", version: "1.4.0", name: "Traders Expansion" }, + { modid: "unreachablemod", version: "1.0.0", name: "Unreachable Mod" }, + { modid: "trulymissing", version: "1.0.0", name: "Truly Missing" } + ] + } + + function mountWith(queryURL: (url: string) => Promise): void { + installMockWindowApi({ netManager: { queryURL } }) + renderWithProviders( + + {}} installation={installation()} installedMods={[]} onFinish={(): void => {}} /> + + ) + } + + /** Answers one modid of MIXED, throwing instead for whichever ones are named as still down. */ + function respond(modid: string, downFor: ReadonlySet = new Set()): Promise { + if (downFor.has(modid)) return Promise.reject(new Error("network down")) + if (modid === "tradie") return Promise.resolve(detailResponse("Traders Expansion", ["1.4.0"])) + if (modid === "unreachablemod") return Promise.resolve(detailResponse("Unreachable Mod", ["1.0.0"])) + if (modid === "trulymissing") return Promise.resolve(JSON.stringify({ statuscode: "404" })) + return Promise.reject(new Error(`The popup queried an unexpected mod: ${modid}`)) + } + + it("tells a lookup that failed apart from a clean 404, and counts each on its own note", async () => { + mountWith(async (url) => respond(url.split("/mod/")[1] ?? "", new Set(["unreachablemod"]))) + + expect(within(await rowFor("Unreachable Mod")).getByText("Couldn't reach the mod database")).toBeTruthy() + expect(within(await rowFor("Truly Missing")).getByText("Not on the mod database")).toBeTruthy() + expect(within(await rowFor("Traders Expansion")).getByText("New install")).toBeTruthy() + + // Only the genuine 404 counts toward the fork/private-build note. + expect( + screen.getByText( + "1 mod(s) are not on the mod database: no listing there declares the mod id in any of its releases. Those are most likely forks or private builds, and have to be installed by hand." + ) + ).toBeTruthy() + expect(screen.getByText("1 mod(s) could not be checked: the mod database could not be reached.")).toBeTruthy() + }) + + it("shows the whole pack as unreachable, not a table of forks, when every lookup fails", async () => { + mountWith(async () => Promise.reject(new Error("network down"))) + + expect(await screen.findByText("The mod database could not be reached, so none of these mods could be checked yet.")).toBeTruthy() + // The table, with its "not on the mod database" rows, never renders at all. + expect(screen.queryByText("Status")).toBeNull() + expect(screen.queryByText("Not on the mod database")).toBeNull() + }) + + it("retries the failed lookups, and the table replaces the unreachable state once they answer", async () => { + let down = true + mountWith(async (url) => (down ? Promise.reject(new Error("network down")) : respond(url.split("/mod/")[1] ?? ""))) + + await screen.findByText("The mod database could not be reached, so none of these mods could be checked yet.") + + down = false + fireEvent.click(screen.getByRole("button", { name: "Try again" })) + + expect(within(await rowFor("Traders Expansion")).getByText("New install")).toBeTruthy() + expect(within(await rowFor("Truly Missing")).getByText("Not on the mod database")).toBeTruthy() + expect(screen.queryByText("The mod database could not be reached, so none of these mods could be checked yet.")).toBeNull() + }) + + it("retries a mixed failure back to a resolved row from the note's own retry action", async () => { + let down = true + mountWith(async (url) => respond(url.split("/mod/")[1] ?? "", down ? new Set(["unreachablemod"]) : new Set())) + + expect(within(await rowFor("Unreachable Mod")).getByText("Couldn't reach the mod database")).toBeTruthy() + + down = false + fireEvent.click(screen.getByRole("button", { name: "Try again" })) + + expect(within(await rowFor("Unreachable Mod")).getByText("New install")).toBeTruthy() + }) +}) diff --git a/tests/renderer-dom/manageMods.test.tsx b/tests/renderer-dom/manageMods.test.tsx index 9625a84d..be23ea5e 100644 --- a/tests/renderer-dom/manageMods.test.tsx +++ b/tests/renderer-dom/manageMods.test.tsx @@ -557,7 +557,7 @@ describe("ManageMods: searching the installed Mods", () => { await user.click(plainExport) await waitFor(() => expect(exportModpack).toHaveBeenCalledTimes(1)) - expect(exportModpack.mock.calls[0]?.[0].mods).toEqual([{ modid: "quirkid", version: "4.0.0" }]) + expect(exportModpack.mock.calls[0]?.[0].mods).toEqual([{ modid: "quirkid", version: "4.0.0", name: "Delta Mod" }]) }) it("writes the server modpack from the same visible list that decides the button", async () => { @@ -579,7 +579,7 @@ describe("ManageMods: searching the installed Mods", () => { expect(manifest.name).toBe("Install A (Server)") // Beta and nothing else: not Delta, which is on screen but client-only, and not Alpha or Gamma, // which a server would load but the search took away. - expect(manifest.mods).toEqual([{ modid: "beta", version: "2.0.0" }]) + expect(manifest.mods).toEqual([{ modid: "beta", version: "2.0.0", name: "Beta Mod" }]) }) }) @@ -715,12 +715,12 @@ describe("ManageMods: enabling and disabling a Mod", () => { await user.click(screen.getByText("Export Modpack").closest("button") as HTMLElement) await waitFor(() => expect(exportModpack).toHaveBeenCalledTimes(1)) - expect(exportModpack.mock.calls[0]?.[0].mods).toEqual([{ modid: "alpha", version: "1.0.0" }]) + expect(exportModpack.mock.calls[0]?.[0].mods).toEqual([{ modid: "alpha", version: "1.0.0", name: "Alpha Mod" }]) // Epsilon declares no side, which the server export otherwise reads as "the server loads it". await user.click(screen.getByText("Export Server Modpack").closest("button") as HTMLElement) await waitFor(() => expect(exportModpack).toHaveBeenCalledTimes(2)) - expect(exportModpack.mock.calls[1]?.[0].mods).toEqual([{ modid: "alpha", version: "1.0.0" }]) + expect(exportModpack.mock.calls[1]?.[0].mods).toEqual([{ modid: "alpha", version: "1.0.0", name: "Alpha Mod" }]) }) it("finds a disabled Mod by search like any other, name or id", async () => { @@ -1103,8 +1103,8 @@ describe("ManageMods: filtering the installed Mods", { timeout: 20000 }, () => { await waitFor(() => expect(exportModpack).toHaveBeenCalledTimes(1)) expect(exportModpack.mock.calls[0]?.[0].mods).toEqual([ - { modid: "gamma", version: "3.0.0" }, - { modid: "delta", version: "4.0.0" } + { modid: "gamma", version: "3.0.0", name: "Gamma Mod" }, + { modid: "delta", version: "4.0.0", name: "Delta Mod" } ]) }) }) diff --git a/tests/renderer/importModpackAdapter.test.ts b/tests/renderer/importModpackAdapter.test.ts new file mode 100644 index 00000000..bfbf4d93 --- /dev/null +++ b/tests/renderer/importModpackAdapter.test.ts @@ -0,0 +1,63 @@ +import assert from "node:assert/strict" +import { describe, it } from "vitest" + +import { toModpackManifest } from "../../src/renderer/src/features/mods/adapters/importModpack" +import { MAX_MODPACK_MOD_NAME_LENGTH } from "../../src/domain/mods/importModpack" + +function installation(overrides: Partial = {}): InstallationType { + return { + id: "main", + name: "Main", + icon: "", + path: "/installations/main", + version: "1.20.4", + startParams: "", + backupsLimit: 3, + backupsAuto: false, + compressionLevel: 6, + backups: [], + lastTimePlayed: 0, + totalTimePlayed: 0, + mesaGlThread: false, + envVars: "", + ...overrides + } +} + +function installedMod(overrides: Partial = {}): InstalledModType { + return { name: "Traders Expansion", modid: "tradie", version: "1.4.0", path: "/installations/main/Mods/tradie-1.4.0.zip", enabled: true, ...overrides } +} + +describe("toModpackManifest", () => { + it("writes the local display name next to every modid, which is the only name an unresolvable entry ever gets (#379)", () => { + const manifest = toModpackManifest(installation(), [installedMod(), installedMod({ name: "Sammiches", modid: "sandwich", version: "2.1.0" })]) + + assert.deepEqual(manifest.mods, [ + { modid: "tradie", version: "1.4.0", name: "Traders Expansion" }, + { modid: "sandwich", version: "2.1.0", name: "Sammiches" } + ]) + }) + + it("names the pack after the installation and pins its game version", () => { + const manifest = toModpackManifest(installation({ name: "Co-op pack", version: "1.19.8" }), []) + + assert.deepEqual(manifest, { name: "Co-op pack", gameVersion: "1.19.8", mods: [] }) + }) + + // #384: a modinfo.json name can run up to the scanner's own 4096-character cap, well past the + // 256 the manifest reader accepts. Copying it verbatim used to fail the whole export over one + // label; the writer now clamps it so the export always succeeds. + it("clamps a mod name over the manifest reader's cap instead of failing the export on it", () => { + const longName = "A".repeat(MAX_MODPACK_MOD_NAME_LENGTH + 200) + const manifest = toModpackManifest(installation(), [installedMod({ name: longName })]) + + assert.equal(manifest.mods[0]?.name?.length, MAX_MODPACK_MOD_NAME_LENGTH) + assert.equal(manifest.mods[0]?.name, "A".repeat(MAX_MODPACK_MOD_NAME_LENGTH)) + }) + + it("leaves a name at or under the cap exactly as it was read off disk", () => { + const manifest = toModpackManifest(installation(), [installedMod({ name: "Traders Expansion" })]) + + assert.equal(manifest.mods[0]?.name, "Traders Expansion") + }) +}) diff --git a/tests/text-contrast.test.ts b/tests/text-contrast.test.ts index b2becafc..d00fd8b3 100644 --- a/tests/text-contrast.test.ts +++ b/tests/text-contrast.test.ts @@ -299,6 +299,22 @@ describe("prompts the player is meant to read and act on", () => { assert.deepEqual(arrow, alreadyPresent, "the summary arrow should carry the same grey as the row it sits in") }) + /** + * #384: the downgrade/replace and downloading/update hues joined statusColor's switch as part of + * the readable-table pass, but nothing before this measured them the way #366 measures the + * release verdicts below, which is exactly how text-red-700 shipped at 2.18:1 unnoticed. Both are + * outside the zinc ramp, so they read through paletteForeground/tailwindColor like the verdict + * words rather than through the zinc table at the top of this file. + */ + it("keeps the modpack import row's downgrade and update hues readable on the popup table", () => { + const file = "features/mods/components/ImportModpackPopup.tsx" + const downgradeOrReplace = paletteForeground(file, /case "downgrade":\s*\n\s*case "replace":\s*\n\s*return "text-([a-z]+-\d+)"/) + const downloadingOrUpdate = paletteForeground(file, /case "downloading":\s*\n\s*case "update":\s*\n\s*return "text-([a-z]+-\d+)"/) + + assertReadable("downgrade/replace row hue", downgradeOrReplace, POPUP_TABLE_ROW, TEXT_FLOOR) + assertReadable("downloading/update row hue", downloadingOrUpdate, POPUP_TABLE_ROW, TEXT_FLOOR) + }) + /** * #366: the release table's compatibility verdict. The three hues used to be handed to the * download FormButton through `className`, where the ghost variant's own `text-zinc-200` won the