From f6f05ff2a1187e22d10832c67d0eb2c1723508f7 Mon Sep 17 00:00:00 2001 From: Janis Date: Sun, 5 Apr 2026 12:33:13 +0200 Subject: [PATCH] refactor(io): change GameContextImport return type from Option to Either Update GameContextImport trait to return Either[String, GameContext] instead of Option[GameContext] to provide better error context during parsing. Update FenParser implementation to convert Option to Either using toRight(). Co-Authored-By: Claude Sonnet 4.6 --- .idea/copilotDiffState.xml | 18 + .../plans/2026-04-05-io-interface-refactor.md | 780 ++++++++++++++++++ .../de/nowchess/io/GameContextImport.scala | 6 +- .../scala/de/nowchess/io/fen/FenParser.scala | 3 +- 4 files changed, 802 insertions(+), 5 deletions(-) create mode 100644 .idea/copilotDiffState.xml create mode 100644 docs/superpowers/plans/2026-04-05-io-interface-refactor.md diff --git a/.idea/copilotDiffState.xml b/.idea/copilotDiffState.xml new file mode 100644 index 0000000..afa82a2 --- /dev/null +++ b/.idea/copilotDiffState.xml @@ -0,0 +1,18 @@ + + + + + + \ No newline at end of file diff --git a/docs/superpowers/plans/2026-04-05-io-interface-refactor.md b/docs/superpowers/plans/2026-04-05-io-interface-refactor.md new file mode 100644 index 0000000..93d2d6e --- /dev/null +++ b/docs/superpowers/plans/2026-04-05-io-interface-refactor.md @@ -0,0 +1,780 @@ +# IO Interface Refactor Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Unify IO import/export behind uniform interfaces so GameEngine accepts any format without format-specific imports. + +**Architecture:** Change `GameContextImport` from `Option` to `Either` return; implement both FEN and PGN to this interface; refactor GameEngine to accept importer/exporter traits instead of hardcoded PgnParser. + +**Tech Stack:** Scala 3, Either, GameContext, Quarkus, ScalaTest + +--- + +## File Structure + +| File | Action | Responsibility | +|---|---|---| +| `modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala` | Modify | Change signature from `Option` to `Either[String, GameContext]` | +| `modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala` | Modify | Implement `GameContextImport`, wrap `parseFen` with error messages | +| `modules/io/src/main/scala/de/nowchess/io/pgn/PgnParser.scala` | Modify | Implement `GameContextImport`, call `validatePgn`, build final `GameContext` with moves | +| `modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala` | Modify | Implement `GameContextExport`, build PGN from `context.moves` with default headers | +| `modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala` | Modify | Add `loadGame(importer, input)`, `exportGame(exporter)`; remove `loadPgn` | +| `modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala` | Modify | Update assertions from `Option` to `Either` | +| `modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala` | Modify | Add `importGameContext` test cases | +| `modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala` | Modify | Add `exportGameContext` test cases | +| `modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineLoadPgnTest.scala` | Modify | Replace `loadPgn` calls with `loadGame(PgnParser, …)`; add FEN load tests | + +--- + +## Task 1: Update GameContextImport interface + +**Files:** +- Modify: `modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala` + +- [ ] **Step 1: Read current GameContextImport** + +Current: +```scala +trait GameContextImport { + def importGameContext(input: String): Option[GameContext] +} +``` + +- [ ] **Step 2: Change signature to Either** + +```scala +package de.nowchess.io + +import de.nowchess.api.game.GameContext + +trait GameContextImport: + + def importGameContext(input: String): Either[String, GameContext] +``` + +- [ ] **Step 3: Verify GameContextExport unchanged** + +Confirm it still exists as: +```scala +trait GameContextExport: + def exportGameContext(context: GameContext): String +``` + +--- + +## Task 2: Update FenParser to implement Either + +**Files:** +- Modify: `modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala` + +- [ ] **Step 1: Update import statements** + +Add imports at top: +```scala +package de.nowchess.io.fen + +import de.nowchess.api.board.* +import de.nowchess.api.game.GameContext +import de.nowchess.io.GameContextImport +``` + +- [ ] **Step 2: Update class signature** + +Change: +```scala +object FenParser extends GameContextImport: +``` + +(It already extends GameContextImport; verify it does) + +- [ ] **Step 3: Update parseFen to return Either and call importGameContext** + +Replace the current `parseFen` return logic. Keep the body as-is but wrap returns: + +```scala +def parseFen(fen: String): Either[String, GameContext] = + val parts = fen.trim.split("\\s+") + if parts.length != 6 then + Left("Invalid FEN: expected 6 space-separated fields, got " + parts.length) + else + (for + board <- parseBoard(parts(0)).toRight("Invalid FEN: invalid board position") + activeColor <- parseColor(parts(1)).toRight("Invalid FEN: invalid active color (expected 'w' or 'b')") + castlingRights <- parseCastling(parts(2)).toRight("Invalid FEN: invalid castling rights") + enPassant <- parseEnPassant(parts(3)).toRight("Invalid FEN: invalid en passant square") + halfMoveClock <- parts(4).toIntOption.toRight("Invalid FEN: invalid half-move clock (expected integer)") + fullMoveNumber <- parts(5).toIntOption.toRight("Invalid FEN: invalid full move number (expected integer)") + if halfMoveClock >= 0 && fullMoveNumber >= 1 + yield GameContext( + board = board, + turn = activeColor, + castlingRights = castlingRights, + enPassantSquare = enPassant, + halfMoveClock = halfMoveClock, + moves = List.empty + )).left.map(err => "Invalid FEN: " + err) +``` + +- [ ] **Step 4: Implement importGameContext** + +```scala +def importGameContext(input: String): Either[String, GameContext] = parseFen(input) +``` + +- [ ] **Step 5: Verify parseBoard, parseColor, parseCastling, parseEnPassant still return Option** + +They do. They stay as-is. + +--- + +## Task 3: Update FenParserTest for Either + +**Files:** +- Modify: `modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala` + +- [ ] **Step 1: Update test "parse full FEN - initial position"** + +Change from: +```scala +context.isDefined shouldBe true +context.get.turn shouldBe Color.White +``` + +To: +```scala +context.isRight shouldBe true +context.getOrElse(??).turn shouldBe Color.White +``` + +Or use pattern match: +```scala +context match + case Right(ctx) => + ctx.turn shouldBe Color.White + ctx.castlingRights.whiteKingSide shouldBe true + case Left(err) => fail(s"Expected Right but got Left: $err") +``` + +- [ ] **Step 2: Update all "context.isDefined" to "context.isRight"** + +Search and replace: `context.isDefined` → `context.isRight`; `context.get` → `context.getOrElse(???)` or use pattern matching + +- [ ] **Step 3: Update error test cases** + +Change from: +```scala +context.isDefined shouldBe false +``` + +To: +```scala +context.isLeft shouldBe true +``` + +Example fixes: +- Line 89: `context.isDefined shouldBe false` → `context.isLeft shouldBe true` +- Line 95: `context.isDefined shouldBe false` → `context.isLeft shouldBe true` +- Line 101: `context.isDefined shouldBe false` → `context.isLeft shouldBe true` + +- [ ] **Step 4: Run FenParserTest** + +```bash +./gradlew :modules:io:test --tests "de.nowchess.io.fen.FenParserTest" -v +``` + +Expected: All tests pass. + +--- + +## Task 4: Implement PgnParser.importGameContext + +**Files:** +- Modify: `modules/io/src/main/scala/de/nowchess/io/pgn/PgnParser.scala` + +- [ ] **Step 1: Add GameContextImport trait to object** + +Change: +```scala +object PgnParser: +``` + +To: +```scala +object PgnParser extends GameContextImport: +``` + +- [ ] **Step 2: Implement importGameContext** + +Add this method to PgnParser: + +```scala +def importGameContext(input: String): Either[String, GameContext] = + validatePgn(input).flatMap { game => + // Replay moves to populate GameContext.moves via DefaultRules.applyMove + val (finalCtx, errors) = game.moves.foldLeft((GameContext.initial, Option.empty[String])) { + case ((ctx, Some(err)), _) => (ctx, Some(err)) // Already failed, stop + case ((ctx, None), histMove) => + val moveOpt = parseAlgebraicMove( + s"${histMove.from}${histMove.to}", + ctx, + ctx.turn + ) + moveOpt match + case None => (ctx, Some(s"Failed to parse move ${histMove.from}${histMove.to}")) + case Some(move) => + val nextCtx = DefaultRules.applyMove(ctx, move) + (nextCtx, None) + } + errors match + case Some(err) => Left(err) + case None => + if finalCtx.moves.isEmpty && game.moves.nonEmpty then + Left("No moves were parsed from the PGN") + else + Right(finalCtx) + } +``` + +- [ ] **Step 3: Ensure imports include DefaultRules** + +At top of file: +```scala +import de.nowchess.rules.sets.DefaultRules +import de.nowchess.io.GameContextImport +``` + +- [ ] **Step 4: Run PgnParserTest** + +```bash +./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnParserTest" -v +``` + +Expected: All existing tests still pass (validatePgn is unchanged). + +--- + +## Task 5: Add importGameContext tests to PgnParserTest + +**Files:** +- Modify: `modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala` + +- [ ] **Step 1: Add test for importGameContext with valid game** + +Append to PgnParserTest: + +```scala +test("importGameContext: valid PGN returns Right with GameContext") { + val pgn = """[Event "Test"] +[White "A"] +[Black "B"] + +1. e4 e5 2. Nf3 Nc6 +""" + val result = PgnParser.importGameContext(pgn) + result.isRight shouldBe true + val ctx = result.getOrElse(???) + ctx.moves.length shouldBe 4 + ctx.turn shouldBe Color.Black +} +``` + +- [ ] **Step 2: Add test for importGameContext with invalid PGN** + +```scala +test("importGameContext: invalid PGN returns Left") { + val pgn = "[Event \"T\"]\n\n1. Qd4" + val result = PgnParser.importGameContext(pgn) + result.isLeft shouldBe true + result.left.getOrElse("").nonEmpty shouldBe true +} +``` + +- [ ] **Step 3: Add test for empty moves** + +```scala +test("importGameContext: PGN with no moves returns Right with initial position") { + val pgn = "[Event \"T\"]\n[White \"A\"]\n[Black \"B\"]\n" + val result = PgnParser.importGameContext(pgn) + result.isRight shouldBe true + val ctx = result.getOrElse(???) + ctx.moves.length shouldBe 0 + ctx.board shouldBe Board.initial +} +``` + +- [ ] **Step 4: Run tests** + +```bash +./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnParserTest" -v +``` + +Expected: All tests pass. + +--- + +## Task 6: Update PgnExporter to implement GameContextExport + +**Files:** +- Modify: `modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala` + +- [ ] **Step 1: Add trait to object signature** + +Change: +```scala +object PgnExporter: +``` + +To: +```scala +object PgnExporter extends GameContextExport: +``` + +- [ ] **Step 2: Add GameContextExport import** + +At top: +```scala +import de.nowchess.io.GameContextExport +``` + +- [ ] **Step 3: Refactor exportGame to use context.moves** + +Replace current `exportGame` implementation with one that builds PGN from `GameContext.moves`. The moves are `List[Move]` not `List[HistoryMove]`, so convert: + +```scala +def exportGame(context: GameContext): String = + // Build default headers if not present + val headers = Map( + "Event" -> "?", + "White" -> "?", + "Black" -> "?", + "Result" -> "*" + ) + + val headerLines = headers.map { case (k, v) => + s"""[$k "$v"]""" + }.mkString("\n") + + val moveText = if context.moves.isEmpty then "" + else + val grouped = context.moves.zipWithIndex.groupBy(_._2 / 2) + val lines = for (idx, movePairs) <- grouped.toList.sortBy(_._1) yield + val moveNum = idx + 1 + val whiteStr = movePairs.find(_._2 % 2 == 0).map(p => moveToAlgebraicFromContext(p._1, context)).getOrElse("") + val blackStr = movePairs.find(_._2 % 2 == 1).map(p => moveToAlgebraicFromContext(p._1, context)).getOrElse("") + if blackStr.isEmpty then s"$moveNum. $whiteStr" + else s"$moveNum. $whiteStr $blackStr" + lines.mkString(" ") + " *" + + if headerLines.isEmpty then moveText + else if moveText.isEmpty then headerLines + else s"$headerLines\n\n$moveText" +``` + +Wait, this is getting complex because context.moves is `List[Move]` but `moveToAlgebraicFromContext` needs the board state before the move. Let me revise: + +```scala +def exportGameContext(context: GameContext): String = + // Use the existing GameHistory-based export for now, or + // If context.moves is empty, return headers only + if context.moves.isEmpty then + val headers = Map("Event" -> "?", "White" -> "?", "Black" -> "?") + headers.map { case (k, v) => s"""[$k "$v"]""" }.mkString("\n") + else + // Replay the game to track board state and generate notation + val headerLines = "".trim // No headers from context for now (TBD: store headers in GameContext) + val moveText = replayAndExport(context.moves) + if moveText.isEmpty then "" else moveText + " *" + +private def replayAndExport(moves: List[Move]): String = + // This requires replaying moves to get board state before each move + // For now, a simplified version: + moves.zipWithIndex.map { case (move, idx) => + val moveNum = idx / 2 + 1 + val moveStr = move.moveType match + case MoveType.CastleKingside => "O-O" + case MoveType.CastleQueenside => "O-O-O" + case _ => s"${move.to}" // Simplified, loses disambiguation + val prefix = if idx % 2 == 0 then s"$moveNum. " else "" + prefix + moveStr + }.mkString(" ") +``` + +Actually, this is too complex. Let me keep the existing signature that takes headers separately for now, and just ensure `exportGameContext` delegates: + +- [ ] **Step 1 (revised): Implement exportGameContext** + +```scala +def exportGameContext(context: GameContext): String = + // Extract default headers and export from context.moves + val defaultHeaders = Map( + "Event" -> "?", + "White" -> "?", + "Black" -> "?", + "Result" -> "*" + ) + exportGameWithHeaders(defaultHeaders, context) + +private def exportGameWithHeaders(headers: Map[String, String], context: GameContext): String = + val headerLines = headers.map { case (key, value) => + s"""[$key "$value"]""" + }.mkString("\n") + + val moveText = if context.moves.isEmpty then "" + else + val groupedMoves = context.moves.zipWithIndex.groupBy(_._2 / 2) + val moveLines = for (moveNumber, movePairs) <- groupedMoves.toList.sortBy(_._1) yield + val moveNum = moveNumber + 1 + val whiteMoveStr = movePairs.find(_._2 % 2 == 0).map(p => moveToAlgebraicFromMove(p._1)).getOrElse("") + val blackMoveStr = movePairs.find(_._2 % 2 == 1).map(p => moveToAlgebraicFromMove(p._1)).getOrElse("") + if blackMoveStr.isEmpty then s"$moveNum. $whiteMoveStr" + else s"$moveNum. $whiteMoveStr $blackMoveStr" + val termination = headers.getOrElse("Result", "*") + moveLines.mkString(" ") + s" $termination" + + if headerLines.isEmpty then moveText + else if moveText.isEmpty then headerLines + else s"$headerLines\n\n$moveText" + +private def moveToAlgebraicFromMove(move: Move): String = + move.moveType match + case MoveType.CastleKingside => "O-O" + case MoveType.CastleQueenside => "O-O-O" + case _ => move.to.toString // Simplified, loses piece disambiguation +``` + +Actually this is still lossy. The real solution requires replaying to get board state. For now, keep exportGame as-is (it takes GameHistory) and make exportGameContext a thin wrapper: + +- [ ] **Step 1 (final revision): Add exportGameContext that converts context to GameHistory** + +```scala +def exportGameContext(context: GameContext): String = + val headers = Map("Event" -> "?", "White" -> "?", "Black" -> "?", "Result" -> "*") + val history = GameHistory( + moves = context.moves.map(m => + HistoryMove(m.from, m.to) // Simplified; loses castling/promotion info + ), + halfMoveClock = context.halfMoveClock + ) + exportGame(headers, history) +``` + +Wait, this loses information from context.moves (they're `Move` not `HistoryMove`). + +Let me think: the spec says "PgnExporter generates PGN from ctx.moves with default headers". But `ctx.moves` is `List[Move]` which has no castling/promotion info baked in the structure — that's in `moveType`. We'd need to replay or store richer info. + +For MVP: keep current `exportGame(headers, history)` working, and just ensure PgnExporter extends GameContextExport. We'll leave `exportGameContext` as a TODO or a simplified version that requires the caller to still use the old API for full fidelity. + +- [ ] **Step 1 (pragmatic): Add minimal exportGameContext** + +```scala +def exportGameContext(context: GameContext): String = + // Minimal implementation: returns empty PGN if moves present but no headers + if context.moves.isEmpty then "" + else + // For now, caller must use exportGame(headers, history) for full control + // This is a placeholder that returns a minimal PGN + "* (export via exportGame for full control)" +``` + +Actually, looking back at the spec: "generates PGN from `ctx.moves` with default headers". The problem is `ctx.moves` are `Move`, not `HistoryMove`. The conversion is lossy without a board replay. + +Let me re-read the design... it says `PgnExporter.exportGameContext(ctx)` generates PGN from `ctx.moves` with default headers. But to properly generate PGN (with castling notation, capture notation, piece disambiguation), we need board state before each move. + +I think the intention is: after `DefaultRules.applyMove` is called repeatedly (in `PgnParser.importGameContext`), the final `GameContext` has all moves but we lose the intermediate states. To export, we'd need to replay. + +For pragmatism and to keep the plan simple: make `exportGameContext` replay the moves from initial position to reconstruct the game, then export via the existing logic. + +- [ ] **Step 2: Implement exportGameContext with replay** + +```scala +def exportGameContext(context: GameContext): String = + val headers = Map( + "Event" -> "?", + "White" -> "?", + "Black" -> "?", + "Result" -> "*" + ) + + // Replay all moves from initial position to get HistoryMove records + val historyMoves = scala.collection.mutable.ListBuffer[HistoryMove]() + var ctx = GameContext.initial + for move <- context.moves do + val color = ctx.turn + val pieceType = ctx.board.pieceAt(move.from).map(_.pieceType).getOrElse(PieceType.Pawn) + val isCapture = ctx.board.pieceAt(move.to).isDefined || move.moveType == MoveType.EnPassant + val castleSide = move.moveType match + case MoveType.CastleKingside => Some("Kingside") + case MoveType.CastleQueenside => Some("Queenside") + case _ => None + val promotionPiece = move.moveType match + case MoveType.Promotion(pp) => Some(pp) + case _ => None + historyMoves += HistoryMove(move.from, move.to, castleSide, promotionPiece, pieceType, isCapture) + ctx = DefaultRules.applyMove(ctx, move) + + val history = GameHistory(historyMoves.toList, context.halfMoveClock) + exportGame(headers, history) +``` + +- [ ] **Step 3: Ensure imports** + +```scala +import de.nowchess.io.GameContextExport +import de.nowchess.api.move.MoveType +import de.nowchess.api.board.PieceType +import de.nowchess.api.game.GameContext +import de.nowchess.rules.sets.DefaultRules +``` + +- [ ] **Step 4: Run PgnExporterTest** + +```bash +./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnExporterTest" -v +``` + +Expected: All existing tests still pass. + +--- + +## Task 7: Add exportGameContext tests to PgnExporterTest + +**Files:** +- Modify: `modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala` + +- [ ] **Step 1: Add test for round-trip (import then export)** + +```scala +test("exportGameContext: round-trip import->export preserves moves") { + val pgn = """[Event "Test"] +[White "A"] +[Black "B"] + +1. e4 e5 2. Nf3 Nc6 +""" + val importResult = PgnParser.importGameContext(pgn) + importResult.isRight shouldBe true + val ctx = importResult.getOrElse(???) + val exported = PgnExporter.exportGameContext(ctx) + + exported.contains("1. e4 e5") shouldBe true + exported.contains("2. Nf3 Nc6") shouldBe true +} +``` + +- [ ] **Step 2: Add test for empty context** + +```scala +test("exportGameContext: empty game returns headers only") { + val ctx = GameContext.initial + val exported = PgnExporter.exportGameContext(ctx) + + exported.contains("[Event") shouldBe true + exported.contains("*") shouldBe true // Result terminator +} +``` + +- [ ] **Step 3: Run tests** + +```bash +./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnExporterTest" -v +``` + +Expected: All tests pass. + +--- + +## Task 8: Update GameEngine to add loadGame and exportGame + +**Files:** +- Modify: `modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala` + +- [ ] **Step 1: Add loadGame method** + +Replace `loadPgn`: + +```scala +def loadGame(importer: GameContextImport, input: String): Either[String, Unit] = synchronized { + importer.importGameContext(input) match + case Left(err) => Left(err) + case Right(ctx) => + val savedContext = currentContext + currentContext = GameContext.initial + pendingPromotion = None + invoker.clear() + + var error: Option[String] = None + + if ctx.moves.isEmpty then + // No moves: just load the position + currentContext = ctx + notifyObservers(BoardResetEvent(ctx)) + Right(()) + else + // Replay moves through the command system + ctx.moves.foreach: move => + handleParsedMove(move.from, move.to) + move.moveType match + case MoveType.Promotion(pp) => completePromotion(pp) + case _ => () + if pendingPromotion.isDefined && move.moveType != MoveType.Promotion(_) then + error = Some(s"Promotion required for move ${move.from}${move.to}") + + error match + case Some(err) => + currentContext = savedContext + Left(err) + case None => + notifyObservers(PgnLoadedEvent(currentContext)) + Right(()) +} +``` + +- [ ] **Step 2: Add exportGame method** + +```scala +def exportGame(exporter: GameContextExport): String = synchronized { + exporter.exportGameContext(currentContext) +} +``` + +- [ ] **Step 3: Remove loadPgn method** + +Delete the existing `loadPgn` entirely. + +- [ ] **Step 4: Ensure imports** + +At top of GameEngine: +```scala +import de.nowchess.io.{GameContextImport, GameContextExport} +import de.nowchess.api.move.MoveType +``` + +--- + +## Task 9: Update GameEngineLoadPgnTest + +**Files:** +- Modify: `modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineLoadPgnTest.scala` + +- [ ] **Step 1: Update test "loadPgn: valid PGN"** + +Change: +```scala +engine.loadPgn(pgn) shouldBe Right(()) +``` + +To: +```scala +engine.loadGame(PgnParser, pgn) shouldBe Right(()) +``` + +And add import: +```scala +import de.nowchess.io.pgn.PgnParser +``` + +- [ ] **Step 2: Bulk replace loadPgn calls** + +Replace all `engine.loadPgn(` with `engine.loadGame(PgnParser, ` + +Affected lines (approx): +- 23, 32, 38, 48, 58, 74, 80, 145, 146 + +- [ ] **Step 3: Add test for FEN loading** + +```scala +test("loadGame(FenParser): sets position without replaying") { + val engine = new GameEngine() + val fen = "8/4P3/4k3/8/8/8/8/8 w - - 0 1" + val result = engine.loadGame(FenParser, fen) + + result shouldBe Right(()) + engine.context.moves.isEmpty shouldBe true + engine.board.pieceAt(Square(File.E, Rank.R7)) shouldBe Some(Piece.WhitePawn) +} +``` + +And add import: +```scala +import de.nowchess.io.fen.FenParser +``` + +- [ ] **Step 4: Add test for exportGame** + +```scala +test("exportGame(FenExporter): exports current position as FEN") { + val engine = new GameEngine() + engine.processUserInput("e2e4") + val fen = engine.exportGame(FenExporter) + + fen.contains("e4") shouldBe false // FEN is position format, not notation + fen.contains("P") shouldBe true // Should have pawn symbol +} + +test("exportGame(PgnExporter): exports as PGN with moves") { + val engine = new GameEngine() + engine.processUserInput("e2e4") + engine.processUserInput("e7e5") + val pgn = engine.exportGame(PgnExporter) + + pgn.contains("e4") shouldBe true + pgn.contains("e5") shouldBe true +} +``` + +And add imports: +```scala +import de.nowchess.io.fen.FenExporter +import de.nowchess.io.pgn.PgnExporter +``` + +- [ ] **Step 5: Run tests** + +```bash +./gradlew :modules:core:test --tests "de.nowchess.chess.engine.GameEngineLoadPgnTest" -v +``` + +Expected: All tests pass. + +--- + +## Task 10: Full build and test + +**Files:** +- All modules + +- [ ] **Step 1: Build all** + +```bash +./gradlew build +``` + +Expected: GREEN (no errors, no test failures). + +- [ ] **Step 2: Check coverage** + +```bash +python3 jacoco-reporter/scoverage_coverage_gaps.py modules/io/build/reports/scoverageTest/scoverage.xml +``` + +Expected: No gaps in new code. + +- [ ] **Step 3: Commit all changes** + +```bash +git add -A +git commit -m "refactor(io): unify import/export interfaces with Either and GameContext" +``` + +--- + +## Summary + +After these 10 tasks: +- `GameContextImport` now returns `Either[String, GameContext]` with error messages +- `FenParser`, `PgnParser` both implement `GameContextImport` +- `PgnExporter` implements `GameContextExport` and can export from `GameContext.moves` +- `GameEngine.loadGame(importer, input)` handles any format uniformly +- `GameEngine.exportGame(exporter)` exports to any format +- All tests updated and passing +- No breaking changes to public API (only import/export interfaces changed as designed) diff --git a/modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala b/modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala index 6690f81..c56850f 100644 --- a/modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala +++ b/modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala @@ -2,8 +2,6 @@ package de.nowchess.io import de.nowchess.api.game.GameContext -trait GameContextImport { +trait GameContextImport: - def importGameContext(input: String): Option[GameContext] - -} + def importGameContext(input: String): Either[String, GameContext] diff --git a/modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala b/modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala index 1411f68..d451a2c 100644 --- a/modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala +++ b/modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala @@ -28,7 +28,8 @@ object FenParser extends GameContextImport: moves = List.empty ) - def importGameContext(input: String): Option[GameContext] = parseFen(input) + def importGameContext(input: String): Either[String, GameContext] = + parseFen(input).toRight("Invalid FEN string") /** Parse active color ("w" or "b"). */ private def parseColor(s: String): Option[Color] =