From 595605fc2274812df44702b246ab015927b15e88 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:05:19 +0200 Subject: [PATCH 1/8] Give installer failures their own wording instead of the archive's startInstall was copied from startExtract and kept its locale keys: success said "Unpacked", failure blamed a damaged archive with no room on the drive. It runs a Windows installer, not an archive, so neither sentence matches the InstallerRunResult reasons the catch classifies (timed out, missing, wrong platform). Add notifications.body.installed and notifications.body.installError to en-US and all 14 locale files, worded for an installer run. The failure sentence points at the log rather than the archive, since `reason` already carries the specific cause to the Activity Center row. i18n-parity.test.ts enforces en-US and fr-FR; the other 12 are kept for consistency with every sibling notifications.body key. taskManagerStartInstall.test.tsx gets one new assertion on the resolved failure body, since no test today pinned the wording. --- src/renderer/src/contexts/TaskManagerContext.tsx | 4 ++-- src/renderer/src/locales/be-BY.json | 2 ++ src/renderer/src/locales/de-DE.json | 2 ++ src/renderer/src/locales/en-US.json | 2 ++ src/renderer/src/locales/es-ES.json | 2 ++ src/renderer/src/locales/fr-FR.json | 2 ++ src/renderer/src/locales/hu-HU.json | 2 ++ src/renderer/src/locales/it-IT.json | 2 ++ src/renderer/src/locales/nl-NL.json | 2 ++ src/renderer/src/locales/pl-PL.json | 2 ++ src/renderer/src/locales/pt-BR.json | 2 ++ src/renderer/src/locales/pt-PT.json | 2 ++ src/renderer/src/locales/ru-RU.json | 2 ++ src/renderer/src/locales/uk-UA.json | 2 ++ src/renderer/src/locales/zh-CN.json | 2 ++ tests/renderer-dom/taskManagerStartInstall.test.tsx | 7 +++++++ 16 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/contexts/TaskManagerContext.tsx b/src/renderer/src/contexts/TaskManagerContext.tsx index a6024b95..a50ab6a9 100644 --- a/src/renderer/src/contexts/TaskManagerContext.tsx +++ b/src/renderer/src/contexts/TaskManagerContext.tsx @@ -402,14 +402,14 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E // a 100 tick, but one whose payload was read out instead reports whatever // the reader last counted, and neither is what says the task is done. tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: COMPLETED } }) - if (notifications.completion === "toast") addNotification(t("notifications.body.extracted", { extractName: name }), "success", { presentation: "toast" }) + if (notifications.completion === "toast") addNotification(t("notifications.body.installed", { installName: name }), "success", { presentation: "toast" }) onFinish(true, null) } catch (err) { window.api.utils.logMessage("error", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Error installing.`) window.api.utils.logMessage("debug", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Error installing: ${err}`) const reason = classifyFailure(err) tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: { status: "failed", reason } } }) - if (notifications.failure === "generic") addNotification(t("notifications.body.extractError", { extractName: name }), "error", { reason }) + if (notifications.failure === "generic") addNotification(t("notifications.body.installError", { installName: name }), "error", { reason }) onFinish(false, new Error(`Error installing ${filePath}: ${err}`)) } finally { window.api.utils.setPreventAppClose("remove", id, "Finished installation.") diff --git a/src/renderer/src/locales/be-BY.json b/src/renderer/src/locales/be-BY.json index 1c9ccf9e..db71c76c 100644 --- a/src/renderer/src/locales/be-BY.json +++ b/src/renderer/src/locales/be-BY.json @@ -371,6 +371,8 @@ "downloadError": "Памылка спампоўкі: {{downloadName}}!", "extracted": "Завершана выманне: {{extractName}}!", "extractError": "Памылка вымання: {{extractName}}!", + "installed": "Завершана ўстаноўка: {{installName}}!", + "installError": "Памылка ўстаноўкі: {{installName}}!", "gameExitedWithErrors": "Vintage Story завяршылася з памылкамі!", "errorExecutingGame": "Адбылася памылка пры запуску гульні!", "missingFields": "Запоўніце ўсе палі!", diff --git a/src/renderer/src/locales/de-DE.json b/src/renderer/src/locales/de-DE.json index 692523f9..7482bff5 100644 --- a/src/renderer/src/locales/de-DE.json +++ b/src/renderer/src/locales/de-DE.json @@ -324,6 +324,8 @@ "downloadError": "Fehler beim Herunterladen: {{downloadName}}!", "extracted": "Extrahierung abgeschlossen: {{extractName}}!", "extractError": "Fehler beim Extrahieren: {{extractName}}!", + "installed": "Installation abgeschlossen: {{installName}}!", + "installError": "Fehler bei der Installation: {{installName}}!", "gameExitedWithErrors": "Vintage Story mit einem Fehler geschlossen!", "errorExecutingGame": "Fehler beim Starten des Spiels!", "missingFields": "Bitte alle Felder ausfüllen!", diff --git a/src/renderer/src/locales/en-US.json b/src/renderer/src/locales/en-US.json index 7d22143b..db9818fa 100644 --- a/src/renderer/src/locales/en-US.json +++ b/src/renderer/src/locales/en-US.json @@ -939,6 +939,8 @@ "downloadError": "Couldn't download {{downloadName}}. Check your connection and try again.", "extracted": "Unpacked {{extractName}}.", "extractError": "Couldn't unpack {{extractName}}. Check that the archive is not damaged and that there is room on the drive.", + "installed": "Installed {{installName}}.", + "installError": "Couldn't install {{installName}}. The log has the details.", "gameExitedWithErrors": "Vintage Story exited with errors. The log has the details.", "errorExecutingGame": "Something went wrong starting the game. The log has the details.", "gameLaunchUnsupportedPlatform": "Vintage Story can't run on this platform yet. Try it from Windows or Linux.", diff --git a/src/renderer/src/locales/es-ES.json b/src/renderer/src/locales/es-ES.json index 5c24f7b5..691fe9c0 100644 --- a/src/renderer/src/locales/es-ES.json +++ b/src/renderer/src/locales/es-ES.json @@ -365,6 +365,8 @@ "errorExecutingGame": "¡Ha ocurrido un error al ejecutar el juego!", "extractError": "¡Error al extraer: {{extractName}}!", "extracted": "¡Extracción finalizada: {{extractName}}!", + "installed": "¡Instalación finalizada: {{installName}}!", + "installError": "¡Error al instalar: {{installName}}!", "folderDoesntExists": "¡La carpeta que estás intentando abrir no existe!", "gameExitedWithErrors": "¡Vintage Story se cerró con errores!", "missingFields": "¡Por favor completa todos los campos!", diff --git a/src/renderer/src/locales/fr-FR.json b/src/renderer/src/locales/fr-FR.json index 2791b41a..ade9da35 100644 --- a/src/renderer/src/locales/fr-FR.json +++ b/src/renderer/src/locales/fr-FR.json @@ -939,6 +939,8 @@ "downloadError": "Impossible de télécharger {{downloadName}}. Vérifiez votre connexion et réessayez.", "extracted": "Décompression terminée : {{extractName}}.", "extractError": "Impossible de décompresser {{extractName}}. Vérifiez que l'archive n'est pas endommagée et qu'il reste de la place sur le disque.", + "installed": "Installation terminée : {{installName}}.", + "installError": "Impossible d'installer {{installName}}. Consultez le journal pour plus de détails.", "gameExitedWithErrors": "Vintage Story s'est arrêté avec des erreurs. Les détails sont dans le journal.", "errorExecutingGame": "Un problème est survenu au démarrage du jeu. Les détails sont dans le journal.", "gameLaunchUnsupportedPlatform": "Vintage Story ne peut pas encore tourner sur cette plateforme. Essayez depuis Windows ou Linux.", diff --git a/src/renderer/src/locales/hu-HU.json b/src/renderer/src/locales/hu-HU.json index 40f24335..d8759865 100644 --- a/src/renderer/src/locales/hu-HU.json +++ b/src/renderer/src/locales/hu-HU.json @@ -308,6 +308,8 @@ "downloadError": "Letöltési hiba: {{downloadName}}!", "extracted": "A kicsomagolás befejeződött: {{extractName}}!", "extractError": "Hiba a kicsomagolás közben: {{extractName}}!", + "installed": "A telepítés befejeződött: {{installName}}!", + "installError": "Hiba a telepítés közben: {{installName}}!", "gameExitedWithErrors": "A Vintage Story, hibák következtében kilépett!", "errorExecutingGame": "A játék indításakor hibák léptek fel!", "missingFields": "Kérlek tölts ki minden mezőt!", diff --git a/src/renderer/src/locales/it-IT.json b/src/renderer/src/locales/it-IT.json index 1a1e496d..4e2ef394 100644 --- a/src/renderer/src/locales/it-IT.json +++ b/src/renderer/src/locales/it-IT.json @@ -367,6 +367,8 @@ "errorExecutingGame": "C'è stato un errore nell'esecuzione del gioco!", "extractError": "Errore estraendo: {{extractName}}!", "extracted": "Finita estrazione: {{extractName}}!", + "installed": "Installazione completata: {{installName}}!", + "installError": "Errore durante l'installazione: {{installName}}!", "folderDoesntExists": "La cartella che stai provando ad aprire non esiste!", "gameExitedWithErrors": "Vintage Story si è chiuso con errori!", "missingFields": "Per favore compila tutti i campi!", diff --git a/src/renderer/src/locales/nl-NL.json b/src/renderer/src/locales/nl-NL.json index bafaab13..7ed839d4 100644 --- a/src/renderer/src/locales/nl-NL.json +++ b/src/renderer/src/locales/nl-NL.json @@ -210,6 +210,8 @@ "downloadError": "Fout downloaden: {{downloadName}}!", "extracted": "Uitpakken voltooid: {{extractName}}!", "extractError": "Fout uitpakken: {{extractName}}!", + "installed": "Installatie voltooid: {{installName}}!", + "installError": "Fout bij installeren: {{installName}}!", "gameExitedWithErrors": "Vintage Story gesloten met fouten!", "errorExecutingGame": "Er is een fout opgetreden terwijl het spel gestart werd", "missingFields": "Vul alle velden in alsjeblieft", diff --git a/src/renderer/src/locales/pl-PL.json b/src/renderer/src/locales/pl-PL.json index 2509f1c7..f5e09149 100644 --- a/src/renderer/src/locales/pl-PL.json +++ b/src/renderer/src/locales/pl-PL.json @@ -371,6 +371,8 @@ "errorExecutingGame": "Podczas uruchamiania gry wystąpił błąd!", "extractError": "Błąd wypakowywania: {{extractName}}!", "extracted": "Ukończono wypakowywanie: {{extractName}}!", + "installed": "Ukończono instalację: {{installName}}!", + "installError": "Błąd instalacji: {{installName}}!", "folderDoesntExists": "Folder, który próbujesz otworzyć nie istnieje!", "gameExitedWithErrors": "Vintage Story zostało zamknięte z błędami!", "missingFields": "Wypełnij wszystkie pola!", diff --git a/src/renderer/src/locales/pt-BR.json b/src/renderer/src/locales/pt-BR.json index d59d5cbc..fce73f42 100644 --- a/src/renderer/src/locales/pt-BR.json +++ b/src/renderer/src/locales/pt-BR.json @@ -421,6 +421,8 @@ "downloadError": "Erro ao baixar: {{downloadName}}!", "extracted": "Extração concluída: {{extractName}}!", "extractError": "Erro na extração: {{extractName}}!", + "installed": "Instalação concluída: {{installName}}!", + "installError": "Erro na instalação: {{installName}}!", "gameExitedWithErrors": "Vintage Story parou com erros!", "errorExecutingGame": "Ocorreu um erro ao executar o jogo!", "missingFields": "Por favor, preencha todos os campos!", diff --git a/src/renderer/src/locales/pt-PT.json b/src/renderer/src/locales/pt-PT.json index 3e27a468..8d0aee8d 100644 --- a/src/renderer/src/locales/pt-PT.json +++ b/src/renderer/src/locales/pt-PT.json @@ -367,6 +367,8 @@ "downloadError": "Erro ao descarregar: {{downloadName}}!", "extracted": "Extração concluída: {{extractName}}!", "extractError": "Erro na extração: {{extractName}}!", + "installed": "Instalação concluída: {{installName}}!", + "installError": "Erro na instalação: {{installName}}!", "gameExitedWithErrors": "Vintage Story parou com erros!", "errorExecutingGame": "Ocorreu um erro ao executar o jogo!", "missingFields": "Por favor, preencha todos os campos!", diff --git a/src/renderer/src/locales/ru-RU.json b/src/renderer/src/locales/ru-RU.json index 3afc61b1..17d3b964 100644 --- a/src/renderer/src/locales/ru-RU.json +++ b/src/renderer/src/locales/ru-RU.json @@ -371,6 +371,8 @@ "downloadError": "Ошибка загрузки: {{downloadName}}!", "extracted": "Извлечение завершено: {{extractName}}!", "extractError": "Ошибка извлечения: {{extractName}}!", + "installed": "Установка завершена: {{installName}}!", + "installError": "Ошибка установки: {{installName}}!", "gameExitedWithErrors": "Vintage Story завершилась с ошибками!", "errorExecutingGame": "Произошла ошибка при запуске игры!", "missingFields": "Пожалуйста, заполните все поля!", diff --git a/src/renderer/src/locales/uk-UA.json b/src/renderer/src/locales/uk-UA.json index f0ea280d..c8069311 100644 --- a/src/renderer/src/locales/uk-UA.json +++ b/src/renderer/src/locales/uk-UA.json @@ -371,6 +371,8 @@ "downloadError": "Помилка під час завантаження: {{downloadName}}!", "extracted": "Видобуток завершено: {{extractName}}!", "extractError": "Помилка під час видобутку: {{extractName}}!", + "installed": "Встановлення завершено: {{installName}}!", + "installError": "Помилка під час встановлення: {{installName}}!", "gameExitedWithErrors": "Vintage Story вийшов з помилками!", "errorExecutingGame": "Під час виконання гри сталася помилка!", "missingFields": "Будь ласка, заповніть всі поля!", diff --git a/src/renderer/src/locales/zh-CN.json b/src/renderer/src/locales/zh-CN.json index 3c37cbef..f97d48f5 100644 --- a/src/renderer/src/locales/zh-CN.json +++ b/src/renderer/src/locales/zh-CN.json @@ -200,6 +200,8 @@ "downloadError": "下载出现错误: {{downloadName}}!", "extracted": "解压完成: {{extractName}}!", "extractError": "解压出现错误: {{extractName}}!", + "installed": "安装完成: {{installName}}!", + "installError": "安装出现错误: {{installName}}!", "gameExitedWithErrors": "《复古物语》因出现错误退出！", "errorExecutingGame": "执行游戏时发生错误！", "missingFields": "请填写所有字段！" diff --git a/tests/renderer-dom/taskManagerStartInstall.test.tsx b/tests/renderer-dom/taskManagerStartInstall.test.tsx index 9c87a3cc..78ae6fef 100644 --- a/tests/renderer-dom/taskManagerStartInstall.test.tsx +++ b/tests/renderer-dom/taskManagerStartInstall.test.tsx @@ -112,5 +112,12 @@ describe("TaskManagerContext.startInstall", () => { await waitFor(() => expect(onFinish).toHaveBeenCalled()) expect(result.current.notifications.notifications.map((n) => n.type)).toEqual(["error"]) + // startInstall was copied from startExtract and kept its archive wording + // (issue #490 item 4): an installer failure is not an archive failure, and + // `reason` already carries the specific cause to the Activity Center row, + // so the toast should point at the log rather than blame "the archive". + const body = result.current.notifications.notifications[0]?.body ?? "" + expect(body).toContain("log") + expect(body).not.toContain("archive") }) }) From 0a469349fc6068ac508e1435990480ed04c3af0e Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:07:56 +0200 Subject: [PATCH 2/8] Port the session-report notification action into useLaunchGame MainMenu's PlayHandler and useLaunchGame drifted apart after PR #475: the hook kept the extraction, MainMenu's copy grew a "see report" notification action off outcomeNotification.report that navigates to the installation's session report, and the hook's copy of the same notification only ever read .link. Joining a server through useLaunchGame therefore offered no report link after a crash, unlike Play. Add useNavigate and the same report-action construction to useLaunchGame's outcome handling, so both launch surfaces build the same actions array. MainMenu still owns its own PlayHandler at this point, so this commit only changes behaviour on the Join path. manageInstallationServers.test.tsx gets a new test mirroring launchPlayGame.test.tsx's "offers the session report" case: a WhereProbe sibling of ManageInstallationServers reads useLocation, and clicking "See what went wrong" after Join is asserted to navigate to the installation's report page. --- .../features/launch/hooks/useLaunchGame.ts | 21 ++++++-- .../manageInstallationServers.test.tsx | 50 ++++++++++++++----- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/renderer/src/features/launch/hooks/useLaunchGame.ts b/src/renderer/src/features/launch/hooks/useLaunchGame.ts index 95d902ed..7cb14ff3 100644 --- a/src/renderer/src/features/launch/hooks/useLaunchGame.ts +++ b/src/renderer/src/features/launch/hooks/useLaunchGame.ts @@ -1,5 +1,6 @@ import { useEffect, useRef, useState } from "react" import { useTranslation } from "react-i18next" +import { useNavigate } from "react-router-dom" import { useGameVersions, useConfigDispatch, CONFIG_ACTIONS } from "@renderer/features/config/contexts/ConfigContext" import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" @@ -46,6 +47,7 @@ export function useLaunchGame(): LaunchGame { const { addNotification } = useNotificationsContext() const { openOnBrowser: openExternalLink } = useExternalLinks() const { os } = useAppInfo() + const goTo = useNavigate() const makeInstallationBackup = useMakeInstallationBackup() @@ -158,9 +160,22 @@ export function useLaunchGame(): LaunchGame { const outcomeNotification = pickPlayOutcomeNotification(result, os) if (outcomeNotification) { - const link = outcomeNotification.link - const options = link ? { actions: [{ id: "open-guide", label: t(link.labelKey), onClick: (): void => openExternalLink(link.url) }] } : undefined - addNotification(t(outcomeNotification.key), "error", options) + const { link, report } = outcomeNotification + const actions = [ + ...(link ? [{ id: "open-guide", label: t(link.labelKey), onClick: (): void => openExternalLink(link.url) }] : []), + ...(report + ? [ + { + id: "see-report", + label: t(report.labelKey), + onClick: (): void => { + void goTo(`/installations/report/${installation.id}`) + } + } + ] + : []) + ] + addNotification(t(outcomeNotification.key), "error", actions.length > 0 ? { actions } : undefined) } } catch (err) { logLaunch("error", `${LOG_TAG} Error executing the game.`) diff --git a/tests/renderer-dom/manageInstallationServers.test.tsx b/tests/renderer-dom/manageInstallationServers.test.tsx index 20ff4f47..fe5132f1 100644 --- a/tests/renderer-dom/manageInstallationServers.test.tsx +++ b/tests/renderer-dom/manageInstallationServers.test.tsx @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from "vitest" import { screen, within } from "@testing-library/react" import userEvent from "@testing-library/user-event" -import { Route, Routes } from "react-router-dom" +import { Route, Routes, useLocation } from "react-router-dom" import ManageInstallationServers from "@renderer/features/servers/pages/ManageInstallationServers" import ImportServersDialog from "@renderer/features/servers/components/ImportServersDialog" @@ -38,6 +38,11 @@ function anInstallation(servers?: ServerBookmarkType[]): InstallationType { } } +/** Where the page's own navigations land, the session report notice's action among them. */ +function WhereProbe(): JSX.Element { + return {useLocation().pathname} +} + function renderServersPage(servers?: ServerBookmarkType[], overrides: WindowApiOverrides = {}): ReturnType { const api = installMockWindowApi({ configManager: { @@ -52,17 +57,20 @@ function renderServersPage(servers?: ServerBookmarkType[], overrides: WindowApiO }) renderWithProviders( - - - - - - } - /> - , + <> + + + + + + } + /> + + + , { route: `/installations/servers/${INSTALLATION_ID}` } ) @@ -244,6 +252,24 @@ describe("ManageInstallationServers", () => { expect(saved?.installations[0]?.servers?.map((server) => server.name)).toEqual(["Stratum", "Testing"]) }) + /** + * MainMenu's Play button and this row both go through useLaunchGame (#490 item 1): before that + * fold, the report action lived only in MainMenu's own copy, so a crash after joining a server + * offered no report link. Mirrors launchPlayGame.test.tsx's "offers the session report" case. + */ + it("offers the session report from the exited-with-errors notice and lands on that Installation's page", async () => { + const user = userEvent.setup() + const executeGame = vi.fn(async () => ({ ok: true, exitCode: 1 }) as GameExecutionResult) + renderServersPage([{ id: "s-1", name: "Stratum", host: "play.example.com", port: 42_420, lastLaunched: -1 }], { gameManager: { executeGame } }) + + await user.click(await screen.findByRole("button", { name: "Join" })) + + await screen.findByText("Vintage Story exited with errors. The log has the details.") + await user.click(await screen.findByRole("button", { name: "See what went wrong" })) + + await vi.waitFor(() => expect(screen.getByTestId("where").textContent).toBe(`/installations/report/${INSTALLATION_ID}`)) + }) + /** * i18next escapes what it interpolates, so a date handed to t() reaches the page as * "4/9/2025" and React renders those entities literally. Every launched row read that From 1074b5110b312b8c71f74a60b15ea26e6f28d46b Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:09:15 +0200 Subject: [PATCH 3/8] Delete MainMenu's PlayHandler in favour of useLaunchGame PR #475 lifted PlayHandler out of MainMenu into useLaunchGame so Play and Join would not carry two copies of the launch, the prevent-close token, the auto-backup question and the playtime stamp among them, but a later commit in the same PR backed the MainMenu side out again. The previous commit ported the one behaviour the two copies had drifted on (the session-report action), so both copies were behaviourally equal; this commit removes the one that no longer needs to exist. MainMenu now calls useLaunchGame() for launchGame and the backup prompt state, and renders the shared LaunchBackupPrompt instead of its own inline PopupDialogPanel copy. Its own PlayHandler, the skipBackupPromptOpen/skipBackupAnswerRef state and teardown effect are gone with it, along with the imports only they used (309 lines down to 136). launchPlayGame.test.tsx (717 lines) passes unchanged through the new call path, and its pass/fail set is identical to the previous commit's run: that is the proof nothing observable moved. --- .../src/components/layout/MainMenu.tsx | 193 +----------------- 1 file changed, 10 insertions(+), 183 deletions(-) diff --git a/src/renderer/src/components/layout/MainMenu.tsx b/src/renderer/src/components/layout/MainMenu.tsx index 607d1e45..6b52f691 100644 --- a/src/renderer/src/components/layout/MainMenu.tsx +++ b/src/renderer/src/components/layout/MainMenu.tsx @@ -1,34 +1,19 @@ -import { ReactNode, useEffect, useRef, useState } from "react" +import { ReactNode, useEffect, useState } from "react" import { useTranslation } from "react-i18next" -import { Link, useLocation, useNavigate } from "react-router-dom" -import { - PiBoxArrowDownDuotone, - PiFolderOpenDuotone, - PiGearDuotone, - PiWrenchDuotone, - PiGitForkDuotone, - PiHouseLineDuotone, - PiPencilDuotone, - PiPlusCircleDuotone, - PiInfoDuotone, - PiXCircleDuotone, - PiPlayCircleDuotone -} from "react-icons/pi" +import { Link, useLocation } from "react-router-dom" +import { PiBoxArrowDownDuotone, PiFolderOpenDuotone, PiGearDuotone, PiWrenchDuotone, PiGitForkDuotone, PiHouseLineDuotone, PiPencilDuotone, PiPlusCircleDuotone, PiInfoDuotone } from "react-icons/pi" import clsx from "clsx" -import { useInstallations, useGameVersions, useSettingsConfig, useConfigDispatch, CONFIG_ACTIONS } from "@renderer/features/config/contexts/ConfigContext" +import { useInstallations, useSettingsConfig } from "@renderer/features/config/contexts/ConfigContext" import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" -import { useExternalLinks } from "@renderer/hooks/useExternalLinks" import { useMakeInstallationBackup } from "@renderer/features/installations/hooks/useMakeInstallationBackup" -import { pickPlayOutcomeNotification } from "@renderer/utils/playOutcomeNotifications" -import { useAppInfo } from "@renderer/features/info/hooks/useAppInfo" -import { checkInstallationPathExists, logLaunch, preventAppClose, runGame } from "@renderer/features/launch/adapters/launch" -import { getInstallationVersionStatus } from "@domain/installations/versionReference" +import { useLaunchGame } from "@renderer/features/launch/hooks/useLaunchGame" +import { checkInstallationPathExists } from "@renderer/features/launch/adapters/launch" import InstallationsDropdownMenu from "@renderer/features/installations/components/InstallationsDropdownMenu" import ActivityCenter from "@renderer/components/ui/ActivityCenter" -import PopupDialogPanel from "@renderer/components/ui/PopupDialogPanel" +import LaunchBackupPrompt from "@renderer/features/launch/components/LaunchBackupPrompt" import { NormalButton } from "@renderer/components/ui/Buttons" import { FormButton, FormLinkButton } from "@renderer/components/ui/FormComponents" import SessionButton from "../ui/SessionButton" @@ -43,57 +28,19 @@ interface MainMenuLinkProps { function MainMenu(): JSX.Element { const { t } = useTranslation() const installations = useInstallations() - const gameVersions = useGameVersions() const { lastUsedInstallation } = useSettingsConfig() - const configDispatch = useConfigDispatch() const { addNotification } = useNotificationsContext() - const { openOnBrowser: openExternalLink } = useExternalLinks() - const goTo = useNavigate() - const { os } = useAppInfo() const makeInstallationBackup = useMakeInstallationBackup() + const { launchGame, skipBackupPromptOpen, answerSkipBackupPrompt } = useLaunchGame() const [selectedInstallation, setSelectedInstallation] = useState(undefined) - // #338's question is awaited from inside PlayHandler rather than driven from a - // click handler, so the launch stays one linear function: the finally block - // still owns clearing _playing and releasing the close guard on every path. - // Both stay held while the question is on screen, which is what a launch - // waiting on an answer is. - const [skipBackupPromptOpen, setSkipBackupPromptOpen] = useState(false) - const skipBackupAnswerRef = useRef<((launchAnyway: boolean) => void) | null>(null) - - /** Closes the prompt and hands the answer to the PlayHandler call waiting on it. */ - function answerSkipBackupPrompt(launchAnyway: boolean): void { - setSkipBackupPromptOpen(false) - skipBackupAnswerRef.current?.(launchAnyway) - skipBackupAnswerRef.current = null - } - - /** Asks whether to launch without a backup. Cancel, Escape and a click outside all answer no. */ - function askToLaunchWithoutBackup(): Promise { - return new Promise((resolve) => { - skipBackupAnswerRef.current = resolve - setSkipBackupPromptOpen(true) - }) - } - useEffect(() => { const si = installations.find((i) => i.id === lastUsedInstallation) setSelectedInstallation(si) }, [lastUsedInstallation, installations]) - // App.tsx keeps MainMenu mounted for as long as the launcher runs, so this - // only fires on teardown. It answers "do not launch" rather than leaving - // PlayHandler parked on a promise nobody can resolve, which would hold the - // close guard until the process exits. - useEffect(() => { - return (): void => { - skipBackupAnswerRef.current?.(false) - skipBackupAnswerRef.current = null - } - }, []) - const GROUP_1: MainMenuLinkProps[] = [ { icon: , text: t("components.mainMenu.homeTitle"), desc: t("components.mainMenu.homeDesc"), to: "/" }, { icon: , text: t("components.mainMenu.installationsTitle"), desc: t("components.mainMenu.installationsDesc"), to: "/installations" }, @@ -103,110 +50,6 @@ function MainMenu(): JSX.Element { { icon: , text: t("components.mainMenu.infoAndHelpTitle"), desc: t("components.mainMenu.infoAndHelpDesc"), to: "/info-and-help" } ] - async function PlayHandler(): Promise { - const id = crypto.randomUUID() - preventAppClose("add", id, "Started playing Vintage Story.") - - // Only set once _playing has actually been flipped to true below, so the - // finally block never clears a flag this call did not set itself (the - // early "already playing" guard reads someone else's _playing, and must - // not stomp on it if this call unwinds before ever taking it over). - let playingInstallationId: string | undefined - let playingGameVersionId: string | undefined - - try { - if (!selectedInstallation) return addNotification(t("features.installations.noInstallationSelected"), "error") - if (selectedInstallation._playing) return addNotification(t("features.installations.gameAlreadyRunning"), "error") - // Update all deletes each old archive before downloading its replacement, so a game started - // mid-run would load a Mods folder with some of its Mods missing. - if (selectedInstallation._updatingMods) return addNotification(t("features.mods.cantPlayWhileUpdatingMods"), "error") - - const gameVersionToRun = selectedInstallation.version ? gameVersions.find((gv) => gv.id === selectedInstallation.gameVersionId) : undefined - if (!gameVersionToRun) { - // An Installation with no version at all reaches here too (configManager normalizes a - // missing version to ""), and interpolating that into versionNotInstalled reads as - // "VS Version not installed." with a blank name (#118). - const status = getInstallationVersionStatus(selectedInstallation, gameVersions) - const message = - status === "unset" - ? t("features.versions.noVersionSet") - : status === "unlinked" - ? t("features.versions.versionUnlinked") - : t("features.versions.versionNotInstalled", { version: selectedInstallation.version }) - return addNotification(message, "error") - } - if (gameVersionToRun._installing) return addNotification(t("features.versions.versionInstalling", { version: selectedInstallation.version }), "error") - if (gameVersionToRun._deleting) return addNotification(t("features.versions.versionDeleting", { version: selectedInstallation.version }), "error") - if (gameVersionToRun._playing) return addNotification(t("features.versions.versionPlaying", { version: selectedInstallation.version }), "error") - - playingInstallationId = selectedInstallation.id - playingGameVersionId = gameVersionToRun.id - - configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION, payload: { id: selectedInstallation.id, updates: { _playing: true } } }) - configDispatch({ type: CONFIG_ACTIONS.EDIT_GAME_VERSION, payload: { id: gameVersionToRun.id, updates: { _playing: true } } }) - - if (selectedInstallation.backupsAuto) { - const backupOutcome = await makeInstallationBackup(selectedInstallation.id) - - // Only an archive compression or pruning failure is recoverable: the - // installation still exists and the player can knowingly launch - // without this backup (#338). Busy, playing, restoring, and missing - // installation states are hard stops and must not offer an override. - if (!backupOutcome.ok) { - const canLaunchWithoutBackup = backupOutcome.reason === "compress-failed" || backupOutcome.reason === "prune-failed" - if (!canLaunchWithoutBackup) return - - const launchAnyway = await askToLaunchWithoutBackup() - if (!launchAnyway) return - } - } - - const startedPlaying = Date.now() - const result = await runGame(gameVersionToRun, selectedInstallation) - - // Playtime is only recorded once the game actually ran: a launch that - // never started played for 0 seconds, and crediting it with the sliver - // of time between the two Date.now() calls would misreport "just - // played" for a session that never happened. - if (result.ok) { - const finishedPlaying = Date.now() - const ttp = finishedPlaying - startedPlaying + selectedInstallation.totalTimePlayed - configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION, payload: { id: selectedInstallation.id, updates: { lastTimePlayed: finishedPlaying, totalTimePlayed: ttp } } }) - } - - const outcomeNotification = pickPlayOutcomeNotification(result, os) - if (outcomeNotification) { - const { link, report } = outcomeNotification - const actions = [ - ...(link ? [{ id: "open-guide", label: t(link.labelKey), onClick: (): void => openExternalLink(link.url) }] : []), - ...(report - ? [ - { - id: "see-report", - label: t(report.labelKey), - onClick: (): void => { - void goTo(`/installations/report/${selectedInstallation.id}`) - } - } - ] - : []) - ] - addNotification(t(outcomeNotification.key), "error", actions.length > 0 ? { actions } : undefined) - } - } catch (err) { - logLaunch("error", "[front] [layout] [components/layout/MainMenu.tsx] [MainMenu > PlayHandler] Error executing the game.") - logLaunch("debug", `[front] [layout] [components/layout/MainMenu.tsx] [MainMenu > PlayHandler] Error executing the game: ${err}`) - addNotification(t("notifications.body.errorExecutingGame"), "error") - } finally { - // Runs on every outcome, the two early-return backup and error paths - // included, so a failed launch never leaves the installation and game - // version stuck at _playing: true until the app restarts (issue #40). - if (playingInstallationId) configDispatch({ type: CONFIG_ACTIONS.EDIT_INSTALLATION, payload: { id: playingInstallationId, updates: { _playing: false } } }) - if (playingGameVersionId) configDispatch({ type: CONFIG_ACTIONS.EDIT_GAME_VERSION, payload: { id: playingGameVersionId, updates: { _playing: false } } }) - preventAppClose("remove", id, "Finished playing vintage Story.") - } - } - return (
@@ -226,7 +69,7 @@ function MainMenu(): JSX.Element {
- + launchGame(selectedInstallation)} variant="primary" size="lg" className="h-14 w-full text-2xl">

