rbaltrusch / chess_ng

Full chess engine including chess AI
MIT License
2 stars 2 forks source link

Added 50 move draw rule, fixing #7 #34

Closed Natycat33 closed 1 year ago

Natycat33 commented 1 year ago

This fixes #7 by adding a capture flag to the move history and checking the last 50 moves against that. This capture flag should simplify #12 as well.

rbaltrusch commented 1 year ago

Hi @Natycat33, thanks for the PR, I'll review it and let you know if it can be merged :+1: At first glance two things that I want to check:

The algorithm.Minimax class currently only checks for draw by repetition, this could probably be improved by introducing a Board.is_draw() method that checks all types of draws implemented by the Board class (this way the Minimax class does not have to deal with Board internals or be coupled too much to the Board public interface).

rbaltrusch commented 1 year ago

Done with the review. Looks good to me overall, but there are a couple small issues to fix (along with some design issues on my plate). I realise this is asking for quite some changes. Feel free to only fix the basic issues (or more if you would like). As this repo is very much still a work in progress (the v1.0.2 is not quite properly semantic :smiley: ), I would be happy to take care of some of the larger design issues myself in the future. Again, thanks for the PR 👍

Natycat33 commented 1 year ago

Thanks for taking the time to review. I fixed all the minor issues you mentioned.

For the is_draw() method you mentioned, this is a bit tricky with stalemates, at least as it's currently being handled. I'm not sure the best way to implement this.

As far as optimizing by keeping track of the last pawn/capture goes, I think it does add complexity for a very minimal benefit. The check only looks at the move history when there is over 50 moves, and generally the number of iterations will be significantly less than 50. It might be worth reworking this in the future once other optimizations have been made, but I think it's a bit premature now.

rbaltrusch commented 1 year ago

@Natycat33 thanks for the fixes! Merged :+1: