refactor: NCS-22 NCS-23 reworked modules and tests #17

Merged
Janis merged 42 commits from refactor/NCS-22 into main 2026-04-06 09:07:40 +02:00
Member
No description provided.
Janis added 41 commits 2026-04-06 09:00:48 +02:00
Bundles complete game state (board, turn, castling rights, en passant, halfMoveClock, moves)
with immutable builder methods for functional state transitions.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Replace Situation parameter with GameContext across all RuleSet methods
to align with the new game state abstraction. Updated imports to use the
api module's types (GameContext, Square, Move).

StandardRules will need to be updated in Task 3 to implement the new
interface signature and use api types instead of maichess.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Task 6: Updated all GameEvent case classes to use context: GameContext instead of separate board/history/turn
Task 7: Deleted old logic files and restored as compatibility layer in modules/api
Task 8: Verified build - main source compilation succeeds

Changes:
- Updated Observer.scala events: all GameEvent subclasses now accept GameContext
- Restored GameHistory and HistoryMove to modules/api as compatibility types
- Restored logic files (GameRules, MoveValidator, etc.) with updated imports
- Updated GameEngine to use currentContext helper for event creation
- Updated GameController to convert CastleSide to String for HistoryMove
- Updated PgnParser/PgnExporter to work with String representation of castling
- Added modules:rule to settings.gradle.kts for dependency resolution
- All main source code compiles successfully; tests expected to need refactoring

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- 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.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- Update GameContextExport trait to use Scala 3 syntax (colon-based) to match GameContextImport
- Add test coverage for FenParser.importGameContext method:
  * Valid FEN string returns Right[GameContext] with correct context data
  * Invalid FEN string returns Left[String] with error message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Update parseFen to return Either[String, GameContext] with specific error messages for each validation failure:
- Invalid parts count: reports expected 6 fields
- Invalid board: clear message about board position
- Invalid color: explains expected 'w' or 'b'
- Invalid castling, en passant, and move counts with clear descriptions

Simplify importGameContext to delegate directly to parseFen.

Keep helper methods (parseColor, parseCastling, parseEnPassant, etc.) returning Option as before.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FenParser.parseFen now returns Either[String, GameContext] instead of Option.
Updated all tests to use Either combinators (isRight, isLeft, fold) instead
of Option methods (isDefined, get). Both FenParserTest and FenExporterTest
now properly handle the Either-based return type.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added 3 new tests for missing error paths:
- Invalid en passant square parsing (e.g., "x5")
- Half-move clock non-integer (e.g., "abc")
- Full-move number non-integer (e.g., "abc")

Updated 3 existing error tests to verify error messages:
- "expected 6" for invalid parts count
- "color" for invalid active color
- "castling" for invalid castling rights

All error assertions now use .fold() to verify message content, improving test robustness. Test count increased from 19 to 22.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implement the importGameContext method to validate PGN input via validatePgn()
and replay each move using DefaultRules.applyMove to populate GameContext.moves.
Returns final GameContext with all moves applied or error message on failure.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove unnecessary if/else check (lines 49-50) that could never be true.
DefaultRules.applyMove always calls .withMove(move) on the context,
so finalCtx.moves is always populated when game.moves.nonEmpty.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added three test cases to validate importGameContext method:
1. Valid PGN with moves returns Right with populated GameContext
2. Invalid PGN (illegal move) returns Left with error message
3. PGN with no moves returns Right with initial position

Tests use coordinate notation (e.g., e2e4) compatible with importGameContext's move replay mechanism.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PgnExporter now extends GameContextExport and implements exportGameContext,
which replays moves from GameContext.initial to reconstruct HistoryMove records
with proper castling/promotion info before generating PGN.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace unsafe fallback to Pawn with explicit exception on invariant
violation. This ensures data corruption in GameContext.moves is detected
immediately rather than producing silently incorrect PGN output.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add two simple tests for exportGameContext functionality:
1. Test that moves are preserved in PGN output
2. Test that empty game exports headers with proper terminator

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add imports for GameContextImport and GameContextExport
- Add loadGame(importer, input) method to load game from importer
- Add exportGame(exporter) method to export current game context
- Remove entire loadPgn method (replaced by loadGame)
- Remove GameEngineLoadPgnTest (tests old loadPgn API)

loadGame supports both move replay and direct position loading:
- If no moves, sets position directly via loadPosition
- If moves exist, replays through command system for undo/redo support
- Notifies PgnLoadedEvent on success

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add 3 focused tests for GameEngine load/export functionality:
- loadGame with PgnParser validates PGN parsing and undo/redo state
- loadGame with FenParser validates position loading without move replay
- exportGame with PgnExporter validates PGN output

Fix: PgnParser now extends GameContextImport trait for consistency with FenParser.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Janis added 1 commit 2026-04-06 09:03:58 +02:00
docs: moved plans and specs to correct repo
Build & Test (NowChessSystems) TeamCity build finished
41c05a08e8
Janis merged commit 8f56a82104 into main 2026-04-06 09:07:40 +02:00
Janis deleted branch refactor/NCS-22 2026-04-06 09:07:40 +02:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: NowChess/NowChessSystems#17