bhlangonijr / chesslib

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

MoveBackup sets moving piece for promotion move not as pawn #34

Closed dlbbld closed 3 years ago

dlbbld commented 3 years ago

For a promotion move the moving piece is set as the promoted piece in the MoveBackup. This is used as such in the code. If that must be so, I suggest using another name instead of "moving piece". The moving piece for a promotion move is a pawn, so setting the moving piece to the promoted piece is semantically wrong for my understanding. I ran into a small bug for expecting this. Example:

final Board board = new Board();
board.loadFromFen("8/P3k3/8/8/8/8/4K3/8 w - - 0 1");

final Move whitePawnPromotion = new Move("a7a8Q", Side.WHITE);
board.doMove(whitePawnPromotion);
final MoveBackup moveBackup = board.getBackup().getLast();
System.out.println("moveBackup.movingPiece: Expected=WHITE_PAWN, Actual=" + moveBackup.getMovingPiece());

The output is as below, with latest version 1.1.22, which is not as expected for me:

moveBackup.movingPiece: Expected=WHITE_PAWN, Actual=WHITE_QUEEN
bhlangonijr commented 3 years ago

Hello @dlbbld , Yes I agree with you the name may lead to some confusion. The reason for it being like that is to make move undoing (MoveBackup#restore) consistent with other non promotion moves handling: This field is used for unsetting the moved piece when undoing the move. I'll fix this in the next release.

bhlangonijr commented 3 years ago

Fixed in version 1.1.23

dlbbld commented 3 years ago

Thanks, looks fine.