[moves, position] Implement unmaking moves on a board

Declare an UnmakeMove trait in the moves crate, just like the MakeMove trait from
an earlier commit. Implement this trait for all types that also implement
BoardProvider.

Bring in a whole pile of unit tests from Claude. (Holy shit, using Claude really
saves time on these tests…) Several of these tests failed, and all of those
failures revealed bugs in either MakeMove or UnmakeMove. Huzzah! Include fixes for
those bugs here.

moves/src/lib.rs

@@ -8,6 +8,7 @@ mod defs;
 mod make_move;
 mod moves;
 mod record;
+mod unmake_move;
 
 pub use builder::{Builder, Error as BuildMoveError, Result as BuildMoveResult};
 pub use defs::{Kind, PromotionShape};
@@ -15,3 +16,4 @@ pub use generators::GeneratedMove;
 pub use make_move::{MakeMove, MakeMoveError, ValidateMove};
 pub use moves::Move;
 pub use record::MoveRecord;
+pub use unmake_move::{UnmakeMove, UnmakeMoveError};

moves/src/make_move.rs

@@ -61,6 +61,13 @@ pub enum MakeMoveError {
 }
 
 pub trait MakeMove {
+    /// Make a move.
+    ///
+    /// ## Errors
+    ///
+    /// Returns one of [`MakeMoveError`] indicating why the move could not be
+    /// made.
+    ///
     fn make_move(&mut self, ply: Move, validate: ValidateMove) -> MakeMoveResult;
 }
 
