feat: refactor move application logic to improve clarity and maintainability
Build & Test (NowChessSystems) TeamCity build failed
Build & Test (NowChessSystems) TeamCity build failed
This commit is contained in:
File diff suppressed because it is too large
Load Diff
@@ -1,243 +0,0 @@
|
||||
# Pawn Promotion Design — NCS-10
|
||||
|
||||
**Date:** 2026-03-29
|
||||
**Issue:** NCS-10 — Implement Pawn Promotion
|
||||
**Modules:** `modules/api` (domain types), `modules/core` (logic, game loop)
|
||||
|
||||
## Overview
|
||||
|
||||
Pawn promotion is a **two-step interaction**: when a pawn reaches the opponent's back rank, the game pauses and prompts the player to choose a promotion piece (Queen, Rook, Bishop, or Knight). The move is not complete until a piece is selected. The choice is recorded in game history so promotions survive FEN/PGN serialization and round-trips.
|
||||
|
||||
## Requirements (from DoD)
|
||||
|
||||
- [x] Promotion is mandatory — move is not completed until piece is chosen
|
||||
- [x] All four promotion targets are selectable (Q, R, B, N)
|
||||
- [x] Underpromotion (e.g. to knight) works correctly
|
||||
- [x] PGN notation records the promotion piece (e.g. e8=Q)
|
||||
- [x] Tests cover: promotion to each piece, promotion via capture, underpromotion
|
||||
|
||||
## Architecture
|
||||
|
||||
### 1. History Recording
|
||||
|
||||
**File:** `modules/core/src/main/scala/de/nowchess/chess/logic/GameHistory.scala`
|
||||
|
||||
Extend the `Move` type to record promotion choices:
|
||||
|
||||
```scala
|
||||
case class Move(
|
||||
from: Square,
|
||||
to: Square,
|
||||
castleSide: Option[CastleSide],
|
||||
promotionPiece: Option[PromotionPiece] = None
|
||||
)
|
||||
```
|
||||
|
||||
- `promotionPiece = None` for non-promotion moves
|
||||
- `promotionPiece = Some(Queen|Rook|Bishop|Knight)` for promotion moves
|
||||
- `addMove()` overloaded to accept promotion piece: `addMove(from, to, castleSide?, promotionPiece?)`
|
||||
|
||||
### 2. Move Validation
|
||||
|
||||
**File:** `modules/core/src/main/scala/de/nowchess/chess/logic/MoveValidator.scala`
|
||||
|
||||
Add promotion detection:
|
||||
|
||||
```scala
|
||||
def isPromotionMove(board: Board, from: Square, to: Square): Boolean =
|
||||
board.pieceAt(from) match
|
||||
case Some(Piece(_, PieceType.Pawn)) =>
|
||||
val destRank = to.rank
|
||||
(from.rank == Rank.R7 && destRank == Rank.R8) || // White pawn to R8
|
||||
(from.rank == Rank.R2 && destRank == Rank.R1) // Black pawn to R1
|
||||
case _ => false
|
||||
```
|
||||
|
||||
This identifies when a move is pawn reaching the back rank. The move is **legal** (passes `isLegal()`), but **incomplete** until a promotion piece is chosen.
|
||||
|
||||
### 3. Game Loop Flow
|
||||
|
||||
**File:** `modules/core/src/main/scala/de/nowchess/chess/controller/GameController.scala`
|
||||
|
||||
#### New MoveResult variant:
|
||||
|
||||
```scala
|
||||
case class PromotionRequired(
|
||||
from: Square,
|
||||
to: Square,
|
||||
newBoard: Board,
|
||||
newHistory: GameHistory,
|
||||
captured: Option[Piece],
|
||||
newTurn: Color
|
||||
) extends MoveResult
|
||||
```
|
||||
|
||||
#### Flow in `processMove()`:
|
||||
|
||||
1. Validate move is legal (existing logic)
|
||||
2. Detect castling or promotion:
|
||||
- If castling → apply transformation, return `Moved` / `MovedInCheck`
|
||||
- If promotion → return `PromotionRequired` (move board state pre-promotion, pawn still on source square)
|
||||
- Otherwise → apply move, return `Moved` / `MovedInCheck`
|
||||
|
||||
#### New function: Complete promotion
|
||||
|
||||
```scala
|
||||
def completePromotion(
|
||||
board: Board,
|
||||
history: GameHistory,
|
||||
from: Square,
|
||||
to: Square,
|
||||
piece: PromotionPiece,
|
||||
turn: Color
|
||||
): MoveResult
|
||||
```
|
||||
|
||||
This applies the pawn move, places the promoted piece, and returns `Moved` or `MovedInCheck`.
|
||||
|
||||
#### Loop integration:
|
||||
|
||||
```scala
|
||||
def gameLoop(board: Board, history: GameHistory, turn: Color): Unit =
|
||||
// ... existing render + prompt
|
||||
processMove(board, history, turn, input) match
|
||||
case MoveResult.PromotionRequired(from, to, newBoard, newHistory, captured, newTurn) =>
|
||||
println("Promote to: (q/r/b/n)? ")
|
||||
val pieceInput = StdIn.readLine().trim.toLowerCase
|
||||
val piece = pieceInput match
|
||||
case "q" => Some(PromotionPiece.Queen)
|
||||
case "r" => Some(PromotionPiece.Rook)
|
||||
case "b" => Some(PromotionPiece.Bishop)
|
||||
case "n" => Some(PromotionPiece.Knight)
|
||||
case _ => None
|
||||
piece match
|
||||
case None =>
|
||||
println("Invalid piece. Choose (q/r/b/n).")
|
||||
gameLoop(board, history, turn) // retry promotion choice
|
||||
case Some(p) =>
|
||||
// completePromotion returns a MoveResult (Moved or MovedInCheck)
|
||||
// and is handled recursively through the same loop
|
||||
completePromotion(newBoard, newHistory, from, to, p, turn, newTurn) match
|
||||
case result: MoveResult.Moved =>
|
||||
// handle as normal move
|
||||
gameLoop(result.newBoard, result.newHistory, result.newTurn)
|
||||
case result: MoveResult.MovedInCheck =>
|
||||
// handle check state
|
||||
gameLoop(result.newBoard, result.newHistory, result.newTurn)
|
||||
case _ =>
|
||||
// should not happen
|
||||
gameLoop(board, history, turn)
|
||||
case other => // existing cases (Quit, InvalidFormat, NoPiece, WrongColor, IllegalMove, Moved, MovedInCheck, Checkmate, Stalemate)
|
||||
```
|
||||
|
||||
### 4. PGN Support
|
||||
|
||||
**File:** `modules/core/src/main/scala/de/nowchess/chess/notation/PgnExporter.scala`
|
||||
|
||||
When exporting a move that includes promotion:
|
||||
|
||||
```scala
|
||||
def moveToSan(move: HistoryMove): String =
|
||||
val base = s"${move.from}${move.to}"
|
||||
move.promotionPiece match
|
||||
case Some(piece) => s"$base=${piece.label.head.toUpperCase}"
|
||||
case None => base
|
||||
```
|
||||
|
||||
Output: `e7e8=Q`, `e7e8=n` (underpromotion to knight), etc.
|
||||
|
||||
**File:** `modules/core/src/main/scala/de/nowchess/chess/notation/PgnParser.scala`
|
||||
|
||||
Parse promotion notation during PGN import:
|
||||
|
||||
```scala
|
||||
def parsePromotion(move: String): Option[PromotionPiece] =
|
||||
// Extract '=Q' suffix and convert to PromotionPiece
|
||||
```
|
||||
|
||||
### 5. Test Coverage
|
||||
|
||||
**Files:**
|
||||
- `modules/core/src/test/scala/de/nowchess/chess/logic/MoveValidatorTest.scala`
|
||||
- `modules/core/src/test/scala/de/nowchess/chess/controller/GameControllerTest.scala`
|
||||
|
||||
Test scenarios (using FEN to set up board positions):
|
||||
|
||||
1. **Promotion detection:**
|
||||
- White pawn on e7 moving to e8 → `isPromotionMove` returns `true`
|
||||
- Black pawn on e2 moving to e1 → `isPromotionMove` returns `true`
|
||||
- Pawn moving but not to back rank → `isPromotionMove` returns `false`
|
||||
|
||||
2. **Each piece type:**
|
||||
- Promote to Queen: e7e8 + "q" → pawn becomes Queen
|
||||
- Promote to Rook: e7e8 + "r" → pawn becomes Rook
|
||||
- Promote to Bishop: e7e8 + "b" → pawn becomes Bishop
|
||||
- Promote to Knight: e7e8 + "n" → pawn becomes Knight
|
||||
|
||||
3. **Capture + promotion:**
|
||||
- Pawn captures enemy piece while promoting (e7d8 capturing bishop + promote to Queen)
|
||||
|
||||
4. **Underpromotion:**
|
||||
- Promote to Knight instead of Queen (strategic underpromotion)
|
||||
|
||||
5. **Both colors:**
|
||||
- White pawn (R7 → R8)
|
||||
- Black pawn (R2 → R1)
|
||||
|
||||
6. **Rejection cases:**
|
||||
- Pawn blocked on back rank (no move completes)
|
||||
- Illegal capture during promotion
|
||||
|
||||
7. **History recording:**
|
||||
- Move with promotion records `promotionPiece` field
|
||||
- Move without promotion has `promotionPiece = None`
|
||||
|
||||
8. **Game flow:**
|
||||
- `processMove()` returns `PromotionRequired`
|
||||
- `completePromotion()` advances game state correctly
|
||||
- Game status (check, mate, draw) evaluated after promotion completes
|
||||
|
||||
## Data Flow Diagram
|
||||
|
||||
```
|
||||
User input: "e7e8"
|
||||
↓
|
||||
processMove() → parseMove() → (Square(E, R7), Square(E, R8))
|
||||
↓
|
||||
Validate legality → MoveValidator.isLegal(board, history, from, to)
|
||||
↓
|
||||
Detect promotion? → MoveValidator.isPromotionMove(board, from, to)
|
||||
↓
|
||||
Yes → return PromotionRequired(from, to, board, history, ...)
|
||||
↓
|
||||
gameLoop handles result, prompts: "Promote to: (q/r/b/n)?"
|
||||
↓
|
||||
User input: "q"
|
||||
↓
|
||||
completePromotion(board, history, from, to, Queen, turn)
|
||||
↓
|
||||
Apply pawn move, place Queen, record in history with promotionPiece=Queen
|
||||
↓
|
||||
Evaluate game status, continue loop
|
||||
```
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
- **Promotion is not a choice in `processMove()`** — the function only detects and pauses. The loop handles the interaction.
|
||||
- **The board state in `PromotionRequired` is unchanged** — pawn still on source square until `completePromotion()` applies the move.
|
||||
- **Castling remains independent** — no interaction between promotion and castling logic.
|
||||
- **Coverage goals:** 100% line, branch, and method for all new code (per CLAUDE.md).
|
||||
- **Naming:** Rename `de.nowchess.chess.logic.Move` to `HistoryMove` to avoid collision with `de.nowchess.api.move.Move` (feedback from prior work).
|
||||
|
||||
## Scope
|
||||
|
||||
- Core: move validation, history recording, game loop interaction
|
||||
- API: types already exist (`PromotionPiece`, `MoveType.Promotion`)
|
||||
- Notation: PGN export/import support (deferred if integration tests pass without it)
|
||||
- Rendering: no UI changes beyond console prompts
|
||||
|
||||
## Risks
|
||||
|
||||
- **Off-by-one errors on rank detection:** White R7→R8, Black R2→R1. Tests must verify both.
|
||||
- **Game status evaluation:** Must evaluate check/mate/stalemate *after* promotion completes, not before.
|
||||
- **Backward compatibility:** Extending `GameHistory.Move` requires migration of existing saves (none yet; not a blocker).
|
||||
@@ -42,26 +42,30 @@ object PgnParser:
|
||||
if isMoveNumberOrResult(token) then state
|
||||
else
|
||||
parseAlgebraicMove(token, board, history, color) match
|
||||
case None => state // unrecognised token — skip silently
|
||||
case None => state // unrecognised token — skip silently
|
||||
case Some(move) =>
|
||||
val newBoard = move.castleSide match
|
||||
case Some(side) => board.withCastle(color, side)
|
||||
case None =>
|
||||
val (boardAfterMove, _) = board.withMove(move.from, move.to)
|
||||
move.promotionPiece match
|
||||
case Some(pp) =>
|
||||
val pieceType = pp match
|
||||
case PromotionPiece.Queen => PieceType.Queen
|
||||
case PromotionPiece.Rook => PieceType.Rook
|
||||
case PromotionPiece.Bishop => PieceType.Bishop
|
||||
case PromotionPiece.Knight => PieceType.Knight
|
||||
boardAfterMove.updated(move.to, Piece(color, pieceType))
|
||||
case None => boardAfterMove
|
||||
val newBoard = applyMoveToBoard(board, move, color)
|
||||
val newHistory = history.addMove(move)
|
||||
(newBoard, newHistory, color.opposite, acc :+ move)
|
||||
|
||||
moves
|
||||
|
||||
/** Apply a single HistoryMove to a Board, handling castling and promotion. */
|
||||
private def applyMoveToBoard(board: Board, move: HistoryMove, color: Color): Board =
|
||||
move.castleSide match
|
||||
case Some(side) => board.withCastle(color, side)
|
||||
case None =>
|
||||
val (boardAfterMove, _) = board.withMove(move.from, move.to)
|
||||
move.promotionPiece match
|
||||
case Some(pp) =>
|
||||
val pieceType = pp match
|
||||
case PromotionPiece.Queen => PieceType.Queen
|
||||
case PromotionPiece.Rook => PieceType.Rook
|
||||
case PromotionPiece.Bishop => PieceType.Bishop
|
||||
case PromotionPiece.Knight => PieceType.Knight
|
||||
boardAfterMove.updated(move.to, Piece(color, pieceType))
|
||||
case None => boardAfterMove
|
||||
|
||||
/** True for move-number tokens ("1.", "12.") and PGN result tokens. */
|
||||
private def isMoveNumberOrResult(token: String): Boolean =
|
||||
token.matches("""\d+\.""") ||
|
||||
|
||||
Reference in New Issue
Block a user