From 0778882db61e2881d36075a618a30b9d5b840831 Mon Sep 17 00:00:00 2001 From: Janis Date: Sun, 5 Apr 2026 17:48:06 +0200 Subject: [PATCH] refactor(core): simplify castling logic and improve move translation methods --- .../de/nowchess/chess/engine/GameEngine.scala | 64 +++++++++-------- .../de/nowchess/rules/sets/DefaultRules.scala | 72 ++++++++----------- .../de/nowchess/ui/terminal/TerminalUI.scala | 1 - 3 files changed, 66 insertions(+), 71 deletions(-) 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 7d2a693..8051a94 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 @@ -7,7 +7,6 @@ import de.nowchess.chess.controller.Parser import de.nowchess.chess.observer.* import de.nowchess.chess.command.{CommandInvoker, MoveCommand, MoveResult} import de.nowchess.io.{GameContextImport, GameContextExport} -import de.nowchess.io.pgn.PgnParser import de.nowchess.rules.RuleSet import de.nowchess.rules.sets.DefaultRules @@ -142,34 +141,43 @@ class GameEngine( importer.importGameContext(input) match case Left(err) => Left(err) case Right(ctx) => - val savedContext = currentContext - currentContext = GameContext.initial - pendingPromotion = None - invoker.clear() - - if ctx.moves.isEmpty then - currentContext = ctx + replayGame(ctx).map { _ => notifyObservers(PgnLoadedEvent(currentContext)) - Right(()) - else - var error: Option[String] = None - ctx.moves.foreach: move => - handleParsedMove(move.from, move.to) - move.moveType match - case MoveType.Promotion(pp) => - if pendingPromotion.isDefined then completePromotion(pp) - else error = Some(s"Promotion required for move ${move.from}${move.to}") - case _ => () - - error match - case Some(err) => - currentContext = savedContext - Left(err) - case None => - notifyObservers(PgnLoadedEvent(currentContext)) - Right(()) + } } + private def replayGame(ctx: GameContext): Either[String, Unit] = + val savedContext = currentContext + currentContext = GameContext.initial + pendingPromotion = None + invoker.clear() + + if ctx.moves.isEmpty then + currentContext = ctx + Right(()) + else + replayMoves(ctx.moves, savedContext) + + private def replayMoves(moves: List[Move], savedContext: GameContext): Either[String, Unit] = + var error: Option[String] = None + moves.foreach: move => + if error.isEmpty then + handleParsedMove(move.from, move.to) + move.moveType match + case MoveType.Promotion(pp) => + if pendingPromotion.isDefined then + completePromotion(pp) + else + error = Some(s"Promotion required for move ${move.from}${move.to}") + case _ => () + + error match + case Some(err) => + currentContext = savedContext + Left(err) + case None => + Right(()) + /** Export the current game context using the provided exporter. */ def exportGame(exporter: GameContextExport): String = synchronized { exporter.exportGameContext(currentContext) @@ -202,7 +210,7 @@ class GameEngine( to = move.to, moveResult = Some(MoveResult.Successful(nextContext, captured)), previousContext = Some(contextBefore), - notation = moveToPgn(move, contextBefore.board) + notation = translateMoveToNotation(move, contextBefore.board) ) invoker.execute(cmd) currentContext = nextContext @@ -229,7 +237,7 @@ class GameEngine( if currentContext.halfMoveClock >= 100 then notifyObservers(FiftyMoveRuleAvailableEvent(currentContext)) - private def moveToPgn(move: Move, boardBefore: Board): String = + private def translateMoveToNotation(move: Move, boardBefore: Board): String = move.moveType match case MoveType.CastleKingside => "O-O" case MoveType.CastleQueenside => "O-O-O" diff --git a/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala b/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala index 1c665f6..59399cf 100644 --- a/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala +++ b/modules/rule/src/main/scala/de/nowchess/rules/sets/DefaultRules.scala @@ -138,25 +138,10 @@ object DefaultRules extends RuleSet: if from != expected then List.empty else val moves = scala.collection.mutable.ListBuffer[Move]() - - // King-side castling - if context.castlingRights.whiteKingSide then - val clearSqs = List("f1", "g1").flatMap(Square.fromAlgebraic) - if squaresEmpty(context.board, clearSqs) then - for - kf <- Square.fromAlgebraic("e1") - kt <- Square.fromAlgebraic("g1") - do moves += Move(kf, kt, MoveType.CastleKingside) - - // Queen-side castling - if context.castlingRights.whiteQueenSide then - val clearSqs = List("b1", "c1", "d1").flatMap(Square.fromAlgebraic) - if squaresEmpty(context.board, clearSqs) then - for - kf <- Square.fromAlgebraic("e1") - kt <- Square.fromAlgebraic("c1") - do moves += Move(kf, kt, MoveType.CastleQueenside) - + addCastleMove(context, moves, context.castlingRights.whiteKingSide, + "e1", "g1", "f1", "h1", MoveType.CastleKingside) + addCastleMove(context, moves, context.castlingRights.whiteQueenSide, + "e1", "c1", "d1", "a1", MoveType.CastleQueenside) moves.toList private def blackCastles(context: GameContext, from: Square): List[Move] = @@ -164,27 +149,30 @@ object DefaultRules extends RuleSet: if from != expected then List.empty else val moves = scala.collection.mutable.ListBuffer[Move]() - - // King-side castling - if context.castlingRights.blackKingSide then - val clearSqs = List("f8", "g8").flatMap(Square.fromAlgebraic) - if squaresEmpty(context.board, clearSqs) then - for - kf <- Square.fromAlgebraic("e8") - kt <- Square.fromAlgebraic("g8") - do moves += Move(kf, kt, MoveType.CastleKingside) - - // Queen-side castling - if context.castlingRights.blackQueenSide then - val clearSqs = List("b8", "c8", "d8").flatMap(Square.fromAlgebraic) - if squaresEmpty(context.board, clearSqs) then - for - kf <- Square.fromAlgebraic("e8") - kt <- Square.fromAlgebraic("c8") - do moves += Move(kf, kt, MoveType.CastleQueenside) - + addCastleMove(context, moves, context.castlingRights.blackKingSide, + "e8", "g8", "f8", "h8", MoveType.CastleKingside) + addCastleMove(context, moves, context.castlingRights.blackQueenSide, + "e8", "c8", "d8", "a8", MoveType.CastleQueenside) moves.toList + private def addCastleMove( + context: GameContext, + moves: scala.collection.mutable.ListBuffer[Move], + castlingRight: Boolean, + kingFromAlg: String, + kingToAlg: String, + middleAlg: String, + rookAlg: String, + moveType: MoveType + ): Unit = + if castlingRight then + val clearSqs = List(middleAlg, kingToAlg).flatMap(Square.fromAlgebraic) + if squaresEmpty(context.board, clearSqs) then + for + kf <- Square.fromAlgebraic(kingFromAlg) + kt <- Square.fromAlgebraic(kingToAlg) + do moves += Move(kf, kt, moveType) + private def squaresEmpty(board: Board, squares: List[Square]): Boolean = squares.forall(sq => board.pieceAt(sq).isEmpty) @@ -287,12 +275,12 @@ object DefaultRules extends RuleSet: val newBoard = move.moveType match case MoveType.CastleKingside => applyCastle(board, color, kingside = true) case MoveType.CastleQueenside => applyCastle(board, color, kingside = false) - case MoveType.EnPassant => applyEnPassant(board, move, color) + case MoveType.EnPassant => applyEnPassant(board, move) case MoveType.Promotion(pp) => applyPromotion(board, move, color, pp) case MoveType.Normal(_) => board.applyMove(move) val newCastlingRights = updateCastlingRights(context.castlingRights, board, move, color) - val newEnPassantSquare = computeEnPassantSquare(board, move, color) + val newEnPassantSquare = computeEnPassantSquare(board, move) val isCapture = move.moveType match case MoveType.Normal(capture) => capture case MoveType.EnPassant => true @@ -322,7 +310,7 @@ object DefaultRules extends RuleSet: .updated(kingTo, king) .updated(rookTo, rook) - private def applyEnPassant(board: Board, move: Move, color: Color): Board = + private def applyEnPassant(board: Board, move: Move): Board = val capturedRank = move.from.rank // the captured pawn is on the same rank as the moving pawn val capturedSquare = Square(move.to.file, capturedRank) board.applyMove(move).removed(capturedSquare) @@ -360,7 +348,7 @@ object DefaultRules extends RuleSet: if move.to == blackQueensideRook then r = r.revokeQueenSide(Color.Black) r - private def computeEnPassantSquare(board: Board, move: Move, color: Color): Option[Square] = + private def computeEnPassantSquare(board: Board, move: Move): Option[Square] = val piece = board.pieceAt(move.from) val isDoublePawnPush = piece.exists(_.pieceType == PieceType.Pawn) && math.abs(move.to.rank.ordinal - move.from.rank.ordinal) == 2 diff --git a/modules/ui/src/main/scala/de/nowchess/ui/terminal/TerminalUI.scala b/modules/ui/src/main/scala/de/nowchess/ui/terminal/TerminalUI.scala index a71a531..925425f 100644 --- a/modules/ui/src/main/scala/de/nowchess/ui/terminal/TerminalUI.scala +++ b/modules/ui/src/main/scala/de/nowchess/ui/terminal/TerminalUI.scala @@ -59,7 +59,6 @@ class TerminalUI(engine: GameEngine) extends Observer: printPrompt(e.context.turn) case _: PromotionRequiredEvent => - // TODO: Promotion handling needs rework in new architecture println("Promote to: q=Queen, r=Rook, b=Bishop, n=Knight") synchronized { awaitingPromotion = true } case _: DrawClaimedEvent =>