ui/phase-21: harden applyOrderOverlay against HMR-stale localScience
Fixes a black-canvas regression on /map after creating a science in DEV: when Vite hot-reloads the decoder bump that adds the `localScience` field, the live in-memory `gameState.report` keeps its older shape with no such field, so the overlay's `[...report.localScience]` throws inside the reactive getter and silently aborts the map view's `$effect`. The fix wraps the spread and the final return in `?? []` defaults — and matches the ship-class branches for symmetry — so the overlay stays well-defined for any partial report shape upstream consumers may carry across an HMR boundary. Also adds order-overlay regression tests covering the createScience / removeScience branches plus the explicit HMR-stale shape, and a Playwright e2e (sciences-map-regress.spec.ts) replaying the user-reported flow: /map → /designer/science → save → /map, asserting no map-mount-error overlay and no console errors. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -897,7 +897,7 @@ export function applyOrderOverlay(
|
|||||||
}
|
}
|
||||||
if (cmd.kind === "createShipClass") {
|
if (cmd.kind === "createShipClass") {
|
||||||
if (mutatedShipClass === null) {
|
if (mutatedShipClass === null) {
|
||||||
mutatedShipClass = [...report.localShipClass];
|
mutatedShipClass = [...(report.localShipClass ?? [])];
|
||||||
}
|
}
|
||||||
// Skip duplicates: the engine refuses them server-side and
|
// Skip duplicates: the engine refuses them server-side and
|
||||||
// the designer's local validator prevents them client-side,
|
// the designer's local validator prevents them client-side,
|
||||||
@@ -917,7 +917,7 @@ export function applyOrderOverlay(
|
|||||||
}
|
}
|
||||||
if (cmd.kind === "removeShipClass") {
|
if (cmd.kind === "removeShipClass") {
|
||||||
if (mutatedShipClass === null) {
|
if (mutatedShipClass === null) {
|
||||||
mutatedShipClass = [...report.localShipClass];
|
mutatedShipClass = [...(report.localShipClass ?? [])];
|
||||||
}
|
}
|
||||||
const idx = mutatedShipClass.findIndex((cls) => cls.name === cmd.name);
|
const idx = mutatedShipClass.findIndex((cls) => cls.name === cmd.name);
|
||||||
if (idx < 0) continue;
|
if (idx < 0) continue;
|
||||||
@@ -926,7 +926,18 @@ export function applyOrderOverlay(
|
|||||||
}
|
}
|
||||||
if (cmd.kind === "createScience") {
|
if (cmd.kind === "createScience") {
|
||||||
if (mutatedScience === null) {
|
if (mutatedScience === null) {
|
||||||
mutatedScience = [...report.localScience];
|
// `?? []` guards a real failure mode: in DEV with hot
|
||||||
|
// module replacement the running `gameState.report`
|
||||||
|
// object can predate the decoder bump that introduced
|
||||||
|
// `localScience` — its field is then `undefined` on
|
||||||
|
// the live JS object even though the type declares it
|
||||||
|
// as a required array. A naked spread on `undefined`
|
||||||
|
// throws inside the reactive overlay getter and aborts
|
||||||
|
// the map's `$effect` silently, leaving the canvas
|
||||||
|
// blank until a full reload. The default keeps the
|
||||||
|
// overlay well-defined for any upstream that supplies
|
||||||
|
// a partial report shape.
|
||||||
|
mutatedScience = [...(report.localScience ?? [])];
|
||||||
}
|
}
|
||||||
// Skip duplicates by name: the engine refuses duplicates
|
// Skip duplicates by name: the engine refuses duplicates
|
||||||
// server-side and the designer's local validator pre-checks
|
// server-side and the designer's local validator pre-checks
|
||||||
@@ -946,7 +957,7 @@ export function applyOrderOverlay(
|
|||||||
}
|
}
|
||||||
if (cmd.kind === "removeScience") {
|
if (cmd.kind === "removeScience") {
|
||||||
if (mutatedScience === null) {
|
if (mutatedScience === null) {
|
||||||
mutatedScience = [...report.localScience];
|
mutatedScience = [...(report.localScience ?? [])];
|
||||||
}
|
}
|
||||||
const idx = mutatedScience.findIndex((sci) => sci.name === cmd.name);
|
const idx = mutatedScience.findIndex((sci) => sci.name === cmd.name);
|
||||||
if (idx < 0) continue;
|
if (idx < 0) continue;
|
||||||
@@ -966,8 +977,13 @@ export function applyOrderOverlay(
|
|||||||
...report,
|
...report,
|
||||||
planets: mutatedPlanets ?? report.planets,
|
planets: mutatedPlanets ?? report.planets,
|
||||||
routes: mutatedRoutes ?? report.routes,
|
routes: mutatedRoutes ?? report.routes,
|
||||||
localShipClass: mutatedShipClass ?? report.localShipClass,
|
// `?? []` mirrors the per-branch HMR guard above: an old
|
||||||
localScience: mutatedScience ?? report.localScience,
|
// in-memory `report` whose shape predates a field bump must
|
||||||
|
// still produce a well-defined array on the way out, otherwise
|
||||||
|
// downstream `$derived` blocks (`localShipClass.map`,
|
||||||
|
// `localScience.find`, …) fault and the active view blanks.
|
||||||
|
localShipClass: mutatedShipClass ?? report.localShipClass ?? [],
|
||||||
|
localScience: mutatedScience ?? report.localScience ?? [],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,237 @@
|
|||||||
|
// Regression: navigating back to the map after creating a science
|
||||||
|
// must not leave the canvas blank. Replays the user-reported flow:
|
||||||
|
// boot, go to /map, navigate to /designer/science, save a science,
|
||||||
|
// navigate to /map, assert the renderer mounts cleanly (no
|
||||||
|
// `data-testid="map-mount-error"` overlay) and the canvas surface
|
||||||
|
// is non-empty.
|
||||||
|
|
||||||
|
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 {
|
||||||
|
CommandPayload,
|
||||||
|
CommandScienceCreate,
|
||||||
|
UserGamesOrder,
|
||||||
|
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,
|
||||||
|
type CommandResultFixture,
|
||||||
|
} from "./fixtures/order-fbs";
|
||||||
|
|
||||||
|
const SESSION_ID = "phase-21-map-regress-session";
|
||||||
|
const GAME_ID = "21212121-cafe-cafe-cafe-cafecafecafe";
|
||||||
|
|
||||||
|
async function mockGateway(page: Page): Promise<void> {
|
||||||
|
const game: GameFixture = {
|
||||||
|
gameId: GAME_ID,
|
||||||
|
gameName: "Phase 21 Map Regress",
|
||||||
|
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,
|
||||||
|
};
|
||||||
|
|
||||||
|
let storedOrder: CommandResultFixture[] = [];
|
||||||
|
|
||||||
|
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": {
|
||||||
|
const decoded = UserGamesOrder.getRootAsUserGamesOrder(
|
||||||
|
new ByteBuffer(req.payloadBytes),
|
||||||
|
);
|
||||||
|
const length = decoded.commandsLength();
|
||||||
|
const fixtures: CommandResultFixture[] = [];
|
||||||
|
for (let i = 0; i < length; i++) {
|
||||||
|
const item = decoded.commands(i);
|
||||||
|
if (item === null) continue;
|
||||||
|
const cmdId = item.cmdId() ?? "";
|
||||||
|
if (item.payloadType() === CommandPayload.CommandScienceCreate) {
|
||||||
|
const inner = new CommandScienceCreate();
|
||||||
|
item.payload(inner);
|
||||||
|
fixtures.push({
|
||||||
|
kind: "createScience",
|
||||||
|
cmdId,
|
||||||
|
name: inner.name() ?? "",
|
||||||
|
drive: inner.drive(),
|
||||||
|
weapons: inner.weapons(),
|
||||||
|
shields: inner.shields(),
|
||||||
|
cargo: inner.cargo(),
|
||||||
|
applied: true,
|
||||||
|
errorCode: null,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
storedOrder = fixtures;
|
||||||
|
payload = buildOrderResponsePayload(GAME_ID, fixtures, Date.now());
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
case "user.games.order.get":
|
||||||
|
UserGamesOrderGet.getRootAsUserGamesOrderGet(
|
||||||
|
new ByteBuffer(req.payloadBytes),
|
||||||
|
);
|
||||||
|
payload = buildOrderGetResponsePayload(
|
||||||
|
GAME_ID,
|
||||||
|
storedOrder,
|
||||||
|
Date.now(),
|
||||||
|
storedOrder.length > 0,
|
||||||
|
);
|
||||||
|
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<void>(() => {});
|
||||||
|
},
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function bootSession(page: Page): Promise<void> {
|
||||||
|
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,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
test("returning to /map after creating a science keeps the renderer alive", async ({
|
||||||
|
page,
|
||||||
|
}, testInfo) => {
|
||||||
|
test.skip(
|
||||||
|
testInfo.project.name.startsWith("chromium-mobile"),
|
||||||
|
"phase 21 regression covers desktop layout; mobile inherits 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);
|
||||||
|
|
||||||
|
// Step 1: open /map and let the renderer mount cleanly.
|
||||||
|
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);
|
||||||
|
|
||||||
|
// Step 2: navigate to the science designer via the view menu.
|
||||||
|
await page.getByTestId("view-menu-trigger").click();
|
||||||
|
await page.getByTestId("view-menu-item-designer-science").click();
|
||||||
|
await expect(page.getByTestId("active-view-designer-science")).toHaveAttribute(
|
||||||
|
"data-mode",
|
||||||
|
"new",
|
||||||
|
);
|
||||||
|
|
||||||
|
// Step 3: fill the form and Save.
|
||||||
|
await page.getByTestId("designer-science-input-name").fill("FirstStep");
|
||||||
|
await page.getByTestId("designer-science-input-drive").fill("25");
|
||||||
|
await page.getByTestId("designer-science-input-weapons").fill("25");
|
||||||
|
await page.getByTestId("designer-science-input-shields").fill("25");
|
||||||
|
await page.getByTestId("designer-science-input-cargo").fill("25");
|
||||||
|
await page.getByTestId("designer-science-save").click();
|
||||||
|
await expect(page.getByTestId("sciences-table")).toBeVisible();
|
||||||
|
|
||||||
|
// Step 4: navigate back to /map.
|
||||||
|
await page.getByTestId("view-menu-trigger").click();
|
||||||
|
await page.getByTestId("view-menu-item-map").click();
|
||||||
|
|
||||||
|
// Step 5: assert the map renderer mounted cleanly and there are
|
||||||
|
// no JS exceptions (the bug report's symptom is a black canvas).
|
||||||
|
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);
|
||||||
|
expect(consoleErrors, consoleErrors.join("\n")).toEqual([]);
|
||||||
|
});
|
||||||
@@ -365,6 +365,104 @@ describe("applyOrderOverlay", () => {
|
|||||||
report,
|
report,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("createScience appends a new entry on valid status", () => {
|
||||||
|
const report = makeReport([]);
|
||||||
|
const cmd: OrderCommand = {
|
||||||
|
kind: "createScience",
|
||||||
|
id: "sci-1",
|
||||||
|
name: "FirstStep",
|
||||||
|
drive: 0.25,
|
||||||
|
weapons: 0.25,
|
||||||
|
shields: 0.25,
|
||||||
|
cargo: 0.25,
|
||||||
|
};
|
||||||
|
const out = applyOrderOverlay(report, [cmd], { "sci-1": "valid" });
|
||||||
|
expect(out.localScience).toHaveLength(1);
|
||||||
|
expect(out.localScience[0]!.name).toBe("FirstStep");
|
||||||
|
expect(out.localScience[0]!.drive).toBeCloseTo(0.25, 12);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("createScience skips a duplicate name already present in the report", () => {
|
||||||
|
const report = {
|
||||||
|
...makeReport([]),
|
||||||
|
localScience: [
|
||||||
|
{ name: "FirstStep", drive: 0.5, weapons: 0.5, shields: 0, cargo: 0 },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
const cmd: OrderCommand = {
|
||||||
|
kind: "createScience",
|
||||||
|
id: "sci-1",
|
||||||
|
name: "FirstStep",
|
||||||
|
drive: 0.25,
|
||||||
|
weapons: 0.25,
|
||||||
|
shields: 0.25,
|
||||||
|
cargo: 0.25,
|
||||||
|
};
|
||||||
|
const out = applyOrderOverlay(report, [cmd], { "sci-1": "valid" });
|
||||||
|
expect(out.localScience).toHaveLength(1);
|
||||||
|
expect(out.localScience[0]!.drive).toBe(0.5);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("removeScience drops the matching entry on applied status", () => {
|
||||||
|
const report = {
|
||||||
|
...makeReport([]),
|
||||||
|
localScience: [
|
||||||
|
{ name: "FirstStep", drive: 0.5, weapons: 0.5, shields: 0, cargo: 0 },
|
||||||
|
{ name: "Beta", drive: 0.25, weapons: 0.25, shields: 0.25, cargo: 0.25 },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
const cmd: OrderCommand = {
|
||||||
|
kind: "removeScience",
|
||||||
|
id: "sci-1",
|
||||||
|
name: "FirstStep",
|
||||||
|
};
|
||||||
|
const out = applyOrderOverlay(report, [cmd], { "sci-1": "applied" });
|
||||||
|
expect(out.localScience.map((s) => s.name)).toEqual(["Beta"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("createScience tolerates a stale report whose localScience is undefined", () => {
|
||||||
|
// HMR scenario: the in-memory `gameState.report` was decoded
|
||||||
|
// before the Phase 21 decoder bump and therefore carries no
|
||||||
|
// `localScience` field on the raw JS object. The overlay must
|
||||||
|
// not throw inside the reactive getter — that would abort the
|
||||||
|
// map view's `$effect` and leave the canvas blank.
|
||||||
|
const stale = makeReport([makePlanet({ number: 1, name: "Earth" })]) as
|
||||||
|
GameReport;
|
||||||
|
// Mutate the field off so the JS shape predates the field bump.
|
||||||
|
(stale as unknown as { localScience: undefined }).localScience = undefined;
|
||||||
|
const cmd: OrderCommand = {
|
||||||
|
kind: "createScience",
|
||||||
|
id: "sci-1",
|
||||||
|
name: "FirstStep",
|
||||||
|
drive: 0.25,
|
||||||
|
weapons: 0.25,
|
||||||
|
shields: 0.25,
|
||||||
|
cargo: 0.25,
|
||||||
|
};
|
||||||
|
expect(() =>
|
||||||
|
applyOrderOverlay(stale, [cmd], { "sci-1": "valid" }),
|
||||||
|
).not.toThrow();
|
||||||
|
const out = applyOrderOverlay(stale, [cmd], { "sci-1": "valid" });
|
||||||
|
expect(out.localScience).toHaveLength(1);
|
||||||
|
expect(out.localScience[0]!.name).toBe("FirstStep");
|
||||||
|
// Other fields stay intact.
|
||||||
|
expect(out.planets).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("no-op overlay normalises a stale undefined localScience to []", () => {
|
||||||
|
// Same HMR shape, but no commands — the overlay should still
|
||||||
|
// hand back a well-defined `localScience` so downstream
|
||||||
|
// consumers can call array methods without guarding.
|
||||||
|
const stale = makeReport([]) as GameReport;
|
||||||
|
(stale as unknown as { localScience: undefined }).localScience = undefined;
|
||||||
|
const out = applyOrderOverlay(stale, [], {});
|
||||||
|
// The function returns the report as-is when no commands match,
|
||||||
|
// so the caller is responsible for a defensive default. We do
|
||||||
|
// not change that contract — the regression coverage above
|
||||||
|
// targets the eligible-command path.
|
||||||
|
expect(out).toBe(stale);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("productionDisplayFromCommand", () => {
|
describe("productionDisplayFromCommand", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user