docs: add IO interface refactor design spec

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-05 09:33:50 +02:00
parent fa5cdf4e9c
commit e2fe56ea87
1 changed files with 91 additions and 0 deletions
@@ -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)