From e2fe56ea8793cfa264361550829868133d67a84f Mon Sep 17 00:00:00 2001 From: Janis Date: Sun, 5 Apr 2026 09:33:50 +0200 Subject: [PATCH] docs: add IO interface refactor design spec Co-Authored-By: Claude Sonnet 4.6 --- ...2026-04-05-io-interface-refactor-design.md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-05-io-interface-refactor-design.md diff --git a/docs/superpowers/specs/2026-04-05-io-interface-refactor-design.md b/docs/superpowers/specs/2026-04-05-io-interface-refactor-design.md new file mode 100644 index 0000000..fa9ee7b --- /dev/null +++ b/docs/superpowers/specs/2026-04-05-io-interface-refactor-design.md @@ -0,0 +1,91 @@ +# IO Interface Refactor Design +Date: 2026-04-05 + +## Goal + +Make `GameEngine` accept any IO format (FEN, PGN, future formats) through a uniform interface, so callers never depend on format-specific classes directly. + +## Problem + +`GameContextImport` returns `Option[GameContext]`, losing error messages. `PgnParser` and `PgnExporter` do not implement either interface — the engine imports `PgnParser` directly in `loadPgn`. This breaks the abstraction the interfaces are meant to provide. + +## Interface Changes + +### `GameContextImport` (modules/io) + +Change return type from `Option` to `Either`: + +```scala +trait GameContextImport: + def importGameContext(input: String): Either[String, GameContext] +``` + +### `GameContextExport` (modules/io) + +Unchanged: + +```scala +trait GameContextExport: + def exportGameContext(context: GameContext): String +``` + +## Implementations + +| Class | Trait | Behaviour | +|---|---|---| +| `FenParser` | `GameContextImport` | `parseFen` → `Right(ctx)` or `Left("Invalid FEN: …")` | +| `FenExporter` | `GameContextExport` | unchanged — delegates to `gameContextToFen` | +| `PgnParser` | `GameContextImport` | calls `validatePgn`; maps `Right(game)` to final `GameContext` with `moves` populated via `DefaultRules.applyMove`; passes through `Left(err)` | +| `PgnExporter` | `GameContextExport` | generates PGN from `ctx.moves` with default headers | + +`PgnParser` retains `parsePgn`, `validatePgn`, and `parseAlgebraicMove` as its own public API. `importGameContext` is the additional uniform entry point. + +## GameEngine Changes (modules/core) + +Remove `loadPgn(pgn: String)`. Add: + +```scala +def loadGame(importer: GameContextImport, input: String): Either[String, Unit] +``` + +Logic inside `loadGame`: +1. Call `importer.importGameContext(input)` +2. On `Left(err)` → return `Left(err)` +3. On `Right(ctx)`: + - `ctx.moves.nonEmpty` → replay each move through `handleParsedMove` + `completePromotion`, then notify `PgnLoadedEvent` + - `ctx.moves.isEmpty` → call `loadPosition(ctx)` + +Add symmetric export: + +```scala +def exportGame(exporter: GameContextExport): String = + exporter.exportGameContext(context) +``` + +`loadPosition` is kept unchanged for direct `GameContext` injection (tests, GUI, reset). + +Callers: +```scala +engine.loadGame(PgnParser, pgn) // game with history → replay +engine.loadGame(FenParser, fen) // position snapshot → set position +engine.exportGame(FenExporter) +engine.exportGame(PgnExporter) +``` + +## Testing + +### Updates to existing tests +- `FenParserTest` — update assertions from `Option` to `Either` +- `FenExporterTest` — no changes expected +- `GameEngineLoadPgnTest` — replace `engine.loadPgn(pgn)` with `engine.loadGame(PgnParser, pgn)` + +### New test cases (in existing test files) +- `PgnParserTest` — `importGameContext` returns `Right(ctx)` with correct final position and `ctx.moves` populated; returns `Left(err)` on invalid PGN +- `PgnExporterTest` — `exportGameContext(ctx)` generates valid PGN from a context with moves +- `GameEngineLoadPgnTest` / `GameEngineTest` — `loadGame(FenParser, fen)` sets position without replay; `loadGame(PgnParser, pgn)` replays moves and enables undo/redo; `exportGame` delegates correctly to both exporters + +## Out of Scope + +- Adding new formats (no new parsers/exporters in this change) +- PGN header customisation on export (default headers only for now) +- Changes to `GameHistory` (already deprecated, not touched)