bhlangonijr / chesslib

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

King castle moves like e1g1 by rook or queen flagged as castle move #32

Closed dlbbld closed 3 years ago

dlbbld commented 3 years ago

Hi

First thanks for this cool API.

When performing the move e1g1 by a rook or queen the move is interpreted as castle move. That is board.getContext().isCastleMove(move) is true. Which I think should not be the case. If intended it is misleading, as not being a castle move.

But then, as a consequence, in the MoveBackup the rook castle move is set, which is wrong, as this should only be set for castling moves. From the code it looks like board.getContext().isCastleMove(move) is missing a check for the moved piece being a king.

final Board board = new Board();
board.loadFromFen("8/5k2/8/8/8/8/5K2/4R3 w - - 0 1");

final Move whiteRookMoveE1G1 = new Move("e1g1", Side.WHITE);
final boolean gameContextCastleMove = board.getContext().isCastleMove(whiteRookMoveE1G1);
System.out.println("gameContext.isCastleMove: Expected=false, Actual=" + gameContextCastleMove);
board.doMove(whiteRookMoveE1G1);
final MoveBackup moveBackup = board.getBackup().getLast();
System.out.println("moveBackup.isCastleMove: Expected=false, Actual=" + moveBackup.isCastleMove());
System.out.println("moveBackup.rookCastleMove: Expected=null, Actual=" + moveBackup.getRookCastleMove());

The output is now with latest version 1.1.22 as below, so not as expected:

gameContext.isCastleMove: Expected=false, Actual=true
moveBackup.isCastleMove: Expected=false, Actual=true
moveBackup.rookCastleMove: Expected=null, Actual=h1f1

The same is for the other king castling moves e1c1, e8g8 and e8c8. Can you please have a look. Thanks.

bhlangonijr commented 3 years ago

Fixed in version 1.1.23

dlbbld commented 3 years ago

Thanks, looks fine now. I have a question related to castling for GameContext, I'll add it to a new issue.