Skip to content

Ensure Battle of the Kings commoner capture ends game#34

Open
Belzedar94 wants to merge 41 commits intomasterfrom
codex/implement-battle-of-the-kings-variant-7bhqbn
Open

Ensure Battle of the Kings commoner capture ends game#34
Belzedar94 wants to merge 41 commits intomasterfrom
codex/implement-battle-of-the-kings-variant-7bhqbn

Conversation

@Belzedar94
Copy link
Copy Markdown
Owner

Summary

  • replace Battle of the Kings gating piece with a pseudo-royal commoner and flag first-capture extinction
  • extend gating safety, SEE, and immediate game end logic to respect pseudo-royal commoners and parse the new variant option
  • add a Battle of the Kings regression test covering commoner capture victories

Testing

  • make -j2 ARCH=x86-64 build
  • python3 -m unittest test.TestPyffish

https://chatgpt.com/codex/tasks/task_e_68d14c54c3348322acceac0979446f6d

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review: Here are some suggestions.

if (Eval::useNNUE)
{
// Add gating piece
dp.piece[dp.dirty_num] = gating_piece;
dp.handPiece[dp.dirty_num] = gating_piece;
dp.handCount[dp.dirty_num] = pieceCountInHand[us][gating_type(m)];

[P1] Skip NNUE hand updates when gating piece is not taken from hand

When a gating move is executed, the code always records the gated piece in dirtyPiece.handPiece and its count in dirtyPiece.handCount. In Battle of the Kings gatingFromHand is false and no piece is removed from a hand, so pieceCountInHand[us][gating_type(m)] is zero. The NNUE feature update later subtracts one from this count, producing a -1 index and out-of-bounds access as soon as a gating move occurs with NNUE enabled. This new variant will crash or corrupt the NNUE accumulator the first time a pawn advances. The NNUE hand bookkeeping needs to be skipped or use NO_PIECE when gating_from_hand() is false.


Reply with @codex fix comments to fix any unresolved comments.

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, or 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 fix this CI failure" or "@codex address that feedback".

@Belzedar94
Copy link
Copy Markdown
Owner Author

@codex fix comments

@chatgpt-codex-connector
Copy link
Copy Markdown

Summary

  • Guarded the NNUE hand bookkeeping for gating moves so that hands are only updated when a piece is actually taken from hand, preventing negative counts for Battle of the Kings gating. src/position.cppL1972-L1994

Testing

  • make -j2 ARCH=x86-64 build

View task →

@Belzedar94
Copy link
Copy Markdown
Owner Author

@codex there is a bug, after en passant happens during the game, the gating stops working. Investigate and fix

@chatgpt-codex-connector
Copy link
Copy Markdown

Summary

  • Ensured that gates for an opponent captured en passant are cleared on the actual capture square so Battle of the Kings gating remains available after the tactic occurs. src/position.cppL2010-L2018
  • Added a Battle of the Kings regression test that performs an en passant capture and confirms a subsequent pawn move still gates a knight onto its home square. test.pyL392-L412

Testing

  • make -j2 ARCH=x86-64 build
  • python3 -m unittest test.TestPyffish (fails in existing test_battlekings_king_spawn_blocked, which now returns legal queen moves instead of none)

View task →

@Belzedar94
Copy link
Copy Markdown
Owner Author

@codex we still have the 2 issues that were supposed to be fixed, in one hand the en-passant bug (when en-passant happens, gating stops working), in the other hand the promotion bug (notation was fixed, but promotion is not working). Here are two examples:

en-passant-issue:
fen position before error occurs:
8/ppnppppp/8/2n5/2pP4/4PP2/PPPNNNPP/8 b - d3 0 3
From here the move c4d3 produces the wrong fen position
8/ppnppppp/8/2n5/8/3pPP2/PPPNNNPP/8 w - 0 4
the correct one would be
8/ppnppppp/8/2n5/2n5/3pPP2/PPPNNNPP/8 w - 0 4

promotion-issue:
fen position before error occurs:
8/ppnppppp/8/2n5/2N1P3/2P1BP2/PNPpNNPP/8 b - - 0 5
from here the move d2d1 is not accepted by FSF
correct would be d2d1 which produces the fen
8/ppnppppp/8/2n5/2N1P3/2P1BP2/PNPnNNPP/3n4 w - - 0 6

Make sure to review thoroughly, fix the issue and then add this cases to test.py

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ 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

@Belzedar94 Belzedar94 closed this Oct 1, 2025
@Belzedar94 Belzedar94 reopened this Oct 1, 2025
…-half_ka_v2_variants.cpp

Silence unused parameter warnings in NNUE variant feature
@Belzedar94 Belzedar94 force-pushed the codex/implement-battle-of-the-kings-variant-7bhqbn branch from dc16af1 to c6c3296 Compare October 1, 2025 20:59
Remove branch filters for pull requests in workflow.
…-for-battlekings

Fix Battlekings gating legality on captures
…lekings-engine

Fix battlekings gating when capturing commoners
…o codex/fix-illegal-move-in-battlekings-engine-304qlc
…lekings-engine-304qlc

Fix battlekings promotion move encoding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant