Disservin / chess-library

C++ chess library
https://disservin.github.io/chess-library/
MIT License
73 stars 21 forks source link

feat: add pseudo legal moves generator #77

Open aewhite opened 7 months ago

aewhite commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

aewhite commented 7 months ago

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 commented 7 months ago

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.

aewhite commented 7 months ago

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 commented 7 months ago

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