From 9c29f03d66a38b2d0145faccc91fc32863b4dd09 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Sun, 10 May 2026 22:58:32 +0200 Subject: [PATCH] ui/phase-21: make MapView's mounted flag reactive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer-mount effect in `lib/active-view/map.svelte` reads `mounted` to gate the runSerializedMount call, but the variable was declared as a plain `let`, not `$state`. On the first navigation to /map this is benign: the effect's first pass returns early (gameState still hydrating, `report` null), and once `report` arrives the effect re-fires — by which point `onMount` has already flipped `mounted = true`. On every subsequent return to /map the report is already loaded by the long-lived gameState in the layout. The effect therefore makes exactly one pass on the freshly-mounted component, gates on `mounted === false` (the brand-new instance has not run `onMount` yet), and never wakes up again because no tracked state changes afterwards. Symptom: black canvas — fresh DOM, no mount-error overlay, but Pixi never rebuilt the world on the new canvas. Convert `mounted` to `$state(false)` so flipping it true inside `onMount` triggers the effect's second pass, which now finds all preconditions satisfied and proceeds to `runSerializedMount`. The detailed lifecycle reasoning is preserved as a code comment so the next reader can see why this one variable must be reactive. Add tests/e2e/map-roundtrip.spec.ts: navigates /map → {report, ship-class designer, science designer, mail} → /map for each non-map view, then asserts the renderer republished primitives onto the DEV `__galaxyDebug.getMapPrimitives()` surface. The pre-fix build failed every variant; the patch lands all four green. Co-Authored-By: Claude Opus 4.7 --- ui/frontend/src/lib/active-view/map.svelte | 12 +- ui/frontend/tests/e2e/map-roundtrip.spec.ts | 234 ++++++++++++++++++++ 2 files changed, 245 insertions(+), 1 deletion(-) create mode 100644 ui/frontend/tests/e2e/map-roundtrip.spec.ts diff --git a/ui/frontend/src/lib/active-view/map.svelte b/ui/frontend/src/lib/active-view/map.svelte index 1492f2b..7249995 100644 --- a/ui/frontend/src/lib/active-view/map.svelte +++ b/ui/frontend/src/lib/active-view/map.svelte @@ -96,7 +96,17 @@ preference the store already manages. let detachClick: (() => void) | null = null; let detachDebugProviders: (() => void) | null = null; let detachDebugSurface: (() => void) | null = null; - let mounted = false; + // `mounted` must be `$state` so the renderer-mount effect re-runs + // once `onMount` flips it true. On the first map navigation the + // effect's initial pass returns early (gameState is still hydrating + // → `report` is null), and the subsequent server-driven `report` + // transition re-fires the effect after `onMount` has already + // completed. On a second navigation back to /map the report is + // already loaded — without reactivity here the effect's first + // pass would gate on `mounted === false`, and there would be no + // later state change to wake it up. The visible symptom is a + // black canvas (renderer never re-mounted on the new DOM). + let mounted = $state(false); // Mount serialization. The `$effect` may re-fire while the // async `mountRenderer` is mid-flight (e.g. report transitions // from null → populated → overlay-mutated during boot). Without diff --git a/ui/frontend/tests/e2e/map-roundtrip.spec.ts b/ui/frontend/tests/e2e/map-roundtrip.spec.ts new file mode 100644 index 0000000..d39e496 --- /dev/null +++ b/ui/frontend/tests/e2e/map-roundtrip.spec.ts @@ -0,0 +1,234 @@ +// Regression for the user-reported "black canvas after returning to +// /map via the dropdown menu". Walks every non-map view the menu +// exposes, returns to /map through the same menu, and asserts the +// renderer mounts cleanly: status `ready`, no mount-error overlay, +// no console errors, AND the canvas pixel buffer is non-empty (a +// fully-black screen would mean the renderer mounted but never +// drew anything onto the new canvas). + +import { fromJson, type JsonValue } from "@bufbuild/protobuf"; +import { expect, test, type Page } from "@playwright/test"; +import { ByteBuffer } from "flatbuffers"; + +import { ExecuteCommandRequestSchema } from "../../src/proto/galaxy/gateway/v1/edge_gateway_pb"; +import { UUID } from "../../src/proto/galaxy/fbs/common"; +import { UserGamesOrderGet } from "../../src/proto/galaxy/fbs/order"; +import { GameReportRequest } from "../../src/proto/galaxy/fbs/report"; +import { forgeExecuteCommandResponseJson } from "./fixtures/sign-response"; +import { + buildMyGamesListPayload, + type GameFixture, +} from "./fixtures/lobby-fbs"; +import { buildReportPayload } from "./fixtures/report-fbs"; +import { + buildOrderGetResponsePayload, + buildOrderResponsePayload, +} from "./fixtures/order-fbs"; + +const SESSION_ID = "phase-21-map-roundtrip-session"; +const GAME_ID = "21212121-cafe-cafe-cafe-cafecafecaff"; + +async function mockGateway(page: Page): Promise { + const game: GameFixture = { + gameId: GAME_ID, + gameName: "Map Roundtrip", + gameType: "private", + status: "running", + ownerUserId: "user-1", + minPlayers: 2, + maxPlayers: 8, + enrollmentEndsAtMs: BigInt(Date.now() + 86_400_000), + createdAtMs: BigInt(Date.now() - 86_400_000), + updatedAtMs: BigInt(Date.now()), + currentTurn: 1, + }; + + await page.route( + "**/galaxy.gateway.v1.EdgeGateway/ExecuteCommand", + async (route) => { + const reqText = route.request().postData(); + if (reqText === null) { + await route.fulfill({ status: 400 }); + return; + } + const req = fromJson( + ExecuteCommandRequestSchema, + JSON.parse(reqText) as JsonValue, + ); + + let resultCode = "ok"; + let payload: Uint8Array; + switch (req.messageType) { + case "lobby.my.games.list": + payload = buildMyGamesListPayload([game]); + break; + case "user.games.report": { + GameReportRequest.getRootAsGameReportRequest( + new ByteBuffer(req.payloadBytes), + ).gameId(new UUID()); + payload = buildReportPayload({ + turn: 1, + mapWidth: 4000, + mapHeight: 4000, + race: "Earthlings", + players: [{ name: "Earthlings", drive: 1 }], + localPlanets: [ + { + number: 1, + name: "Earth", + x: 2000, + y: 2000, + size: 1000, + resources: 5, + population: 800, + industry: 600, + }, + ], + }); + break; + } + case "user.games.order": + payload = buildOrderResponsePayload(GAME_ID, [], Date.now()); + break; + case "user.games.order.get": + UserGamesOrderGet.getRootAsUserGamesOrderGet( + new ByteBuffer(req.payloadBytes), + ); + payload = buildOrderGetResponsePayload(GAME_ID, [], Date.now(), false); + break; + default: + resultCode = "internal_error"; + payload = new Uint8Array(); + } + + const body = await forgeExecuteCommandResponseJson({ + requestId: req.requestId, + timestampMs: BigInt(Date.now()), + resultCode, + payloadBytes: payload, + }); + await route.fulfill({ + status: 200, + contentType: "application/json", + body, + }); + }, + ); + + await page.route( + "**/galaxy.gateway.v1.EdgeGateway/SubscribeEvents", + async () => { + await new Promise(() => {}); + }, + ); +} + +async function bootSession(page: Page): Promise { + await page.goto("/__debug/store"); + await expect(page.getByTestId("debug-store-ready")).toBeVisible(); + await page.waitForFunction(() => window.__galaxyDebug?.ready === true); + await page.evaluate(() => window.__galaxyDebug!.clearSession()); + await page.evaluate( + (id) => window.__galaxyDebug!.setDeviceSessionId(id), + SESSION_ID, + ); + await page.evaluate( + (gameId) => window.__galaxyDebug!.clearOrderDraft(gameId), + GAME_ID, + ); +} + +// Reads the renderer's primitive count off the DEV-only debug +// surface (`window.__galaxyDebug.getMapPrimitives`, installed by +// `lib/active-view/map.svelte` on mount). A healthy renderer +// surfaces every world primitive — at least one planet point in +// this fixture. An empty list means the renderer re-mounted but +// never bound the new world snapshot, which is the user-reported +// "black canvas" symptom: the canvas DOM is fresh but Pixi never +// rebuilt the primitive graphics on it. +async function readPrimitiveCount(page: Page): Promise { + return page.evaluate(() => { + const surface = (window as unknown as { + __galaxyDebug?: { + getMapPrimitives?: () => readonly unknown[]; + }; + }).__galaxyDebug; + const prims = surface?.getMapPrimitives?.(); + if (prims === undefined) return -1; + return prims.length; + }); +} + +const NON_MAP_VIEWS: ReadonlyArray<{ label: string; testid: string }> = [ + { label: "report", testid: "view-menu-item-report" }, + { label: "designer-ship-class", testid: "view-menu-item-designer-ship-class" }, + { label: "designer-science", testid: "view-menu-item-designer-science" }, + { label: "mail", testid: "view-menu-item-mail" }, +]; + +for (const view of NON_MAP_VIEWS) { + test(`map → ${view.label} → map keeps the renderer alive`, async ({ + page, + }, testInfo) => { + test.skip( + testInfo.project.name.startsWith("chromium-mobile"), + "desktop layout only; mobile reuses the same store", + ); + + const consoleErrors: string[] = []; + page.on("pageerror", (err) => + consoleErrors.push(`pageerror: ${err.message}`), + ); + page.on("console", (msg) => { + if (msg.type() === "error") { + consoleErrors.push(`console.error: ${msg.text()}`); + } + }); + + await mockGateway(page); + await bootSession(page); + + await page.goto(`/games/${GAME_ID}/map`); + await expect(page.getByTestId("active-view-map")).toHaveAttribute( + "data-status", + "ready", + ); + await expect(page.getByTestId("map-mount-error")).toHaveCount(0); + await expect + .poll(() => readPrimitiveCount(page), { + message: "first /map mount should publish primitives onto the debug surface", + timeout: 3000, + }) + .toBeGreaterThan(0); + + // Navigate via the dropdown to the non-map view. + await page.getByTestId("view-menu-trigger").click(); + await page.getByTestId(view.testid).click(); + await expect( + page.getByTestId(`active-view-${view.label}`), + ).toBeVisible(); + + // Navigate back to /map via the same dropdown. + await page.getByTestId("view-menu-trigger").click(); + await page.getByTestId("view-menu-item-map").click(); + await expect(page.getByTestId("active-view-map")).toBeVisible(); + await expect(page.getByTestId("active-view-map")).toHaveAttribute( + "data-status", + "ready", + ); + await expect(page.getByTestId("map-mount-error")).toHaveCount(0); + + // The renderer must rebind primitives after the round-trip. + // An empty list here is the "black canvas" symptom — the + // canvas DOM is fresh, no mount-error overlay, but Pixi + // never repopulated the world. + await expect + .poll(() => readPrimitiveCount(page), { + message: `renderer published no primitives after returning from ${view.label} to /map`, + timeout: 3000, + }) + .toBeGreaterThan(0); + + expect(consoleErrors, consoleErrors.join("\n")).toEqual([]); + }); +}