Skip to content

fix: reset halfmove counter on non-pawn captures - #24

Merged
Disservin merged 2 commits into
Disservin:mainfrom
MatthewDBTodd:bugfix/halfmove-nonpawn-cap
Jul 9, 2026
Merged

fix: reset halfmove counter on non-pawn captures#24
Disservin merged 2 commits into
Disservin:mainfrom
MatthewDBTodd:bugfix/halfmove-nonpawn-cap

Conversation

@MatthewDBTodd

Copy link
Copy Markdown
Contributor

Hi, I'm working on datagen for my own chess engine, and have been using this crate for the binpack compression.

I noticed in testing that my own engine and this crate were diverging on the halfmove clock after certain moves. The issue is that while you're resetting the halfmove clock after a capture, you're then unconditionally incrementing it further down if the piece is not a pawn, clobbering the previous reset, which means that any capture made by a non-pawn is not resetting the halfmove clock. This fixes that.

I've also added a unit test which fails without the actual code changes, and passes with the changes.

Thanks for the excellent library!

Comment thread src/chess/position.rs Outdated

// Update halfmove clock
if pt == PieceType::Pawn {
if pt == PieceType::Pawn || is_capture {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think you can simply move this part (the if and else) above the // capture piece and then we don't need this is_capture variable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah good point, I should have thought of that, that's a simpler fix. Have pushed

@Disservin
Disservin merged commit 77e7ec7 into Disservin:main Jul 9, 2026
6 of 12 checks passed
@Disservin

Copy link
Copy Markdown
Owner

published v0.6.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants