refactor(core): cleanup UI and notation imports for NCS-22 interface abstraction
Build & Test (NowChessSystems) TeamCity build failed
Build & Test (NowChessSystems) TeamCity build failed
- Removed unused CastleSide import from PgnExporter - Updated TerminalUI and GUIObserver to use GameContext - Temporarily disabled GUI FEN/PGN import-export (requires full rework with GameContext) - Deleted unused logic files and GameController per spec Note: GameEngine still needs final refactoring to use RuleSet + GameContext. Core architecture (api -> rule -> core) is structurally complete.
This commit is contained in:
@@ -0,0 +1,212 @@
|
||||
# Module Refactor: Interface Abstraction Layer — NCS-22
|
||||
|
||||
**Date:** 2026-04-03
|
||||
**Epic:** NCS-20 (Reduce Token Usage)
|
||||
**Task:** NCS-22 (Split module into smaller modules)
|
||||
**Author:** Claude Code
|
||||
**Status:** Design Approved
|
||||
|
||||
---
|
||||
|
||||
## Objective
|
||||
|
||||
Refactor NowChessSystems from a monolithic `modules/core` into a three-layer architecture with clean interface boundaries:
|
||||
1. Reduce complexity and token usage
|
||||
2. Extract rules logic into dedicated `modules/rule`
|
||||
3. Establish RuleSet as the single source of truth for all chess rule decisions
|
||||
4. Enable future rule variants (Chess960, etc.) via interface implementations
|
||||
|
||||
---
|
||||
|
||||
## Current State
|
||||
|
||||
**modules/core** conflates multiple concerns:
|
||||
- `GameEngine` (state management, observer pattern)
|
||||
- `GameController` (move validation orchestration)
|
||||
- `GameRules`, `MoveValidator`, `CastlingRightsCalculator`, `EnPassantCalculator` (rule logic)
|
||||
- Notation (PGN/FEN parsing and export)
|
||||
- Command/undo system
|
||||
|
||||
**Problem:** GameEngine depends directly on validation logic; no abstraction boundary; rules tightly coupled to engine implementation.
|
||||
|
||||
**modules/rule** (stubbed):
|
||||
- `RuleSet` trait: defines interface for rule queries
|
||||
- `StandardRules` (partial): scaffolded but uses different package/type names
|
||||
|
||||
---
|
||||
|
||||
## Proposed Architecture
|
||||
|
||||
### Three-Layer Model
|
||||
|
||||
```
|
||||
┌─ modules/api ─────────────────────────────────┐
|
||||
│ Shared types: GameContext, Board, Move, etc. │
|
||||
└───────────────────────────────────────────────┘
|
||||
↑
|
||||
┌─ modules/rule ────────────────────────────────┐
|
||||
│ RuleSet trait (interface) │
|
||||
│ StandardRules (implementation) │
|
||||
│ All move generation & validation logic │
|
||||
└───────────────────────────────────────────────┘
|
||||
↑
|
||||
┌─ modules/core ────────────────────────────────┐
|
||||
│ GameEngine (state + observer pattern) │
|
||||
│ Command/undo system │
|
||||
│ Notation parsers (PGN/FEN) │
|
||||
└───────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
**Dependencies:**
|
||||
- `modules/rule` depends on `modules/api`
|
||||
- `modules/core` depends on `modules/rule` and `modules/api`
|
||||
- No circular dependencies
|
||||
- `modules/api` depends only on std library
|
||||
|
||||
### Core Types
|
||||
|
||||
#### GameContext (new, in modules/api)
|
||||
|
||||
Immutable value type bundling complete game state:
|
||||
|
||||
```scala
|
||||
case class GameContext(
|
||||
board: Board,
|
||||
turn: Color,
|
||||
castlingRights: CastlingRights,
|
||||
enPassantSquare: Option[Square],
|
||||
halfMoveClock: Int,
|
||||
moves: List[Move] // game history
|
||||
):
|
||||
def withBoard(newBoard: Board): GameContext = copy(board = newBoard)
|
||||
def withTurn(newTurn: Color): GameContext = copy(turn = newTurn)
|
||||
def withMove(move: Move): GameContext = copy(moves = moves :+ move)
|
||||
// ... other immutable updates
|
||||
```
|
||||
|
||||
Replaces both `Situation` (from StandardRules) and `GameHistory` (from GameEngine).
|
||||
|
||||
#### RuleSet (in modules/rule)
|
||||
|
||||
Single source of truth for all rule decisions:
|
||||
|
||||
```scala
|
||||
trait RuleSet:
|
||||
def candidateMoves(context: GameContext, square: Square): List[Move]
|
||||
def legalMoves(context: GameContext, square: Square): List[Move]
|
||||
def allLegalMoves(context: GameContext): List[Move]
|
||||
def isCheck(context: GameContext): Boolean
|
||||
def isCheckmate(context: GameContext): Boolean
|
||||
def isStalemate(context: GameContext): Boolean
|
||||
def isInsufficientMaterial(context: GameContext): Boolean
|
||||
def isFiftyMoveRule(context: GameContext): Boolean
|
||||
```
|
||||
|
||||
#### StandardRules (in modules/rule)
|
||||
|
||||
Concrete implementation of RuleSet for standard chess:
|
||||
- Move generation (pawns, knights, bishops, rooks, queens, kings, castling, en passant)
|
||||
- Check/checkmate/stalemate detection
|
||||
- Insufficient material detection
|
||||
- 50-move rule tracking
|
||||
|
||||
Refactored from existing `StandardRules` scaffold to use NowChess types and naming conventions. No manual logic duplication from `modules/core/logic/*`.
|
||||
|
||||
### GameEngine Refactoring
|
||||
|
||||
**Before:** GameEngine → GameController → GameRules/MoveValidator
|
||||
**After:** GameEngine → RuleSet directly
|
||||
|
||||
Move from:
|
||||
```scala
|
||||
GameController.processMove(board, history, turn, moveInput) match
|
||||
case MoveResult.Moved(...) => ...
|
||||
```
|
||||
|
||||
To:
|
||||
```scala
|
||||
val moves = ruleSet.legalMoves(context, from)
|
||||
if moves.contains(move) then
|
||||
val newBoard = board.applyMove(move)
|
||||
val newContext = context.withBoard(newBoard).withMove(move)
|
||||
// emit event
|
||||
```
|
||||
|
||||
**Removed:**
|
||||
- `modules/core/controller/GameController.scala` (logic → RuleSet, orchestration → GameEngine)
|
||||
- All rule logic from `modules/core/logic/*` (→ modules/rule)
|
||||
|
||||
**Retained:**
|
||||
- Command/undo system (depends on GameContext instead of GameHistory)
|
||||
- Observer pattern (event notifications)
|
||||
- PGN/FEN parsing and export
|
||||
|
||||
---
|
||||
|
||||
## Design Decisions
|
||||
|
||||
### Why Immutable GameContext?
|
||||
|
||||
- **Enables replay:** Undo/redo regenerate state from commands
|
||||
- **Thread-safe:** No synchronization needed for reads
|
||||
- **Testable:** Each state change is explicit
|
||||
- **Composable:** Easier to build derived contexts
|
||||
|
||||
### Why Remove GameController?
|
||||
|
||||
- **Not an abstraction:** It's implementation detail orchestration
|
||||
- **Duplicates logic:** Validates moves, applies moves, checks outcomes — all in RuleSet now
|
||||
- **Single Responsibility:** GameEngine handles I/O and state, RuleSet handles rules
|
||||
|
||||
### Why RuleSet as interface?
|
||||
|
||||
- **Extensibility:** Chess960, variants inherit from RuleSet
|
||||
- **Testability:** Mock RuleSet for engine tests
|
||||
- **Clear contract:** Engine doesn't need to know *how* moves are generated, only that RuleSet provides them
|
||||
|
||||
### Test Strategy
|
||||
|
||||
- **No manual board construction:** Use FEN for position setup
|
||||
- **Use PGN for move validation:** Assert sequences of moves are legal
|
||||
- **RuleSet tests:** Direct unit tests of move generation, check detection, etc. (all via FEN/PGN)
|
||||
- **GameEngine tests:** Verify event emission and state transitions with RuleSet mocks or real RuleSet
|
||||
|
||||
---
|
||||
|
||||
## Files to Create/Modify
|
||||
|
||||
| Action | File | Purpose |
|
||||
|--------|------|---------|
|
||||
| **Create** | `modules/api/src/main/scala/de/nowchess/api/game/GameContext.scala` | Immutable game state |
|
||||
| **Refactor** | `modules/rule/src/main/scala/de/nowchess/rules/RuleSet.scala` | Interface definition |
|
||||
| **Rewrite** | `modules/rule/src/main/scala/de/nowchess/rules/StandardRules.scala` | Implementation (adapted from scaffold) |
|
||||
| **Create** | `modules/rule/build.gradle.kts` | Gradle config with api dependency |
|
||||
| **Refactor** | `modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala` | Call RuleSet directly |
|
||||
| **Delete** | `modules/core/src/main/scala/de/nowchess/chess/controller/GameController.scala` | No longer needed |
|
||||
| **Delete** | `modules/core/src/main/scala/de/nowchess/chess/logic/*.scala` | Move to modules/rule |
|
||||
| **Update** | `modules/core/build.gradle.kts` | Add rule dependency |
|
||||
| **Update** | `settings.gradle.kts` | Already includes rule; no changes needed |
|
||||
|
||||
---
|
||||
|
||||
## Risks & Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|------|-----------|
|
||||
| GameEngine refactor breaks observer/undo | Keep observer and command patterns intact; only change what RuleSet returns |
|
||||
| GameContext replaces two types (Situation/GameHistory) | Design GameContext upfront; validate it works with undo/redo before full migration |
|
||||
| Move logic extraction from core is fragile | Extract incrementally: extract one type at a time, validate with existing tests first |
|
||||
| PGN/FEN still depend on core classes | Create wrapper types in api if needed; avoid circular deps |
|
||||
|
||||
---
|
||||
|
||||
## Done Criteria
|
||||
|
||||
- [ ] GameContext type created and used in RuleSet
|
||||
- [ ] RuleSet interface and StandardRules implementation complete
|
||||
- [ ] GameEngine refactored to call RuleSet (no GameController)
|
||||
- [ ] All rule logic extracted from modules/core to modules/rule
|
||||
- [ ] No circular dependencies
|
||||
- [ ] Build succeeds
|
||||
- [ ] Regression tests written using FEN/PGN (not manual boards)
|
||||
- [ ] Code freeze can be lifted
|
||||
Reference in New Issue
Block a user