Conversation
winapiadmin
commented
Feb 25, 2026
winapiadmin
commented
- added copying position test
Removed coverage reporting options and related logic from CMakeLists.txt.
Refactor castling move handling for chess960 support.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d41592fcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85545c017d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
position.cpp
Outdated
| case 'K': | ||
| INVALID_ARG_IF(chess960, "Chess960 needs specific file"); | ||
| current_state.castlingRights |= WHITE_OO; | ||
| current_state.castlingMetadata[WHITE].king_start = kingSq(WHITE); | ||
| current_state.castlingMetadata[WHITE].rook_start_ks = SQ_H1; |
There was a problem hiding this comment.
When chess960=true, this branch now accepts KQkq but still hard-codes the rook starts to the orthodox a/h files. For any X-FEN style Chess960 position whose rooks are elsewhere, the castling metadata becomes wrong immediately: for example, Position("1r2k1r1/8/8/8/8/8/8/1R2K1R1 w KQkq - 0 1", true) round-trips as ... w HAha ..., and castling generation/parsing uses the wrong rook squares. Either keep rejecting KQkq in Chess960 mode or derive the rook files from the board before setting rook_start_*.
Useful? React with 👍 / 👎.
| // after the root, or repeats twice before or at the root. | ||
| inline bool is_repetition(int ply) const { return current_state.repetition && current_state.repetition < ply; } | ||
| // Test if it's draw of 75 move rule (that forces everyone to draw). Excludes checkmates, of course! | ||
| inline bool is_repetition(int ply) const { return current_state.repetition + 1 >= ply; } |
There was a problem hiding this comment.
This changes is_repetition(int ply) from the documented “distance from root” test to a raw occurrence-count check, but is_draw(int ply) still forwards the caller’s ply value unchanged. That makes the API return nonsense for existing callers: even the initial position now reports a draw at ply == 1 (Position().is_draw(1) becomes true), and any search code that passes the current ply/root distance will get false repetition results. The comment above the function still describes the old contract, so this is a behavioral regression rather than just a rename.
Useful? React with 👍 / 👎.
… tests.cpp on amount of tests)
…into maintaince_1
2 participants
