diff --git a/modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala b/modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala index 074ed74..8e95169 100644 --- a/modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala +++ b/modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala @@ -418,7 +418,6 @@ class GameEngine( val contextBefore = currentContext val nextContext = ruleSet.applyMove(currentContext)(move) val captured = computeCaptured(currentContext, move) - val notation = translateMoveToNotation(move, contextBefore.board) currentContext = nextContext advanceClock(contextBefore.turn) @@ -463,13 +462,18 @@ class GameEngine( redoStack = Nil else if status.isCheck then notifyObservers(CheckDetectedEvent(currentContext)) - private def translateMoveToNotation(move: Move, boardBefore: Board): String = - move.moveType match + private def translateMoveToNotation(move: Move, ctxBefore: GameContext, status: PostMoveStatus): String = + val suffix = + if status.isCheckmate then "#" + else if status.isCheck then "+" + else "" + val base = move.moveType match case MoveType.CastleKingside => "O-O" case MoveType.CastleQueenside => "O-O-O" case MoveType.EnPassant => enPassantNotation(move) case MoveType.Promotion(pp) => promotionNotation(move, pp) - case MoveType.Normal(isCapture) => normalMoveNotation(move, boardBefore, isCapture) + case MoveType.Normal(isCapture) => normalMoveNotation(move, ctxBefore, isCapture) + base + suffix private def enPassantNotation(move: Move): String = s"${move.from.file.toString.toLowerCase}x${move.to}" @@ -482,16 +486,31 @@ class GameEngine( case PromotionPiece.Knight => "N" s"${move.to}=$ppChar" - private[engine] def normalMoveNotation(move: Move, boardBefore: Board, isCapture: Boolean): String = - boardBefore.pieceAt(move.from).map(_.pieceType) match + private[engine] def normalMoveNotation(move: Move, ctxBefore: GameContext, isCapture: Boolean): String = + ctxBefore.board.pieceAt(move.from).map(_.pieceType) match case Some(PieceType.Pawn) => if isCapture then s"${move.from.file.toString.toLowerCase}x${move.to}" else move.to.toString case Some(pt) => val letter = pieceNotation(pt) - if isCapture then s"${letter}x${move.to}" else s"$letter${move.to}" + val d = disambiguatePiece(move.from, move.to, pt, ctxBefore) + if isCapture then s"$letter${d}x${move.to}" else s"$letter$d${move.to}" case None => move.to.toString + private def disambiguatePiece(from: Square, to: Square, pieceType: PieceType, ctx: GameContext): String = + if pieceType == PieceType.King then "" + else + val competitors = ruleSet + .allLegalMoves(ctx) + .filter(m => m.to == to && m.from != from && ctx.board.pieceAt(m.from).exists(_.pieceType == pieceType)) + if competitors.isEmpty then "" + else + val sameFile = competitors.exists(_.from.file == from.file) + val sameRank = competitors.exists(_.from.rank == from.rank) + if !sameFile then from.file.toString.toLowerCase + else if !sameRank then (from.rank.ordinal + 1).toString + else from.toString + private[engine] def pieceNotation(pieceType: PieceType): String = pieceType match case PieceType.Knight => "N" @@ -519,9 +538,10 @@ class GameEngine( if currentContext.moves.isEmpty then notifyObservers(InvalidMoveEvent(currentContext, InvalidMoveReason.NothingToUndo)) else - val lastMove = currentContext.moves.last - val prevCtx = replayContextFromMoves(currentContext.moves.dropRight(1)) - val notation = translateMoveToNotation(lastMove, prevCtx.board) + val lastMove = currentContext.moves.last + val prevCtx = replayContextFromMoves(currentContext.moves.dropRight(1)) + val postStatus = ruleSet.postMoveStatus(currentContext) + val notation = translateMoveToNotation(lastMove, prevCtx, postStatus) redoStack = lastMove :: redoStack currentContext = prevCtx notifyObservers(MoveUndoneEvent(currentContext, notation)) diff --git a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala index f1edee5..60bd93b 100644 --- a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala +++ b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineIntegrationTest.scala @@ -1,6 +1,6 @@ package de.nowchess.chess.engine -import de.nowchess.api.board.{Board, Color, File, PieceType, Rank, Square} +import de.nowchess.api.board.{Color, File, PieceType, Rank, Square} import de.nowchess.api.game.GameContext import de.nowchess.api.move.{Move, MoveType, PromotionPiece} import de.nowchess.chess.observer.{GameEvent, InvalidMoveEvent, InvalidMoveReason, Observer} @@ -137,7 +137,7 @@ class GameEngineIntegrationTest extends AnyFunSuite with Matchers: test("normalMoveNotation handles missing source piece"): val engine = new GameEngine(ruleSet = DefaultRules) - val result = engine.normalMoveNotation(Move(sq("e3"), sq("e4")), Board.initial, isCapture = false) + val result = engine.normalMoveNotation(Move(sq("e3"), sq("e4")), GameContext.initial, isCapture = false) result shouldBe "e4" diff --git a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineNotationTest.scala b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineNotationTest.scala index 6482b35..e72394d 100644 --- a/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineNotationTest.scala +++ b/modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineNotationTest.scala @@ -106,7 +106,7 @@ class GameEngineNotationTest extends AnyFunSuite with Matchers: engine.undo() val evt = events.collect { case e: MoveUndoneEvent => e }.head - evt.pgnNotation shouldBe "e8=B" + evt.pgnNotation shouldBe "e8=B+" // ── King normal move notation (line 246) ─────────────────────────── @@ -134,3 +134,87 @@ class GameEngineNotationTest extends AnyFunSuite with Matchers: val evt = events.collect { case e: MoveUndoneEvent => e }.head evt.pgnNotation should startWith("K") evt.pgnNotation should include("f1") + + // ── Disambiguation: two knights same rank (file suffix) ──────────── + + test("undo with two knights on same rank disambiguates by file"): + // White knights on b1 and f3, black pawn on h7 prevents draw, both knights can reach d2 + val board = FenParser.parseBoard("k7/7p/8/8/8/5N2/8/1N5K").get + val ctx = GameContext.initial + .withBoard(board) + .withTurn(Color.White) + .withCastlingRights(de.nowchess.api.board.CastlingRights(false, false, false, false)) + + val engine = new GameEngine(ctx, DefaultRules) + val events = captureEvents(engine) + + // Knight on b1 moves to d2; notation must be "Nbd2" to disambiguate from Nf3 + engine.processUserInput("b1d2") + events.clear() + engine.undo() + + val evt = events.collect { case e: MoveUndoneEvent => e }.head + evt.pgnNotation should startWith("Nb") + evt.pgnNotation should include("d2") + + // ── Disambiguation: two knights same file (rank suffix) ──────────── + + test("undo with two knights on same file disambiguates by rank"): + // White knights on b1 and b3, both can reach d2 or c5; use b1->d2 + val board = FenParser.parseBoard("k7/7p/8/8/8/1N6/8/1N5K").get + val ctx = GameContext.initial + .withBoard(board) + .withTurn(Color.White) + .withCastlingRights(de.nowchess.api.board.CastlingRights(false, false, false, false)) + + val engine = new GameEngine(ctx, DefaultRules) + val events = captureEvents(engine) + + // Knight on b1 moves to d2; notation must be "N1d2" to disambiguate from b3 + engine.processUserInput("b1d2") + events.clear() + engine.undo() + + val evt = events.collect { case e: MoveUndoneEvent => e }.head + evt.pgnNotation should include("1") + evt.pgnNotation should include("d2") + + // ── Check suffix (+) ─────────────────────────────────────────────── + + test("undo after move that gives check emits notation with + suffix"): + // White rook a1, white king h1, black king e8; Ra1-e1 gives check on e-file + val board = FenParser.parseBoard("4k3/8/8/8/8/8/8/R6K").get + val ctx = GameContext.initial + .withBoard(board) + .withTurn(Color.White) + .withCastlingRights(de.nowchess.api.board.CastlingRights(false, false, false, false)) + + val engine = new GameEngine(ctx, DefaultRules) + val events = captureEvents(engine) + + engine.processUserInput("a1e1") + events.clear() + engine.undo() + + val evt = events.collect { case e: MoveUndoneEvent => e }.head + evt.pgnNotation should endWith("+") + + // ── Checkmate suffix (#) ────────────────────────────────────────── + + test("undo after checkmate move emits notation with # suffix"): + // Fool's mate setup (before final move): 1.f3 e5 2.g4 -- black plays Qd8-h4# + val board = FenParser.parseBoard("rnbqkbnr/pppp1ppp/8/4p3/6P1/5P2/PPPPP2P/RNBQKBNR").get + val ctx = GameContext.initial + .withBoard(board) + .withTurn(Color.Black) + .withCastlingRights(de.nowchess.api.board.CastlingRights(true, true, true, true)) + + val engine = new GameEngine(ctx, DefaultRules) + val events = captureEvents(engine) + + engine.processUserInput("d8h4") + events.clear() + engine.undo() + + val evt = events.collect { case e: MoveUndoneEvent => e }.head + evt.pgnNotation should endWith("#") diff --git a/modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala b/modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala index 4a7b832..2c4ed09 100644 --- a/modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala +++ b/modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala @@ -31,7 +31,9 @@ object PgnExporter extends GameContextExport: if moves.isEmpty then "" else val contexts = moves.scanLeft(GameContext.initial)((ctx, move) => DefaultRules.applyMove(ctx)(move)) - val sanMoves = moves.zip(contexts).map { case (move, ctx) => moveToAlgebraic(move, ctx.board) } + val sanMoves = moves.zip(contexts).zip(contexts.tail).map { case ((move, ctxBefore), ctxAfter) => + moveToAlgebraic(move, ctxBefore, ctxAfter) + } val groupedMoves = sanMoves.zipWithIndex.groupBy(_._2 / 2) val moveLines = for (moveNumber, movePairs) <- groupedMoves.toList.sortBy(_._1) yield @@ -48,9 +50,24 @@ object PgnExporter extends GameContextExport: else if moveText.isEmpty then headerLines else s"$headerLines\n\n$moveText" - /** Convert a Move to Standard Algebraic Notation using the board state before the move. */ - private def moveToAlgebraic(move: Move, boardBefore: Board): String = - move.moveType match + private def disambiguate(from: Square, to: Square, pieceType: PieceType, ctx: GameContext): String = + val competitors = DefaultRules + .allLegalMoves(ctx) + .filter(m => m.to == to && m.from != from && ctx.board.pieceAt(m.from).exists(_.pieceType == pieceType)) + if competitors.isEmpty then "" + else + val sameFile = competitors.exists(_.from.file == from.file) + val sameRank = competitors.exists(_.from.rank == from.rank) + if !sameFile then from.file.toString.toLowerCase + else if !sameRank then (from.rank.ordinal + 1).toString + else from.toString + + private def moveToAlgebraic(move: Move, ctxBefore: GameContext, ctxAfter: GameContext): String = + val suffix = + if DefaultRules.isCheckmate(ctxAfter) then "#" + else if DefaultRules.isCheck(ctxAfter) then "+" + else "" + val base = move.moveType match case MoveType.CastleKingside => "O-O" case MoveType.CastleQueenside => "O-O-O" case MoveType.EnPassant => s"${move.from.file.toString.toLowerCase}x${move.to}" @@ -60,18 +77,19 @@ object PgnExporter extends GameContextExport: case PromotionPiece.Rook => "=R" case PromotionPiece.Bishop => "=B" case PromotionPiece.Knight => "=N" - val isCapture = boardBefore.pieceAt(move.to).isDefined + val isCapture = ctxBefore.board.pieceAt(move.to).isDefined if isCapture then s"${move.from.file.toString.toLowerCase}x${move.to}$promSuffix" else s"${move.to}$promSuffix" case MoveType.Normal(isCapture) => val dest = move.to.toString val capStr = if isCapture then "x" else "" - boardBefore.pieceAt(move.from).map(_.pieceType).getOrElse(PieceType.Pawn) match + ctxBefore.board.pieceAt(move.from).map(_.pieceType).getOrElse(PieceType.Pawn) match case PieceType.Pawn => if isCapture then s"${move.from.file.toString.toLowerCase}x$dest" else dest - case PieceType.Knight => s"N$capStr$dest" - case PieceType.Bishop => s"B$capStr$dest" - case PieceType.Rook => s"R$capStr$dest" - case PieceType.Queen => s"Q$capStr$dest" + case PieceType.Knight => s"N${disambiguate(move.from, move.to, PieceType.Knight, ctxBefore)}$capStr$dest" + case PieceType.Bishop => s"B${disambiguate(move.from, move.to, PieceType.Bishop, ctxBefore)}$capStr$dest" + case PieceType.Rook => s"R${disambiguate(move.from, move.to, PieceType.Rook, ctxBefore)}$capStr$dest" + case PieceType.Queen => s"Q${disambiguate(move.from, move.to, PieceType.Queen, ctxBefore)}$capStr$dest" case PieceType.King => s"K$capStr$dest" + base + suffix diff --git a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala index 0aeed63..ae94be7 100644 --- a/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala +++ b/modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala @@ -112,3 +112,38 @@ class PgnExporterTest extends AnyFunSuite with Matchers: pgn should include("exf8=Q") pawnCapturePgn should include("exd3") quietPromotionPgn should include("e8=Q") + + test("exportGame disambiguates when two knights can reach same square"): + // 1.Nf3 a6 2.d3 a5 3.Nfd2: d3 clears d2 so both Nb1 and Nf3 can reach d2; must emit "Nfd2" + val moves = List( + Move(sq("g1"), sq("f3")), + Move(sq("a7"), sq("a6")), + Move(sq("d2"), sq("d3")), + Move(sq("a6"), sq("a5")), + Move(sq("f3"), sq("d2")), + ) + val pgn = PgnExporter.exportGame(Map.empty, moves) + pgn should include("Nfd2") + + test("exportGame appends + after move that gives check"): + // 1.e4 e5 2.Qh5 Nc6 3.Qxf7+ — queen captures f7, gives check to black king on e8 + val moves = List( + Move(sq("e2"), sq("e4")), + Move(sq("e7"), sq("e5")), + Move(sq("d1"), sq("h5")), + Move(sq("b8"), sq("c6")), + Move(sq("h5"), sq("f7"), MoveType.Normal(isCapture = true)), + ) + val pgn = PgnExporter.exportGame(Map.empty, moves) + pgn should include("Qxf7+") + + test("exportGame appends # after checkmate move"): + // Fool's mate: 1.f3 e5 2.g4 Qh4# + val moves = List( + Move(sq("f2"), sq("f3")), + Move(sq("e7"), sq("e5")), + Move(sq("g2"), sq("g4")), + Move(sq("d8"), sq("h4")), + ) + val pgn = PgnExporter.exportGame(Map("Result" -> "*"), moves) + pgn should include("Qh4#")