@@ -82,8 +89,8 @@ impl<T: BoardProvider> MakeMove for T {
     ///
     /// ## Errors
     ///
-    /// If `validate` is [`ValidateMove::Yes`], perform validation of move correctness prior to
-    /// applying the move. See [`Position::validate_move`].
+    /// If `validate` is [`ValidateMove::Yes`], perform validation of move
+    /// correctness prior to applying the move. See [`Position::validate_move`].
     fn make_move(
         &mut self,
         ply: Move,
@@ -158,14 +165,16 @@ impl<T: BoardProvider> MakeMoveInternal for T {
             .place_piece(piece, target, PlacePieceStrategy::PreserveExisting)
             .map_err(MakeMoveError::PlacePieceError)?;
 
+        // Capture move record before setting the en passant square, to ensure
+        // board state before the change is preserved.
+        let record = MoveRecord::new(board, ply, None);
+
         board.en_passant_target = match target.rank() {
             Rank::FOUR => Some(Square::from_file_rank(target.file(), Rank::THREE)),
             Rank::FIVE => Some(Square::from_file_rank(target.file(), Rank::SIX)),
             _ => unreachable!(),
         };
 
-        let record = MoveRecord::new(board, ply, None);
-
         self.advance_clocks(HalfMoveClock::Advance);
 
         Ok(record)
@@ -227,10 +236,12 @@ impl<T: BoardProvider> MakeMoveInternal for T {
         let rook = board.remove_piece(parameters.origin.rook).unwrap();
         board.place_piece(rook, parameters.target.rook, PlacePieceStrategy::default())?;
 
-        board.castling_rights.revoke(active_color, wing);
-
+        // Capture move record before revoking castling rights to ensure
+        // original board state is preserved.
         let record = MoveRecord::new(board, ply, None);
 
+        board.castling_rights.revoke(active_color, wing);
+
         self.advance_clocks(HalfMoveClock::Advance);
 
         Ok(record)
@@ -426,7 +437,9 @@ mod tests {
         ];
 
         let ply = Move::double_push(Square::E2, Square::E4);
-        board.make_move(ply, ValidateMove::Yes)?;
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        assert_eq!(record.en_passant_target, None);
 
         assert_eq!(board.get_piece(Square::E2), None);
         assert_eq!(board.get_piece(Square::E4), Some(piece!(White Pawn)));

moves/src/unmake_move.rs

@@ -0,0 +1,500 @@
+// Eryn Wells <eryn@erynwells.me>
+
+use crate::MoveRecord;
+use chessfriend_board::{Board, BoardProvider, PlacePieceError, PlacePieceStrategy};
+use chessfriend_core::{Piece, Square};
+use thiserror::Error;
+
+type UnmakeMoveResult = Result<(), UnmakeMoveError>;
+
+#[derive(Debug, Error, Eq, PartialEq)]
+pub enum UnmakeMoveError {
+    #[error("no move to unmake")]
+    NoMove,
+
+    #[error("no piece on {0}")]
+    NoPiece(Square),
+
+    #[error("no capture square")]
+    NoCaptureSquare,
+
+    #[error("no captured piece to unmake capture move")]
+    NoCapturedPiece,
+
+    #[error("{0}")]
+    PlacePieceError(#[from] PlacePieceError),
+}
+
+pub trait UnmakeMove {
+    /// Unmake the given move. Unmaking a move that wasn't the most recent one
+    /// made will likely cause strange results.
+    ///
+    /// ## To-Do
+    ///
+    /// Some implementations I've seen in other engines take a move to unmake. I
+    /// don't understand why they do this because I don't think it makes sense
+    /// to unmake any move other than the last move made. I need to do some
+    /// research on this to understand if/when passing a move might be useful or
+    /// necessary.
+    ///
+    /// ## Errors
+    ///
+    /// Returns one of [`UnmakeMoveError`] indicating why the move cannot be
+    /// unmade.
+    ///
+    fn unmake_move(&mut self, record: MoveRecord) -> UnmakeMoveResult;
+}
+
+trait UnmakeMoveInternal {
+    fn unmake_quiet_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult;
+    fn unmake_capture_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult;
+    fn unmake_promotion_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult;
+    fn unmake_castle_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult;
+}
+
+impl<T: BoardProvider> UnmakeMove for T {
+    fn unmake_move(&mut self, record: MoveRecord) -> UnmakeMoveResult {
+        let ply = record.ply;
+
+        if ply.is_quiet() || ply.is_double_push() {
+            self.unmake_quiet_move(&record)?;
+        } else if ply.is_capture() || ply.is_en_passant() {
+            self.unmake_capture_move(&record)?;
+        } else if ply.is_promotion() {
+            self.unmake_promotion_move(&record)?;
+        } else if ply.is_castle() {
+            self.unmake_castle_move(&record)?;
+        } else {
+            unreachable!();
+        }
+
+        let board = self.board_mut();
+        board.active_color = record.color;
+        board.en_passant_target = record.en_passant_target;
+        board.castling_rights = record.castling_rights;
+        board.half_move_clock = record.half_move_clock;
+
+        Ok(())
+    }
+}
+
+impl<T: BoardProvider> UnmakeMoveInternal for T {
+    fn unmake_quiet_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult {
+        let board = self.board_mut();
+
+        let ply = record.ply;
+
+        let target = ply.target_square();
+        let piece = board
+            .get_piece(target)
+            .ok_or(UnmakeMoveError::NoPiece(target))?;
+
+        let origin = ply.origin_square();
+        board.place_piece(piece, origin, PlacePieceStrategy::PreserveExisting)?;
+
+        board.remove_piece(target);
+
+        Ok(())
+    }
+
+    fn unmake_capture_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult {
+        let board = self.board_mut();
+
+        let ply = record.ply;
+
+        let target = ply.target_square();
+
+        let mut piece = board
+            .get_piece(target)
+            .ok_or(UnmakeMoveError::NoPiece(target))?;
+        if ply.is_promotion() {
+            piece = Piece::pawn(piece.color);
+        }
+
+        let origin = ply.origin_square();
+        board.place_piece(piece, origin, PlacePieceStrategy::PreserveExisting)?;
+
+        let capture_square = ply
+            .capture_square()
+            .ok_or(UnmakeMoveError::NoCaptureSquare)?;
+        let captured_piece = record
+            .captured_piece
+            .ok_or(UnmakeMoveError::NoCapturedPiece)?;
+
+        board.remove_piece(target);
+
+        board.place_piece(
+            captured_piece,
+            capture_square,
+            PlacePieceStrategy::PreserveExisting,
+        )?;
+
+        Ok(())
+    }
+
+    fn unmake_promotion_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult {
+        let board = self.board_mut();
+
+        let ply = record.ply;
+
+        let target = ply.target_square();
+
+        let piece = Piece::pawn(
+            board
+                .get_piece(target)
+                .ok_or(UnmakeMoveError::NoPiece(target))?
+                .color,
+        );
+
+        let origin = ply.origin_square();
+        board.place_piece(piece, origin, PlacePieceStrategy::PreserveExisting)?;
+
+        board.remove_piece(target);
+
+        Ok(())
+    }
+
+    fn unmake_castle_move(&mut self, record: &MoveRecord) -> UnmakeMoveResult {
+        let ply = record.ply;
+
+        let wing = ply.castle_wing().expect("no wing for unmaking castle move");
+        let color = record.color;
+        let parameters = Board::castling_parameters(wing, color);
+
+        let board = self.board_mut();
+
+        let king = board
+            .get_piece(parameters.target.king)
+            .ok_or(UnmakeMoveError::NoPiece(parameters.target.king))?;
+        let rook = board
+            .get_piece(parameters.target.rook)
+            .ok_or(UnmakeMoveError::NoPiece(parameters.target.rook))?;
+
+        board.place_piece(
+            king,
+            parameters.origin.king,
+            PlacePieceStrategy::PreserveExisting,
+        )?;
+        board.place_piece(
+            rook,
+            parameters.origin.rook,
+            PlacePieceStrategy::PreserveExisting,
+        )?;
+
+        board.remove_piece(parameters.target.king);
+        board.remove_piece(parameters.target.rook);
+
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{MakeMove, Move, PromotionShape, ValidateMove};
+    use chessfriend_board::test_board;
+    use chessfriend_core::{piece, Color, Square, Wing};
+
+    type TestResult = Result<(), Box<dyn std::error::Error>>;
+
+    /// Helper function to test make/unmake idempotency
+    fn test_make_unmake_idempotent(
+        initial_board: &mut impl BoardProvider,
+        ply: Move,
+    ) -> TestResult {
+        // Capture initial state
+        let initial_state = initial_board.board().clone();
+
+        // Make the move
+        let record = initial_board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify the move changed the board
+        assert_ne!(
+            *initial_board.board(),
+            initial_state,
+            "Move should change board state"
+        );
+
+        // Unmake the move
+        initial_board.unmake_move(record)?;
+
+        // Verify we're back to the initial state
+        assert_eq!(
+            *initial_board.board(),
+            initial_state,
+            "Board should return to initial state after unmake"
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_quiet_move_ai_claude() -> TestResult {
+        let mut board = test_board!(White Pawn on C2);
+
+        let ply = Move::quiet(Square::C2, Square::C3);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify move was made
+        assert_eq!(board.get_piece(Square::C2), None);
+        assert_eq!(board.get_piece(Square::C3), Some(piece!(White Pawn)));
+        assert_eq!(board.active_color, Color::Black);
+
+        // Unmake the move
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::C2), Some(piece!(White Pawn)));
+        assert_eq!(board.get_piece(Square::C3), None);
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_double_push_move_ai_claude() -> TestResult {
+        let mut board = test_board!(White Pawn on E2);
+
+        let ply = Move::double_push(Square::E2, Square::E4);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify move was made
+        assert_eq!(board.get_piece(Square::E2), None);
+        assert_eq!(board.get_piece(Square::E4), Some(piece!(White Pawn)));
+        assert_eq!(board.en_passant_target, Some(Square::E3));
+        assert_eq!(board.active_color, Color::Black);
+
+        // Unmake the move
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::E2), Some(piece!(White Pawn)));
+        assert_eq!(board.get_piece(Square::E4), None);
+        assert_eq!(board.en_passant_target, None);
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_capture_move_ai_claude() -> TestResult {
+        let mut board = test_board![
+            White Bishop on C2,
+            Black Rook on F5,
+        ];
+
+        let ply = Move::capture(Square::C2, Square::F5);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify move was made
+        assert_eq!(board.get_piece(Square::C2), None);
+        assert_eq!(board.get_piece(Square::F5), Some(piece!(White Bishop)));
+        assert_eq!(record.captured_piece, Some(piece!(Black Rook)));
+        assert_eq!(board.active_color, Color::Black);
+
+        // Unmake the move
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::C2), Some(piece!(White Bishop)));
+        assert_eq!(board.get_piece(Square::F5), Some(piece!(Black Rook)));
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_en_passant_capture_ai_claude() -> TestResult {
+        let mut board = test_board![
+            Black Pawn on F4,
+            White Pawn on E2
+        ];
+
+        // Set up en passant situation
+        let double_push = Move::double_push(Square::E2, Square::E4);
+        board.make_move(double_push, ValidateMove::Yes)?;
+
+        // Make en passant capture
+        let en_passant = Move::en_passant_capture(Square::F4, Square::E3);
+        let record = board.make_move(en_passant, ValidateMove::Yes)?;
+
+        // Verify en passant was made
+        assert_eq!(board.get_piece(Square::F4), None);
+        assert_eq!(board.get_piece(Square::E3), Some(piece!(Black Pawn)));
+        assert_eq!(
+            board.get_piece(Square::E4),
+            None,
+            "captured pawn was not removed"
+        );
+        assert_eq!(record.captured_piece, Some(piece!(White Pawn)));
+
+        // Unmake the en passant capture
+        board.unmake_move(record)?;
+
+        // Verify state before en passant is restored
+        assert_eq!(board.get_piece(Square::F4), Some(piece!(Black Pawn)));
+        assert_eq!(board.get_piece(Square::E3), None);
+        assert_eq!(
+            board.get_piece(Square::E4),
+            Some(piece!(White Pawn)),
+            "captured pawn was not restored"
+        );
+        assert_eq!(board.active_color, Color::Black);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_promotion_move_ai_claude() -> TestResult {
+        let mut board = test_board![
+            White Pawn on F7,
+        ];
+
+        let ply = Move::promotion(Square::F7, Square::F8, PromotionShape::Queen);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify promotion was made
+        assert_eq!(board.get_piece(Square::F7), None);
+        assert_eq!(board.get_piece(Square::F8), Some(piece!(White Queen)));
+        assert_eq!(board.active_color, Color::Black);
+
+        // Unmake the promotion
+        board.unmake_move(record)?;
+
+        // Verify original pawn is restored
+        assert_eq!(board.get_piece(Square::F7), Some(piece!(White Pawn)));
+        assert_eq!(board.get_piece(Square::F8), None);
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_capture_promotion_ai_claude() -> TestResult {
+        let mut board = test_board![
+            White Pawn on F7,
+            Black Rook on G8,
+        ];
+
+        let ply = Move::capture_promotion(Square::F7, Square::G8, PromotionShape::Queen);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify promotion capture was made
+        assert_eq!(board.get_piece(Square::F7), None);
+        assert_eq!(board.get_piece(Square::G8), Some(piece!(White Queen)));
+        assert_eq!(record.captured_piece, Some(piece!(Black Rook)));
+
+        // Unmake the promotion capture
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::F7), Some(piece!(White Pawn)));
+        assert_eq!(board.get_piece(Square::G8), Some(piece!(Black Rook)));
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_white_kingside_castle_ai_claude() -> TestResult {
+        let mut board = test_board![
+            White King on E1,
+            White Rook on H1,
+        ];
+
+        let original_castling_rights = board.castling_rights;
+
+        let ply = Move::castle(Wing::KingSide);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify castle was made
+        assert_eq!(board.get_piece(Square::E1), None);
+        assert_eq!(board.get_piece(Square::H1), None);
+        assert_eq!(board.get_piece(Square::G1), Some(piece!(White King)));
+        assert_eq!(board.get_piece(Square::F1), Some(piece!(White Rook)));
+        assert!(!board
+            .castling_rights
+            .color_has_right(Color::White, Wing::KingSide));
+
+        // Unmake the castle
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::E1), Some(piece!(White King)));
+        assert_eq!(board.get_piece(Square::H1), Some(piece!(White Rook)));
+        assert_eq!(board.get_piece(Square::G1), None);
+        assert_eq!(board.get_piece(Square::F1), None);
+        assert_eq!(board.castling_rights, original_castling_rights);
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_white_queenside_castle_ai_claude() -> TestResult {
+        let mut board = test_board![
+            White King on E1,
+            White Rook on A1,
+        ];
+
+        let original_castling_rights = board.castling_rights;
+
+        let ply = Move::castle(Wing::QueenSide);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify castle was made
+        assert_eq!(board.get_piece(Square::E1), None);
+        assert_eq!(board.get_piece(Square::A1), None);
+        assert_eq!(board.get_piece(Square::C1), Some(piece!(White King)));
+        assert_eq!(board.get_piece(Square::D1), Some(piece!(White Rook)));
+        assert!(!board
+            .castling_rights
+            .color_has_right(Color::White, Wing::QueenSide));
+
+        // Unmake the castle
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::E1), Some(piece!(White King)));
+        assert_eq!(board.get_piece(Square::A1), Some(piece!(White Rook)));
+        assert_eq!(board.get_piece(Square::C1), None);
+        assert_eq!(board.get_piece(Square::D1), None);
+        assert_eq!(board.castling_rights, original_castling_rights);
+        assert_eq!(board.active_color, Color::White);
+
+        Ok(())
+    }
+
+    #[test]
+    fn unmake_black_kingside_castle() -> TestResult {
+        let mut board = test_board![
+            Black King on E8,
+            Black Rook on H8,
+        ];
+        board.active_color = Color::Black;
+
+        let original_castling_rights = board.castling_rights;
+
+        let ply = Move::castle(Wing::KingSide);
+        let record = board.make_move(ply, ValidateMove::Yes)?;
+
+        // Verify castle was made
+        assert_eq!(board.get_piece(Square::E8), None);
+        assert_eq!(board.get_piece(Square::H8), None);
+        assert_eq!(board.get_piece(Square::G8), Some(piece!(Black King)));
+        assert_eq!(board.get_piece(Square::F8), Some(piece!(Black Rook)));
+
+        // Unmake the castle
+        board.unmake_move(record)?;
+
+        // Verify original state restored
+        assert_eq!(board.get_piece(Square::E8), Some(piece!(Black King)));
+        assert_eq!(board.get_piece(Square::H8), Some(piece!(Black Rook)));
+        assert_eq!(board.get_piece(Square::G8), None);
+        assert_eq!(board.get_piece(Square::F8), None);
+        assert_eq!(board.castling_rights, original_castling_rights);
+        assert_eq!(board.active_color, Color::Black);
+
+        Ok(())
+    }
+}

position/src/position/unmake_move.rs

@@ -1,3 +0,0 @@
-// Eryn Wells <eryn@erynwells.me>
-
-pub enum UnmakeMoveError {}