bhlangonijr / chesslib

chess library for legal move generation, FEN/PGN parsing and more
Apache License 2.0
223 stars 78 forks source link

isMoveLegal does not behave as expected #76

Closed phikui closed 2 years ago

phikui commented 2 years ago

I am using this library to create a chess GUI and I noticed that isMoveLegal validates some moves that are clearly illegal

 Board board = new Board();

// Move pawn at D2 across entire board capturing at H7
Move illegalMove = new Move(Square.D2, Square.H7);

System.out.println(board.isMoveLegal(illegalMove, true));

Expected behavior: print false

Observed behavior: print true

The workaround I am using for now is the following since move generation seems to be correct:

if (board.legalMoves().contains(selected_move)) {
    board.doMove(selected_move);
}
bhlangonijr commented 2 years ago

Hi @phikui , It may sound a bit counter-intuitive but the API is actually behaving as expected. Board#isLegalMove() is only checking if that move is not gonna leave the current chess Board in an illegal state after the move is played. You could, for example, design a chess UI where you can drag and drop chess pieces for setting up an arbitrary chess position where you are free to shuffle around all the pieces. It's not checking for a legal move within the current chessboard position. If you want to play only legal moves for the current chess position you should instead generate the moves using the method board.legalMoves(): you pick one of the moves generated by this method to play it on the board. You can otherwise play any move as long as it is not leaving the board in an illegal state. You can certify if the moves generated by this method are really legal and there is no bug by testing the position with a perft function. It's explained here: https://github.com/bhlangonijr/chesslib#sanity-checking-of-chesslib-move-generation-with-perft

phikui commented 2 years ago

Thank you for the answer, but it is counter-intuitive in the naming as you mention.

But I have a follow-up question that goes back to the original scenario of having a move supplied externally, say user input. Is generating all moves to do

if (board.legalMoves().contains(selected_move)) {
    board.doMove(selected_move);
}

the intended way to do this?

bhlangonijr commented 2 years ago

yes, this is how you do it if the user input is intended to be played on the board by applying the standard chess rules. Now, say you are creating a chess board editor and the user is just shuffling around the pieces on it. In that case you just need to call Board#isMoveLegal() to make sure the resulting position is a valid one.

Also, by getting all legal moves you could do things like highlight valid target squares, traverse the game tree to evaluate variations, etc.