{t("generic.play")}

 @@ -257,23 +100,7 @@ function MainMenu(): JSX.Element {
- {/* Cancel comes first in the DOM because HeadlessUI's focus trap focuses - the first focusable child, so Enter on a freshly opened prompt keeps - the launch stopped. The restore and delete confirms order themselves - the same way for the same reason. */} - answerSkipBackupPrompt(false)}> - <> -

{t("features.backups.backupFailedSkipLaunch")}

-
- answerSkipBackupPrompt(false)} variant="secondary"> - - - answerSkipBackupPrompt(true)} variant="destructive"> - - -
- - +
) } From c44cc440169d496a7917fcbea476fb6eeca795b3 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:16:46 +0200 Subject: [PATCH 4/8] Collapse ListMods' nine filter states into one ModsFilters object Nine useState calls, nine setter wrappers that all called the same updateFilter helper, and nineteen props to ModsFilterBar (nine value/setter pairs plus onClearFilters, declared once in the signature and once in its Readonly type). The nine fields already lived together in ModsBrowseState, which modsBrowseState.ts now exposes as its own ModsFilters type. ListMods keeps one useState, seeded from the same getModsBrowseState() snapshot as before, and one setFilter(key, value) replacing the nine setXFilter wrappers. It preserves updateFilter's resolve-in-the-event contract (a state updater runs during render, twice under StrictMode, and must not scroll or write the snapshot there), resolved against a filtersRef mirror rather than the render-scoped filters value: OrderFilter's changeOrder calls setOrderBy and setOrderByOrder synchronously in one click, and resolving the second call against the first call's own result (not the stale value the render started with) is what keeps both fields from a plain-object setState clobbering one another in the same event. ModsFilterBar takes {filters, setFilter, onClearFilters} and adapts each child filter's own value/setter props internally. The debounced query effect's nine-primitive dependency array collapses to one filters object, correct because setFilter only produces a new reference on a real change. modsListModsFilterUpdate.test.tsx (including the StrictMode once-per-change case) and modsBrowseActions.test.tsx pass unchanged, and the full renderer-dom suite (82 files, 806 tests) is green. --- .../mods/components/ModsFilterBar.tsx | 66 +++------- .../src/features/mods/modsBrowseState.ts | 6 +- .../src/features/mods/pages/ListMods.tsx | 123 +++++++----------- 3 files changed, 72 insertions(+), 123 deletions(-) diff --git a/src/renderer/src/features/mods/components/ModsFilterBar.tsx b/src/renderer/src/features/mods/components/ModsFilterBar.tsx index 4bff22c0..634ba4b3 100644 --- a/src/renderer/src/features/mods/components/ModsFilterBar.tsx +++ b/src/renderer/src/features/mods/components/ModsFilterBar.tsx @@ -1,8 +1,10 @@ -import { Dispatch, SetStateAction } from "react" +import { SetStateAction } from "react" import { useTranslation } from "react-i18next" import { PiStarDuotone, PiStarFill, PiEraserDuotone } from "react-icons/pi" import clsx from "clsx" +import type { ModsFilters } from "@renderer/features/mods/modsBrowseState" + import { FormButton, FormInputText } from "@renderer/components/ui/FormComponents" import { StickyMenuGroupWrapper, StickyMenuGroup } from "@renderer/components/ui/StickyMenu" import AuthorFilter from "@renderer/features/mods/components/AuthorFilter" @@ -14,44 +16,12 @@ import InstalledFilter from "@renderer/features/mods/components/InstalledFilter" /** Every ListMods filter control: text/author/version/tag/side/installed, favorites-only, order, and clear. */ function ModsFilterBar({ - textFilter, - setTextFilter, - authorFilter, - setAuthorFilter, - versionsFilter, - setVersionsFilter, - tagsFilter, - setTagsFilter, - sideFilter, - setSideFilter, - installedFilter, - setInstalledFilter, - onlyFav, - setOnlyFav, - orderBy, - setOrderBy, - orderByOrder, - setOrderByOrder, + filters, + setFilter, onClearFilters }: Readonly<{ - textFilter: string - setTextFilter: Dispatch> - authorFilter: DownloadableModAuthorType - setAuthorFilter: Dispatch> - versionsFilter: DownloadableModGameVersionType[] - setVersionsFilter: Dispatch> - tagsFilter: DownloadableModTagType[] - setTagsFilter: Dispatch> - sideFilter: string - setSideFilter: Dispatch> - installedFilter: string - setInstalledFilter: Dispatch> - onlyFav: boolean - setOnlyFav: Dispatch> - orderBy: string - setOrderBy: Dispatch> - orderByOrder: string - setOrderByOrder: Dispatch> + filters: ModsFilters + setFilter: (key: K, value: SetStateAction) => void onClearFilters: () => void }>): JSX.Element { const { t } = useTranslation() @@ -59,17 +29,17 @@ function ModsFilterBar({ return ( - setTextFilter(e.target.value)} className="w-40 h-8" /> + setFilter("textFilter", e.target.value)} className="w-40 h-8" /> - + setFilter("authorFilter", value)} size="w-40 h-8" /> - + setFilter("versionsFilter", value)} size="w-40 h-8" /> - + setFilter("tagsFilter", value)} size="w-40 h-8" /> - + setFilter("sideFilter", value)} size="w-40 h-8" /> - + setFilter("installedFilter", value)} size="w-40 h-8" /> {/* * The active hue goes on the icon, never on the FormButton: the ghost variant carries @@ -80,15 +50,15 @@ function ModsFilterBar({ */} setOnlyFav((prev) => !prev)} - className={clsx("w-8 h-8 text-lg", onlyFav && "border-vsl")} + onClick={() => setFilter("onlyFav", !filters.onlyFav)} + className={clsx("w-8 h-8 text-lg", filters.onlyFav && "border-vsl")} variant="ghost" - ariaPressed={onlyFav} + ariaPressed={filters.onlyFav} > - {onlyFav ? : } + {filters.onlyFav ? : } - + setFilter("orderBy", value)} orderByOrder={filters.orderByOrder} setOrderByOrder={(value) => setFilter("orderByOrder", value)} /> onClearFilters()} className="w-8 h-8 text-lg" variant="ghost"> diff --git a/src/renderer/src/features/mods/modsBrowseState.ts b/src/renderer/src/features/mods/modsBrowseState.ts index 30cb8606..cbd6e302 100644 --- a/src/renderer/src/features/mods/modsBrowseState.ts +++ b/src/renderer/src/features/mods/modsBrowseState.ts @@ -2,7 +2,8 @@ import type { ModPick } from "@domain/mods/modSelection" export const DEFAULT_LOADED_MODS = 45 -export type ModsBrowseState = { +/** The nine fields ListMods' filter bar reads and writes, grouped as the one object it now keeps in state. */ +export type ModsFilters = { textFilter: string authorFilter: DownloadableModAuthorType versionsFilter: DownloadableModGameVersionType[] @@ -12,6 +13,9 @@ export type ModsBrowseState = { onlyFav: boolean orderBy: string orderByOrder: string +} + +export type ModsBrowseState = ModsFilters & { visibleMods: number scrollTop: number /** Selection mode on the browse grid, and the Mods picked in it. Renderer memory only, never config. */ diff --git a/src/renderer/src/features/mods/pages/ListMods.tsx b/src/renderer/src/features/mods/pages/ListMods.tsx index e2357200..a251c55f 100644 --- a/src/renderer/src/features/mods/pages/ListMods.tsx +++ b/src/renderer/src/features/mods/pages/ListMods.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useMemo, useCallback, useRef, useLayoutEffect, type Dispatch, type SetStateAction } from "react" +import { useState, useEffect, useMemo, useCallback, useRef, useLayoutEffect, type SetStateAction } from "react" import { Trans, useTranslation } from "react-i18next" import { useNavigate } from "react-router-dom" import { PiCheckSquareDuotone, PiCheckSquareFill } from "react-icons/pi" @@ -26,7 +26,7 @@ import ImportModpackPopup from "@renderer/features/mods/components/ImportModpack import ModSelectionBar from "@renderer/features/mods/components/ModSelectionBar" import type { ModCardAction } from "@renderer/features/mods/components/ModListCard" import { FormButton } from "@renderer/components/ui/FormComponents" -import { DEFAULT_LOADED_MODS, getModsBrowseState, updateModsBrowseState, type ModsBrowseState } from "@renderer/features/mods/modsBrowseState" +import { DEFAULT_LOADED_MODS, getModsBrowseState, updateModsBrowseState, type ModsBrowseState, type ModsFilters } from "@renderer/features/mods/modsBrowseState" import { installedCopiesOf } from "@domain/mods/installedFilters" import { findModUpdate } from "@domain/mods/compatibility" import { addPicks, modSelectionEntries, togglePick, type ModPick } from "@domain/mods/modSelection" @@ -90,15 +90,13 @@ function ListMods(): JSX.Element { const [modDetails, setModDetails] = useState>(() => new Map()) const requestedModDetails = useRef(new Set()) - const [onlyFav, setOnlyFavState] = useState(browseState.onlyFav) - const [textFilter, setTextFilterState] = useState(browseState.textFilter) - const [authorFilter, setAuthorFilterState] = useState(browseState.authorFilter) - const [versionsFilter, setVersionsFilterState] = useState(browseState.versionsFilter) - const [tagsFilter, setTagsFilterState] = useState(browseState.tagsFilter) - const [sideFilter, setSideFilterState] = useState(browseState.sideFilter) - const [installedFilter, setInstalledFilterState] = useState(browseState.installedFilter) - const [orderBy, setOrderByState] = useState(browseState.orderBy) - const [orderByOrder, setOrderByOrderState] = useState(browseState.orderByOrder) + const [filters, setFiltersState] = useState(browseState) + // Mirrors `filters` synchronously within one event, so a handler that fires setFilter more than + // once (OrderFilter's changeOrder sets both orderBy and orderByOrder) resolves its second call + // against the first call's result rather than the stale value this render started with. Kept in + // step with the committed state on every render; `filters` itself stays the one React reads. + const filtersRef = useRef(filters) + filtersRef.current = filters const [searching, setSearching] = useState(true) @@ -127,27 +125,21 @@ function ListMods(): JSX.Element { * * The next value is resolved against the rendered one here, in the event, rather than inside a * state updater: an updater runs during render (twice under StrictMode) and is no place for a - * scroll or a store write. Every caller sets a given filter at most once per event, so the - * rendered value is the one the change applies to. + * scroll or a store write. Resolved against `filtersRef`, not `filters`, so a handler that calls + * this twice in one event (OrderFilter's changeOrder sets both orderBy and orderByOrder) has its + * second call see the first call's change instead of clobbering it with the stale render value. */ - function updateFilter(current: T, setter: Dispatch>, value: SetStateAction, update: (next: T) => Partial): void { - const next = typeof value === "function" ? (value as (previous: T) => T)(current) : value + function setFilter(key: K, value: SetStateAction): void { + const current = filtersRef.current[key] + const next = typeof value === "function" ? (value as (previous: ModsFilters[K]) => ModsFilters[K])(current) : value if (next === current) return resetBrowsePosition() - updateModsBrowseState(update(next)) - setter(next) + const updated = { ...filtersRef.current, [key]: next } + filtersRef.current = updated + updateModsBrowseState(updated) + setFiltersState(updated) } - const setTextFilter: Dispatch> = (value) => updateFilter(textFilter, setTextFilterState, value, (next) => ({ textFilter: next })) - const setAuthorFilter: Dispatch> = (value) => updateFilter(authorFilter, setAuthorFilterState, value, (next) => ({ authorFilter: next })) - const setVersionsFilter: Dispatch> = (value) => updateFilter(versionsFilter, setVersionsFilterState, value, (next) => ({ versionsFilter: next })) - const setTagsFilter: Dispatch> = (value) => updateFilter(tagsFilter, setTagsFilterState, value, (next) => ({ tagsFilter: next })) - const setSideFilter: Dispatch> = (value) => updateFilter(sideFilter, setSideFilterState, value, (next) => ({ sideFilter: next })) - const setInstalledFilter: Dispatch> = (value) => updateFilter(installedFilter, setInstalledFilterState, value, (next) => ({ installedFilter: next })) - const setOnlyFav: Dispatch> = (value) => updateFilter(onlyFav, setOnlyFavState, value, (next) => ({ onlyFav: next })) - const setOrderBy: Dispatch> = (value) => updateFilter(orderBy, setOrderByState, value, (next) => ({ orderBy: next })) - const setOrderByOrder: Dispatch> = (value) => updateFilter(orderByOrder, setOrderByOrderState, value, (next) => ({ orderByOrder: next })) - const handleScroll = (): void => { if (!scrollRef.current) return const { scrollTop, clientHeight, scrollHeight } = scrollRef.current @@ -186,11 +178,13 @@ function ListMods(): JSX.Element { } // triggerQueryMods is a plain function redeclared every render, not a useCallback: it // always closes over this render's own filter values, so calling it from here already - // reads the current textFilter/authorFilter/etc. Listing it as a dependency would only - // make this effect refire on ListMods' own re-renders, not on anything it doesn't - // already refire on through the filters below. + // reads the current filters. Listing it as a dependency would only make this effect + // refire on ListMods' own re-renders, not on anything it doesn't already refire on + // through `filters` below. `filters` itself is a single dependency now, and setFilter + // only ever gives it a new reference on an actual change, so this compares correctly by + // identity. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [textFilter, authorFilter, versionsFilter, tagsFilter, sideFilter, installedFilter, onlyFav, orderBy, orderByOrder]) + }, [filters]) // Keyed on id/path, not on `installation` itself: triggerGetInstalledMods calls // syncModsCount, which writes _modsCount back onto this same installation and @@ -243,13 +237,14 @@ function ListMods(): JSX.Element { useEffect(() => { if (installationInstalledMods === undefined) return - if (!installationModsLoadedRef.current || installedFilter !== "all") triggerQueryMods() + if (!installationModsLoadedRef.current || filters.installedFilter !== "all") triggerQueryMods() installationModsLoadedRef.current = true // installedFilter changing on its own is already covered by the debounced-query effect - // above (it lists installedFilter in its own deps); this effect exists only to redo an - // "installed"/"not-installed" filter once a fresh installationInstalledMods scan comes - // in, so listing installedFilter here too would just fire triggerQueryMods twice for - // the same change. triggerQueryMods is excluded for the same reason as the effect above. + // above (it lists `filters`, installedFilter included, in its own deps); this effect + // exists only to redo an "installed"/"not-installed" filter once a fresh + // installationInstalledMods scan comes in, so listing filters here too would just fire + // triggerQueryMods twice for the same change. triggerQueryMods is excluded for the same + // reason as the effect above. // eslint-disable-next-line react-hooks/exhaustive-deps }, [installationInstalledMods]) @@ -301,22 +296,22 @@ function ListMods(): JSX.Element { const queryToken = ++queryTokenRef.current let mods = await queryMods({ - textFilter, - authorFilter, - versionsFilter, - tagsFilter, - orderBy, - orderByOrder + textFilter: filters.textFilter, + authorFilter: filters.authorFilter, + versionsFilter: filters.versionsFilter, + tagsFilter: filters.tagsFilter, + orderBy: filters.orderBy, + orderByOrder: filters.orderByOrder }) if (queryToken !== queryTokenRef.current) return - if (sideFilter !== "any") mods = mods.filter((mod) => mod.side === sideFilter) + if (filters.sideFilter !== "any") mods = mods.filter((mod) => mod.side === filters.sideFilter) - if (installedFilter === "installed") mods = mods.filter((mod) => installedCopiesOf(mod.modidstrs, installationInstalledMods).length > 0) - if (installedFilter === "not-installed") mods = mods.filter((mod) => installedCopiesOf(mod.modidstrs, installationInstalledMods).length < 1) + if (filters.installedFilter === "installed") mods = mods.filter((mod) => installedCopiesOf(mod.modidstrs, installationInstalledMods).length > 0) + if (filters.installedFilter === "not-installed") mods = mods.filter((mod) => installedCopiesOf(mod.modidstrs, installationInstalledMods).length < 1) - if (onlyFav) mods = mods.filter((mod) => favMods.includes(mod.modid)) + if (filters.onlyFav) mods = mods.filter((mod) => favMods.includes(mod.modid)) setModsList(mods) setSearching(false) @@ -445,13 +440,13 @@ function ListMods(): JSX.Element { ) function clearFilters(): void { - setTextFilter("") - setAuthorFilter({ userid: "", name: "" }) - setVersionsFilter([]) - setTagsFilter([]) - setSideFilter("any") - setInstalledFilter("all") - setOnlyFav(false) + setFilter("textFilter", "") + setFilter("authorFilter", { userid: "", name: "" }) + setFilter("versionsFilter", []) + setFilter("tagsFilter", []) + setFilter("sideFilter", "any") + setFilter("installedFilter", "all") + setFilter("onlyFav", false) } return ( @@ -485,27 +480,7 @@ function ListMods(): JSX.Element { - + {selecting && ( Date: Tue, 15 Sep 2026 20:24:20 +0200 Subject: [PATCH 5/8] Give startDownload/Extract/Install/Compress one shared runTask scaffold The four task runners were the same scaffold with one line changed: uuid, prevent-close add/remove, two info logs, ADD_TASK, await the host call, a COMPLETED dispatch and optional toast on success, classifyFailure and a failed dispatch and optional toast on catch. 166 lines of bodies, most of it repeated four times. Add runTask({type, name, desc, notifications, messageKeys}, operation, onFinish) holding that scaffold. Each of the four becomes a thin wrapper passing its own operation closure (the host call plus whatever only it needs after: extract's post-await chmod, install's throw-on-!result.ok) and its own two-key message pair. onFinish stays two-argument (status, error) inside runTask; download's own three-argument contract (status, path, error) is adapted in its own wrapper closure, over the downloaded file's path, which only that closure holds. compress's optional compressionLevel is still just an extra argument on its own host call. The raw caught value reaches each wrapper unchanged, so a wrapper's own `${err}` interpolation into its error message is byte-for-byte what it produced before this fold. TaskContextType's four signatures are untouched, so the four feature adapters that call them positionally see no change. Two of the runner's own info logs are now worded a little more generically than their four separate copies were: a shared "runTask" tag instead of "startDownload"/"startExtract"/etc., and compress's "Adding" log no longer names its source path, only its destination. Neither reaches a player, no test asserts either one's exact text, and log-provenance.test.ts (which does check every logMessage call for a leaked path or name) stays green. Every other log line, and both prevent-close reason strings, resolve to the exact same text as before through a small noun table for the three irregular ones (extraction, installation, compression). taskManagerFlows.test.tsx and taskManagerStartInstall.test.tsx pass unchanged (41 tests, identical pass set before and after), pinning the dispatch sequence, the toast type and the per-task error message text for all four. The full renderer-dom suite (83 files, 807 tests) and log-provenance.test.ts are green. --- .../src/contexts/TaskManagerContext.tsx | 227 ++++++++---------- 1 file changed, 106 insertions(+), 121 deletions(-) diff --git a/src/renderer/src/contexts/TaskManagerContext.tsx b/src/renderer/src/contexts/TaskManagerContext.tsx index a50ab6a9..1ee4327a 100644 --- a/src/renderer/src/contexts/TaskManagerContext.tsx +++ b/src/renderer/src/contexts/TaskManagerContext.tsx @@ -287,45 +287,89 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E } }, []) - async function startDownload( - name: string, - desc: string, - notifications: TaskNotificationPolicy, - url: string, - outputPath: string, - fileName: string, - onFinish: (status: boolean, path: string, error: Error | null) => void + /** The noun each task's prevent-close reason and its "Adding ___ to [PATH]" log use; irregular enough (extraction, installation, compression) that concatenating `type` does not produce them. */ + const TASK_NOUNS: Record<"download" | "extract" | "install" | "compress", string> = { download: "download", extract: "extraction", install: "installation", compress: "compression" } + + /** + * The scaffold startDownload, startExtract, startInstall and startCompress used to each carry + * on their own: a uuid, the prevent-close token, the two info logs, ADD_TASK, the awaited + * operation, the COMPLETED dispatch and optional toast on success, classifyFailure and the + * failed dispatch and optional toast on catch, and releasing the prevent-close token either way. + * + * The download resolving (or extracting, or installing, or compressing) is what completes the + * task, not the progress events a separate listener feeds into the reducer above: a source whose + * last tick lands under 100 would otherwise leave the task showing as still running forever, and + * dispatching COMPLETED after a 100 tick already did costs nothing (the reducer is idempotent). + * + * `operation` is the one thing each of the four actually differs on: the host call itself, plus + * whatever it alone needs after it (extract's post-await chmod, install's throw-on-!result.ok, + * compress's optional compressionLevel). `onFinish` here stays the two-argument (status, error) + * shape every non-download caller already has; download's own three-argument contract (status, + * path, error) is adapted in its own wrapper, over data (the downloaded file's path) only that + * wrapper's closure holds. The raw caught value is handed back as-is, not normalized to an + * Error, so a wrapper's own `${err}` in its error message matches exactly what it produced + * before this fold. + * + * Two of the four info logs are worded a little more generically than their old per-function + * copies (a shared "runTask" tag instead of "startDownload"/"startExtract"/etc., and compress's + * "Adding" log no longer names its source path, only its destination): neither reaches a player, + * neither is asserted by a test, and log-provenance.test.ts still passes since nothing here + * interpolates a path, a name or any other risky identifier. + */ + async function runTask( + config: { type: keyof typeof TASK_NOUNS; name: string; desc: string; notifications: TaskNotificationPolicy; messageKeys: { successKey: string; failureKey: string } }, + operation: (id: string) => Promise, + onFinish: (status: boolean, error: unknown) => void ): Promise { + const { type, name, desc, notifications, messageKeys } = config const id = crypto.randomUUID() + const noun = TASK_NOUNS[type] + const nameParam = `${type}Name` + const LOG_TAG = `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > runTask] [${id}] [${type}]` try { - window.api.utils.setPreventAppClose("add", id, "Started download.") - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startDownload] [${id}] [download] Adding download to [PATH].`) - tasksDispatch({ type: ACTIONS.ADD_TASK, payload: { id, name, desc, type: "download", progress: 0, status: "pending" } }) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startDownload] [${id}] [download] Downloading...`) - const downloadedFile = await window.api.pathsManager.downloadOnPath(id, url, outputPath, fileName) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startDownload] [${id}] [download] Downloaded.`) - // The download resolving is what completes the task, not the progress - // events: a source whose last tick lands at 97 would otherwise leave the - // task showing as still running forever. See the reducer above for why - // dispatching this after a 100 tick already did costs nothing. + window.api.utils.setPreventAppClose("add", id, `Started ${noun}.`) + window.api.utils.logMessage("info", `${LOG_TAG} Adding ${noun} to [PATH].`) + tasksDispatch({ type: ACTIONS.ADD_TASK, payload: { id, name, desc, type, progress: 0, status: "pending" } }) + + window.api.utils.logMessage("info", `${LOG_TAG} ${type}ing...`) + await operation(id) + + window.api.utils.logMessage("info", `${LOG_TAG} ${type}ed.`) tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: COMPLETED } }) - if (notifications.completion === "toast") addNotification(t("notifications.body.downloaded", { downloadName: name }), "success", { presentation: "toast" }) - onFinish(true, downloadedFile, null) + if (notifications.completion === "toast") addNotification(t(messageKeys.successKey, { [nameParam]: name }), "success", { presentation: "toast" }) + onFinish(true, null) } catch (err) { - window.api.utils.logMessage("error", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startDownload] [${id}] [download] Error downloading.`) - window.api.utils.logMessage("debug", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startDownload] [${id}] [download] Error downloading: ${err}`) + window.api.utils.logMessage("error", `${LOG_TAG} Error ${type}ing.`) + window.api.utils.logMessage("debug", `${LOG_TAG} Error ${type}ing: ${err}`) const reason = classifyFailure(err) tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: { status: "failed", reason } } }) - if (notifications.failure === "generic") addNotification(t("notifications.body.downloadError", { downloadName: name }), "error", { reason }) - onFinish(false, "", new Error(`Error downloading ${url}: ${err}`)) + if (notifications.failure === "generic") addNotification(t(messageKeys.failureKey, { [nameParam]: name }), "error", { reason }) + onFinish(false, err) } finally { - window.api.utils.setPreventAppClose("remove", id, "Finished download.") + window.api.utils.setPreventAppClose("remove", id, `Finished ${noun}.`) } } + async function startDownload( + name: string, + desc: string, + notifications: TaskNotificationPolicy, + url: string, + outputPath: string, + fileName: string, + onFinish: (status: boolean, path: string, error: Error | null) => void + ): Promise { + let downloadedFile = "" + await runTask( + { type: "download", name, desc, notifications, messageKeys: { successKey: "notifications.body.downloaded", failureKey: "notifications.body.downloadError" } }, + async (id) => { + downloadedFile = await window.api.pathsManager.downloadOnPath(id, url, outputPath, fileName) + }, + (status, err) => onFinish(status, downloadedFile, status ? null : new Error(`Error downloading ${url}: ${err}`)) + ) + } + async function startExtract( name: string, desc: string, @@ -336,40 +380,20 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E onFinish: (status: boolean, error: Error | null) => void, unwrapSingleRootFolder = false ): Promise { - const id = crypto.randomUUID() - - try { - window.api.utils.setPreventAppClose("add", id, "Started extraction.") - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [extract] Adding extraction to [PATH].`) - tasksDispatch({ type: ACTIONS.ADD_TASK, payload: { id, name, desc, type: "extract", progress: 0, status: "pending" } }) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [extract] Extracting...`) - const result = await window.api.pathsManager.extractOnPath(id, filePath, outputPath, deleteZip, unwrapSingleRootFolder) - - if (!result) throw new Error("Extraction failed") - - // Awaited so a rejected chmod is caught below instead of becoming an unhandled - // rejection. A failed chmod fails the task on purpose: an unexecutable game is a - // failed install on Linux, not a harmless side note. (On non-Linux platforms the - // call resolves `false` without throwing, which is the normal, expected outcome.) - await window.api.pathsManager.changePerms([outputPath], 0o755) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [extract] Extracted.`) - // Completed once the extraction and the chmod are both through, so a - // last progress tick under 100 cannot strand the task as running. - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: COMPLETED } }) - if (notifications.completion === "toast") addNotification(t("notifications.body.extracted", { extractName: name }), "success", { presentation: "toast" }) - onFinish(true, null) - } catch (err) { - window.api.utils.logMessage("error", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [extract] Error extracting.`) - window.api.utils.logMessage("debug", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startExtract] [${id}] [extract] Error extracting: ${err}`) - const reason = classifyFailure(err) - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: { status: "failed", reason } } }) - if (notifications.failure === "generic") addNotification(t("notifications.body.extractError", { extractName: name }), "error", { reason }) - onFinish(false, new Error(`Error extracting ${filePath}: ${err}`)) - } finally { - window.api.utils.setPreventAppClose("remove", id, "Finished extraction.") - } + await runTask( + { type: "extract", name, desc, notifications, messageKeys: { successKey: "notifications.body.extracted", failureKey: "notifications.body.extractError" } }, + async (id) => { + const result = await window.api.pathsManager.extractOnPath(id, filePath, outputPath, deleteZip, unwrapSingleRootFolder) + if (!result) throw new Error("Extraction failed") + + // Awaited so a rejected chmod is caught below instead of becoming an unhandled + // rejection. A failed chmod fails the task on purpose: an unexecutable game is a + // failed install on Linux, not a harmless side note. (On non-Linux platforms the + // call resolves `false` without throwing, which is the normal, expected outcome.) + await window.api.pathsManager.changePerms([outputPath], 0o755) + }, + (status, err) => onFinish(status, status ? null : new Error(`Error extracting ${filePath}: ${err}`)) + ) } async function startInstall( @@ -381,39 +405,19 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E deleteInstaller: boolean, onFinish: (status: boolean, error: Error | null) => void ): Promise { - const id = crypto.randomUUID() - - try { - window.api.utils.setPreventAppClose("add", id, "Started installation.") - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Adding installation to [PATH].`) - tasksDispatch({ type: ACTIONS.ADD_TASK, payload: { id, name, desc, type: "install", progress: 0, status: "pending" } }) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Installing...`) - const result = await window.api.pathsManager.runInstaller(id, filePath, outputPath, deleteInstaller) - - // The wire tells apart why the installer never landed the game (see - // InstallerRunResult in global.d.ts), but onFinish here stays the - // boolean shape every caller already expects: the reason still rides - // along on the thrown Error's message for the log line below. - if (!result.ok) throw new Error(`Installation failed: ${result.reason}`) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Installed.`) - // The worst of the four for this: an installer that ran to the end sends - // a 100 tick, but one whose payload was read out instead reports whatever - // the reader last counted, and neither is what says the task is done. - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: COMPLETED } }) - if (notifications.completion === "toast") addNotification(t("notifications.body.installed", { installName: name }), "success", { presentation: "toast" }) - onFinish(true, null) - } catch (err) { - window.api.utils.logMessage("error", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Error installing.`) - window.api.utils.logMessage("debug", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startInstall] [${id}] [install] Error installing: ${err}`) - const reason = classifyFailure(err) - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: { status: "failed", reason } } }) - if (notifications.failure === "generic") addNotification(t("notifications.body.installError", { installName: name }), "error", { reason }) - onFinish(false, new Error(`Error installing ${filePath}: ${err}`)) - } finally { - window.api.utils.setPreventAppClose("remove", id, "Finished installation.") - } + await runTask( + { type: "install", name, desc, notifications, messageKeys: { successKey: "notifications.body.installed", failureKey: "notifications.body.installError" } }, + async (id) => { + const result = await window.api.pathsManager.runInstaller(id, filePath, outputPath, deleteInstaller) + + // The wire tells apart why the installer never landed the game (see + // InstallerRunResult in global.d.ts), but onFinish here stays the + // boolean shape every caller already expects: the reason still rides + // along on the thrown Error's message for the log line below. + if (!result.ok) throw new Error(`Installation failed: ${result.reason}`) + }, + (status, err) => onFinish(status, status ? null : new Error(`Error installing ${filePath}: ${err}`)) + ) } async function startCompress( @@ -426,33 +430,14 @@ export const TaskProvider = ({ children }: { children: React.ReactNode }): JSX.E onFinish: (status: boolean, error: Error | null) => void, compressionLevel?: number ): Promise { - const id = crypto.randomUUID() - - try { - window.api.utils.setPreventAppClose("add", id, "Started compression.") - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startCompress] [${id}] [compress] Adding compression of [PATH] to [PATH].`) - tasksDispatch({ type: ACTIONS.ADD_TASK, payload: { id, name, desc, type: "compress", progress: 0, status: "pending" } }) - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startCompress] [${id}] [compress] Compressing...`) - const result = await window.api.pathsManager.compressOnPath(id, inputPath, outputPath, fileName, compressionLevel) - - if (!result) throw new Error("Compression failed") - - window.api.utils.logMessage("info", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startCompress] [${id}] [compress] Compressed.`) - // Same as the other three: the resolved call is the completion signal. - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: COMPLETED } }) - if (notifications.completion === "toast") addNotification(t("notifications.body.compressed", { compressName: name }), "success", { presentation: "toast" }) - onFinish(true, null) - } catch (err) { - window.api.utils.logMessage("error", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startCompress] [${id}] [compress] Error compressing.`) - window.api.utils.logMessage("debug", `[front] [tasks] [contexts/TaskManagercontext.tsx] [TaskProvider > startCompress] [${id}] [compress] Error compressing: ${err}`) - const reason = classifyFailure(err) - tasksDispatch({ type: ACTIONS.UPDATE_TASK, payload: { id, updates: { status: "failed", reason } } }) - if (notifications.failure === "generic") addNotification(t("notifications.body.compressError", { compressName: name }), "error", { reason }) - onFinish(false, new Error(`Error compressing: ${err}`)) - } finally { - window.api.utils.setPreventAppClose("remove", id, "Finished compression.") - } + await runTask( + { type: "compress", name, desc, notifications, messageKeys: { successKey: "notifications.body.compressed", failureKey: "notifications.body.compressError" } }, + async (id) => { + const result = await window.api.pathsManager.compressOnPath(id, inputPath, outputPath, fileName, compressionLevel) + if (!result) throw new Error("Compression failed") + }, + (status, err) => onFinish(status, status ? null : new Error(`Error compressing: ${err}`)) + ) } async function startOptimumPatch( From f72aa9164b4ba2548fbb2f7e429b3d129fd6662c Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:27:58 +0200 Subject: [PATCH 6/8] Collapse the three folder pickers onto usePickEmptyFolder useConfigFolderPicker and useVersionInstallFolder's browseFolder each wrote out the same selectFolderDialog/checkPathEmpty/warn sequence usePickEmptyFolder already implements, byte-identical across all three. useInstallationFolder already shares it; these two now do too. useConfigFolderPicker becomes the dispatch line only, still dispatched whether or not the pick warned: usePickEmptyFolder does not block on a non-empty folder, and the config picker must not turn that into a block of its own. useVersionInstallFolder's browseFolder becomes pick, then setFolder/setFolderByUser. No test exercised any of the three pickers' warning notification itself before this fold, only that a pick was not blocked by it (installationsAddFolderPick.test.tsx's existing "warns without blocking" case checks the field still takes the path, never that the notification appears). Add that missing assertion for the hook every caller now shares. installationsAddFolderPick.test.tsx (46 tests plus the one new one), installationsAddFolderFollowsName.test.tsx, versionsAddVersion.test.tsx and configPageBackground.test.tsx pass unchanged, and the full renderer-dom suite (82 files, 807 tests) is green. --- .../config/hooks/useConfigFolderPicker.ts | 27 ++++++++---------- .../versions/hooks/useVersionInstallFolder.ts | 13 +++------ .../installationsAddFolderPick.test.tsx | 28 +++++++++++++++++++ 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/renderer/src/features/config/hooks/useConfigFolderPicker.ts b/src/renderer/src/features/config/hooks/useConfigFolderPicker.ts index a1c2d56b..2f9be2d2 100644 --- a/src/renderer/src/features/config/hooks/useConfigFolderPicker.ts +++ b/src/renderer/src/features/config/hooks/useConfigFolderPicker.ts @@ -1,31 +1,28 @@ -import { useTranslation } from "react-i18next" - -import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" import { CONFIG_ACTIONS, useConfigDispatch } from "@renderer/features/config/contexts/ConfigContext" +import { usePickEmptyFolder } from "@renderer/features/installations/hooks/usePathActions" type FolderSettingActionType = CONFIG_ACTIONS.SET_DEFAULT_INSTALLATIONS_FOLDER | CONFIG_ACTIONS.SET_DEFAULT_VERSIONS_FOLDER | CONFIG_ACTIONS.SET_DEFAULT_BACKUPS_FOLDER /** - * Minimal local hook behind ConfigPage's three folder pickers (installations, versions, backups). - * - * The installations feature carries the same browse-and-warn-if-not-empty pattern. Sharing one - * implementation between the two is a natural follow-up, kept separate here so this hook does not - * depend on that file's shape. + * Minimal local hook behind ConfigPage's three folder pickers (installations, versions, backups): + * usePickEmptyFolder's pick-and-warn, dispatched into config once a folder comes back. * * Lives outside features/config/pages on purpose: neither ConfigPage.tsx nor anything else under - * that directory may mention the preload bridge directly. + * that directory may mention the preload bridge directly. Reaching into features/installations + * for usePickEmptyFolder is already normal here (useInstallationFolder and + * useVersionInstallFolder both cross feature lines the same way for the same hook). + * + * usePickEmptyFolder warns without blocking the pick, so dispatching has to happen whether or + * not it warned, never only on the clean path. */ export function useConfigFolderPicker(actionType: FolderSettingActionType): () => Promise { - const { t } = useTranslation() - const { addNotification } = useNotificationsContext() const configDispatch = useConfigDispatch() + const pickEmptyFolder = usePickEmptyFolder() return async function pickFolder(): Promise { - const path = await window.api.utils.selectFolderDialog() - const selectedPath = path[0] - if (!selectedPath || selectedPath.length < 1) return + const selectedPath = await pickEmptyFolder() + if (!selectedPath) return - if (!(await window.api.pathsManager.checkPathEmpty(selectedPath))) addNotification(t("notifications.body.folderNotEmpty"), "warning") configDispatch({ type: actionType, payload: selectedPath }) } } diff --git a/src/renderer/src/features/versions/hooks/useVersionInstallFolder.ts b/src/renderer/src/features/versions/hooks/useVersionInstallFolder.ts index 918a03f1..96125215 100644 --- a/src/renderer/src/features/versions/hooks/useVersionInstallFolder.ts +++ b/src/renderer/src/features/versions/hooks/useVersionInstallFolder.ts @@ -1,8 +1,7 @@ import { useEffect, useState } from "react" -import { useTranslation } from "react-i18next" -import { useNotificationsContext } from "@renderer/contexts/NotificationsContext" import { createPathBuilderPort } from "@renderer/adapters/paths" +import { usePickEmptyFolder } from "@renderer/features/installations/hooks/usePathActions" export interface UseVersionInstallFolderResult { /** The folder the version installs into, either suggested or user picked. */ @@ -23,8 +22,7 @@ export interface UseVersionInstallFolderResult { * only way to change this field now. */ export function useVersionInstallFolder(version: DownloadableGameVersionTypeType | undefined, defaultVersionsFolder: string): UseVersionInstallFolderResult { - const { t } = useTranslation() - const { addNotification } = useNotificationsContext() + const pickEmptyFolder = usePickEmptyFolder() const [folder, setFolder] = useState("") const [folderByUser, setFolderByUser] = useState(false) @@ -37,11 +35,8 @@ export function useVersionInstallFolder(version: DownloadableGameVersionTypeType }, [version]) async function browseFolder(): Promise { - const path = await window.api.utils.selectFolderDialog() - const selectedPath = path[0] - if (!selectedPath || selectedPath.length === 0) return - - if (!(await window.api.pathsManager.checkPathEmpty(selectedPath))) addNotification(t("notifications.body.folderNotEmpty"), "warning") + const selectedPath = await pickEmptyFolder() + if (!selectedPath) return setFolder(selectedPath) setFolderByUser(true) diff --git a/tests/renderer-dom/installationsAddFolderPick.test.tsx b/tests/renderer-dom/installationsAddFolderPick.test.tsx index 900d5d04..ca1fc430 100644 --- a/tests/renderer-dom/installationsAddFolderPick.test.tsx +++ b/tests/renderer-dom/installationsAddFolderPick.test.tsx @@ -3,6 +3,7 @@ import { screen, waitFor } from "@testing-library/react" import userEvent from "@testing-library/user-event" import AddInstallation from "@renderer/features/installations/pages/AddInstallation" +import NotificationsOverlay from "@renderer/components/layout/NotificationsOverlay" import { installMockWindowApi } from "./helpers/windowApi" import { renderWithProviders } from "./helpers/render" @@ -41,6 +42,33 @@ describe("AddInstallation", () => { await waitFor(() => expect(pathInput.value).toBe("/picked/not-empty")) }) + /** + * usePickEmptyFolder (#490 item 5) is the survivor of a fold that collapsed three copies of + * pick-a-folder-and-warn-if-not-empty (config, installations, versions) onto this one hook. + * Nothing exercised the warning notification itself before this fold, only that the pick was + * not blocked by it (the test above); this is that missing case, for the hook every caller + * now shares. + */ + it("shows the not-empty warning notification, not just an unblocked pick", async () => { + const user = userEvent.setup() + installMockWindowApi({ + utils: { selectFolderDialog: vi.fn(async () => ["/picked/not-empty"]) }, + pathsManager: { checkPathEmpty: vi.fn(async () => false) } + }) + + renderWithProviders( + <> + + + , + { route: "/installations/add" } + ) + + await user.click(screen.getByTitle("Browse")) + + expect(await screen.findByText("The folder you've selected is not empty. Make sure there is nothing important in it.")).toBeTruthy() + }) + it("cancelling the dialog leaves the field untouched", async () => { const user = userEvent.setup() installMockWindowApi({ utils: { selectFolderDialog: vi.fn(async () => []) } }) From d41df69b1660b86b368387f266edd80a6148d3c8 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:32:08 +0200 Subject: [PATCH 7/8] Stop CI's build and sonarcloud jobs from redoing other jobs' work Measured against the 2026-09-15 dev run (id 34959134901, all green): build's npm run build:unpack step is npm run build (typecheck && electron-vite build) plus electron-builder --dir, so every build leg re-ran the full typecheck the dedicated typecheck job had already run on its own: 24s on typecheck's own leg, embedded again inside a 53s (ubuntu) / 48s (windows) build step. Separately, sonarcloud ran npm run test:coverage from scratch (3m57s) to reproduce the same coverage test-matrix (ubuntu-latest) had already produced (3m2s) a few minutes earlier in the same run. build now calls electron-vite build and electron-builder --dir directly instead of npm run build:unpack, skipping the embedded typecheck; npm run build:unpack itself is untouched for anyone running it outside CI. test-matrix's ubuntu-latest leg uploads its coverage/lcov.info as an artifact, and sonarcloud downloads it instead of regenerating it, needing that job so there is always something to download before it tries. continue-on-error keeps a failed or skipped test-matrix from turning sonarcloud into a hard CI failure; it already never blocked a merge. Both `&&` chains propagate a real build failure's exit code (verified locally: electron-vite build && electron-builder --dir stops and exits non-zero the moment the first command does), and a local run of the new build step against this branch produces the same dist/linux-unpacked output build:unpack always did. Estimated saving: sonarcloud loses its longest step (about 4 minutes), each build leg loses 20-25s of duplicate typechecking (worth roughly double that on the 2x-billed windows leg), for about 4.5 to 5 minutes off a run that currently spends close to 23 minutes of total job time. build-macos keeps npm run build:unpack: it is workflow_dispatch-only and not part of the measured run, so left alone for its own pass. --- .github/workflows/ci.yml | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a39cd273..7e6f99f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,15 @@ jobs: cache: npm - run: npm ci - run: npm run test:coverage + # sonarcloud reads this instead of regenerating the same coverage a + # few minutes later: one leg's report is enough for one Sonar scan, + # and ubuntu-latest is picked because sonarcloud itself runs on it. + - if: matrix.os == 'ubuntu-latest' + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: coverage/lcov.info + retention-days: 1 # `dev` branch protection requires a context literally named `test`, and a # matrixed job can't produce one: it reports `test (ubuntu-latest)` and @@ -75,7 +84,14 @@ jobs: # Informational quality scan: never blocks a merge, so a SonarCloud outage # or configuration change can't turn the whole run red. # Dependabot events carry no secrets so the scan cannot authenticate. + # + # needs test-matrix rather than running in parallel with it: the coverage + # this reads comes from that job's ubuntu-latest leg (uploaded above), on + # this same commit, so there is nothing to scan with until it has actually + # produced one. A failed test-matrix leaves no artifact to download, and + # continue-on-error below is what keeps that from failing the run. sonarcloud: + needs: [test-matrix] if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && (github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]') continue-on-error: true runs-on: ubuntu-latest @@ -88,7 +104,10 @@ jobs: node-version: 22 cache: npm - run: npm ci - - run: npm run test:coverage + - uses: actions/download-artifact@v4 + with: + name: coverage-report + path: coverage - uses: SonarSource/sonarqube-scan-action@v8 env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -108,7 +127,14 @@ jobs: node-version: 22 cache: npm - run: npm ci - - run: npm run build:unpack + # npm run build:unpack is `npm run build && electron-builder --dir`, + # and build is `npm run typecheck && electron-vite build`: run as an + # npm script here, that re-runs the full typecheck the dedicated + # typecheck job already ran on its own. Calling electron-vite and + # electron-builder directly skips the repeat; npm run build:unpack + # itself is untouched for a local build outside CI. + - shell: bash + run: ./node_modules/.bin/electron-vite build && ./node_modules/.bin/electron-builder --dir # macos-latest bills Actions minutes at a 10x multiplier and the launcher # can't launch the game on macOS yet, so this build is manual-only. From fb299c60278f4c5b4b393d960c7024a031f31145 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:26:13 +0200 Subject: [PATCH 8/8] Keep sonarcloud from being skipped when test-matrix goes red sonarcloud gained `needs: [test-matrix]` so it could read that job's coverage artifact instead of regenerating it, but its `if:` named none of success()/always()/failure()/cancelled(). GitHub Actions applies an implicit success() to a plain `if:` combined with `needs` (the `test` job's own comment above already documents this rule), so a single red or cancelled test-matrix leg, a Windows-only flake included, made sonarcloud SKIP outright instead of attempting the download and failing gracefully via the existing continue-on-error. On dev, sonarcloud had no `needs` and always attempted its own independent coverage run regardless of anything else in the workflow; always() restores that always-attempts behaviour on top of the artifact reuse. --- .github/workflows/ci.yml | 7 +++- .../ci-sonarcloud-runs-on-failure.test.ts | 39 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/config/ci-sonarcloud-runs-on-failure.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e6f99f2..a5aa2942 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,9 +90,14 @@ jobs: # this same commit, so there is nothing to scan with until it has actually # produced one. A failed test-matrix leaves no artifact to download, and # continue-on-error below is what keeps that from failing the run. + # always() is required here: a plain `if:` combined with `needs` defaults + # to success() (same rule the `test` job's comment above documents), so + # without it a single red or cancelled test-matrix leg (e.g. a + # Windows-only flake) would SKIP this job outright instead of letting it + # attempt the download and fail gracefully via continue-on-error. sonarcloud: needs: [test-matrix] - if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && (github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]') + if: always() && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) && (github.event_name != 'pull_request' || github.event.pull_request.user.login != 'dependabot[bot]') continue-on-error: true runs-on: ubuntu-latest steps: diff --git a/tests/config/ci-sonarcloud-runs-on-failure.test.ts b/tests/config/ci-sonarcloud-runs-on-failure.test.ts new file mode 100644 index 00000000..25647109 --- /dev/null +++ b/tests/config/ci-sonarcloud-runs-on-failure.test.ts @@ -0,0 +1,39 @@ +import assert from "node:assert/strict" +import { readFileSync } from "node:fs" +import { resolve } from "node:path" +import { describe, it } from "vitest" + +/** + * Guards against sonarcloud silently vanishing from the checks list. + * + * sonarcloud gained `needs: [test-matrix]` so it can download that job's + * coverage artifact instead of regenerating it. But GitHub Actions applies + * an implicit `success()` to any `if:` that names none of + * success()/always()/failure()/cancelled() when the job also has `needs` + * (see the `test` job's own comment two jobs above, which relies on the + * same rule). Without `always()`, a single failed or cancelled test-matrix + * leg (say, a Windows-only flake) makes the whole sonarcloud job SKIP + * outright, even though the ubuntu-latest leg already uploaded a usable + * coverage-report artifact. On dev, sonarcloud had no `needs` and always + * attempted its own independent test:coverage run regardless of anything + * else in the workflow. + */ +describe("ci sonarcloud job", () => { + const workflow = readFileSync(resolve(__dirname, "../../.github/workflows/ci.yml"), "utf8") + const sonarcloudBlock = /^ {2}sonarcloud:$([\s\S]*?)(?=^ {2}\S+:$)/m.exec(workflow)?.[1] + + it("still has a coverage artifact to read from test-matrix", () => { + assert.ok(sonarcloudBlock, "sonarcloud job not found in ci.yml") + assert.match(sonarcloudBlock!, /needs:\s*\[test-matrix\]/) + }) + + it("keeps attempting to run even when a test-matrix leg fails or is cancelled", () => { + // A plain `if:` with `needs` defaults to success(): only always() (or an + // explicit failure()/cancelled()) stops a red test-matrix leg from + // skipping this job outright instead of letting continue-on-error do its + // job of turning a real failure into an informational miss. + const ifLine = /^ {4}if:\s*(.+)$/m.exec(sonarcloudBlock!)?.[1] + assert.ok(ifLine, "sonarcloud job has no if: condition") + assert.match(ifLine!, /always\(\)/) + }) +})