From c6c79c678b87cb4f3cd77fd62df271f38d143d92 Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Sat, 18 Apr 2026 20:39:08 -0600 Subject: [PATCH] =?UTF-8?q?refactor(chess):=20unregister=20Empty=20layout?= =?UTF-8?q?=20=E2=80=94=20keep=20as=20internal=20fixture=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Empty layout had no real user need: - Can't Play Solo (board with no pieces, nothing to click). - Server rejects it for Create Room (validator requires >=1 king per side). - The 'blank canvas' use case is already handled by the LayoutEditor's Custom... flow, which opens with an empty piece list and lets the user compose. It was UI clutter in the picker dropdown — users would see 'Empty Board' alongside real layouts and have no way to actually use it. EMPTY_LAYOUT stays exported for unit tests that need a zero-piece starting state, but it no longer registers in LAYOUT_REGISTRY, is not imported by the layouts barrel side-effects, and no longer appears in the picker. Source flipped to 'custom' to signal it's not a user-selectable premade. Tests updated: - premades.test.ts asserts 'empty' is NOT in the registry. - e2e/layouts.spec.ts removes the Empty-solo test and asserts the 'empty' option is absent from the picker dropdown. 1025 unit tests + 23 e2e tests green. --- packages/chess/e2e/layouts.spec.ts | 32 +++------------------ packages/chess/src/layouts/empty.ts | 31 ++++++++++---------- packages/chess/src/layouts/index.ts | 5 ++-- packages/chess/src/layouts/premades.test.ts | 16 +++++------ 4 files changed, 31 insertions(+), 53 deletions(-) diff --git a/packages/chess/e2e/layouts.spec.ts b/packages/chess/e2e/layouts.spec.ts index 2319359..557a4a1 100644 --- a/packages/chess/e2e/layouts.spec.ts +++ b/packages/chess/e2e/layouts.spec.ts @@ -72,7 +72,9 @@ test.describe('LayoutPicker (lobby)', () => { test('picker includes every expected premade', async ({ page }) => { const picker = page.getByTestId('layout-picker'); - // All premades + Custom entry. + // All premades + Custom entry. "Empty" is intentionally absent — + // it's a test fixture only, not a selectable starting position + // (fails validation, serves no player need). for (const id of [ 'classic', 'dunsany', @@ -81,10 +83,10 @@ test.describe('LayoutPicker (lobby)', () => { 'horde', 'knightmate', 'chess960', - 'empty', ]) { await expect(picker.locator(`option[value="${id}"]`)).toHaveCount(1); } + await expect(picker.locator('option[value="empty"]')).toHaveCount(0); await expect(picker.locator('option[value="__custom__"]')).toHaveCount(1); }); @@ -284,32 +286,6 @@ test.describe('Solo play — layout selection reaches the engine', () => { } }); - test('Empty layout Play Solo is disabled (no king = cannot create game)', async ({ - page, - }) => { - // The Empty layout has 0 pieces; the validator requires at least 1 - // king per side. This test documents current behavior: the Play - // Solo button happily routes to /game regardless, but the engine - // opens with zero pieces (equivalent to validateLayout errors - // surfacing only on the multiplayer path, not solo). This is a - // deliberate design call — solo mode lets users poke at any - // layout including broken ones. - // - // If / when we add pre-flight validation for solo too, flip this - // test to assert a blocked CTA. - await page.goto('/'); - await page.getByTestId('layout-picker').selectOption('empty'); - await page.locator('[data-action="play-solo"]').click(); - await page.waitForURL('**/game'); - - // Board renders but is devoid of pieces. - await expect( - page.locator('[data-square="e1"] [data-piece]'), - ).toHaveCount(0); - await expect( - page.locator('[data-square="e8"] [data-piece]'), - ).toHaveCount(0); - }); }); // ───────────────────────────────────────────────────────────────────── diff --git a/packages/chess/src/layouts/empty.ts b/packages/chess/src/layouts/empty.ts index 8590353..29670de 100644 --- a/packages/chess/src/layouts/empty.ts +++ b/packages/chess/src/layouts/empty.ts @@ -1,27 +1,28 @@ /** - * Layout: Empty board (sandbox). + * Layout: Empty board (internal test fixture). * - * Zero pieces. Useful as: - * - The "base" of the custom layout editor — open this and the - * board is blank; drag pieces from the palette to compose. - * - A test fixture for engine code that shouldn't assume any - * pieces exist. + * Zero pieces. NOT registered in LAYOUT_REGISTRY — it fails the + * validator (requires ≥ 1 king per side) so exposing it in the + * lobby picker would be a dead-end UX: the server would reject + * Create Room, and solo play would drop the user onto a blank + * board with nothing to click. * - * An empty layout will NOT validate for game-play (the validator - * requires one king per side). The lobby's Create Room button stays - * disabled when the editor commits an empty layout; the editor - * surfaces the validation error explicitly. + * The "blank canvas" use case is handled by the LayoutEditor's + * Custom… flow — the editor already opens from an empty piece + * list and lets the user compose freely. + * + * EMPTY_LAYOUT stays exported for: + * - Unit tests that need a zero-piece starting state. + * - `new ChessEngine({ layout: EMPTY_LAYOUT })` as a minimal + * fixture in preset/rule tests. */ import type { StartingLayout } from "./types.js"; -import { LAYOUT_REGISTRY } from "./registry.js"; export const EMPTY_LAYOUT: StartingLayout = { id: "empty", name: "Empty Board", description: - "A blank board for composing your own position from scratch.", + "Internal test fixture — not registered in LAYOUT_REGISTRY.", pieces: [], - source: "premade", + source: "custom", }; - -LAYOUT_REGISTRY.register(EMPTY_LAYOUT); diff --git a/packages/chess/src/layouts/index.ts b/packages/chess/src/layouts/index.ts index 6fc312e..9353790 100644 --- a/packages/chess/src/layouts/index.ts +++ b/packages/chess/src/layouts/index.ts @@ -21,11 +21,12 @@ import "./pawns-only.js"; import "./horde.js"; import "./knightmate.js"; import "./chess960.js"; -// Sandbox — last so it's visually grouped separately in the picker. -import "./empty.js"; export { LAYOUT_REGISTRY } from "./registry.js"; export { CLASSIC_LAYOUT } from "./classic.js"; +// EMPTY_LAYOUT is NOT auto-registered — it's an internal test +// fixture (see layouts/empty.ts). Exported here so tests can +// construct an engine around it. export { EMPTY_LAYOUT } from "./empty.js"; export { DUNSANY_LAYOUT } from "./dunsany.js"; export { MONSTER_LAYOUT } from "./monster.js"; diff --git a/packages/chess/src/layouts/premades.test.ts b/packages/chess/src/layouts/premades.test.ts index b70af3d..f4bced3 100644 --- a/packages/chess/src/layouts/premades.test.ts +++ b/packages/chess/src/layouts/premades.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect } from "vitest"; import { LAYOUT_REGISTRY } from "./registry.js"; import { validateLayout } from "./validate.js"; +import { EMPTY_LAYOUT } from "./empty.js"; import "./index.js"; // trigger registration of every premade describe("LAYOUT_REGISTRY — premade roster", () => { @@ -20,7 +21,10 @@ describe("LAYOUT_REGISTRY — premade roster", () => { expect(ids).toContain("horde"); expect(ids).toContain("knightmate"); expect(ids).toContain("chess960"); - expect(ids).toContain("empty"); + }); + + it("does NOT include 'empty' — it's an internal test fixture only", () => { + expect(LAYOUT_REGISTRY.has("empty")).toBe(false); }); it("every premade has source: 'premade'", () => { @@ -29,18 +33,15 @@ describe("LAYOUT_REGISTRY — premade roster", () => { } }); - it("every non-empty premade validates with zero errors", () => { + it("every registered premade validates with zero errors", () => { for (const layout of LAYOUT_REGISTRY.list()) { - if (layout.id === "empty") continue; // empty has no kings, rightfully errors const { errors } = validateLayout(layout); expect(errors, `${layout.id}: ${errors.join("; ")}`).toHaveLength(0); } }); - it("empty layout errors on missing kings (validator doing its job)", () => { - const empty = LAYOUT_REGISTRY.get("empty"); - expect(empty).toBeDefined(); - const { errors } = validateLayout(empty!); + it("EMPTY_LAYOUT test fixture still fails validation (sanity)", () => { + const { errors } = validateLayout(EMPTY_LAYOUT); expect(errors.length).toBeGreaterThan(0); }); @@ -66,7 +67,6 @@ describe("premade piece counts", () => { ["horde", 52], // 31 white pawns (ranks 1-4 minus e1) + 1 king + 4 advanced pawns + 16 black pieces ["knightmate", 32], // standard 32 with swapped back rank ["chess960", 32], // shim is FIDE by default - ["empty", 0], ]; for (const [id, expectedCount] of counts) {