Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add pseudo legal moves generator #77

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aewhite
Copy link

@aewhite aewhite commented Feb 24, 2024

Support pseudo legal moves for the purposes of implementing variants and working with boards in incomplete states. This is particularly helpful in my use case which involves training ML models.

This PR is based heavily on the functionality of the python chess library. In particular the Board.pseudo_legal_moves feature.

A pseudo legal move is like a normal legal move except:

  1. Capturing the king is allowed
  2. The player's king may be in check at the end their turn

Castling still following normal chess rules.

@aewhite
Copy link
Author

aewhite commented Feb 24, 2024

Thanks for creating/maintaining this library. I attempted to following existing patterns and guidelines. If this PR is outside the scope of your project then I understand and will attempt to maintain my own fork. I do believe this functionality will be applicable to a range of use cases.

@Disservin
Copy link
Owner

Hi thanks for this, I did want to add this eventually but never really got around it.
Basically this duplicates a lot of code right now, I would like to avoid that.. for example by templating it with legal or pseudolegal. Also I'd ultimately want another function which tests if a pseudo legal move is legal (without generating all legal moves and checking if the move is part of that).

@Disservin
Copy link
Owner

another small nit, I typically increment the version here https://github.com/Disservin/chess-library/blob/master/src/include.hpp#L28, and I'm not sure (didn't check) if the code if formatted using clang-format. Not really important i can do that myself when the pr is ready to merge.

@aewhite
Copy link
Author

aewhite commented Feb 24, 2024

I can see about doing the templating. And I certainly didn't run clang-format. I haven't touched C++ in a LONG time so I'm pretty out of touch with the day-to-day conventions. The code for testing if a pseudo legal move is legal might be more complicated than I'm comfortable with in this PR.

Let me see what I can do I'll push some updates.

@Disservin
Copy link
Owner

Yeah, testing the pseudo legal move we can delay to a later stage..

@aewhite
Copy link
Author

aewhite commented Feb 25, 2024

Ok, my first stab at rolling the pseudo generator into the template function did not go well. The logic gets WAY more complicated due to the interactions between the legality and the mask variables. There may be an elegant way to get it right but it's not obvious to me.

Personally, I believe the paths are dissimilar enough and the core logic stable enough that having two functions is more clear and easier to maintain in the long run. The final call is obviously yours, and I take no offense to any modifications to the code as given and/or rejecting the PR outright.

I will update the version and try to run the formatter tomorrow and then I'll leave it in your hands. Either way, thanks for reviewing.

@Disservin
Copy link
Owner

I just updated it with what I imagined, I think your old code also had a bug where it didnt generate pin_hv for castling moves, which could lead to illegal castling moves.

src/movegen.hpp Outdated Show resolved Hide resolved
@aewhite
Copy link
Author

aewhite commented Feb 25, 2024

This looks good. My only comments are:

  1. The current code will fail if there is no king on the board (that has debatable applications)
  2. Before my latest commit, the code would fail in double check cases which are pretty common in pseudo cases due to the related check/pin rules.

I'm testing this by simulating random games in a loop. The game ends under normal chess rules OR a king is captured.

Unrelated to correctness, this produces ~24k games/s which is amazing for my use cases.

@Disservin
Copy link
Owner

Disservin commented Feb 25, 2024

btw i will leave this open until i get around to writing some unit tests for this..
or if you want to write some, here's a command to easier test specific test suites without testing the entire lib

meson test -C build --test-args="--test-suite=\"NAME\""

just replace NAME with the new test suite name

@Disservin Disservin added the enhancement New feature or request label Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants