fix: fixed bug causing Zobrist keys to not update properly when makin…

…g moves

README.md

@@ -98,6 +98,8 @@ Special thanks in particular to:
 
 ## Changelog
 
+-   `1.2.1`:
+    -   Fixed major bug causing Zobrist keys to not update properly when making moves on positions (thanks @Serdra on the EP discord).
 -   `1.2.0`:
     -   Added `Position::can_draw_by_insufficient_material`.
     -   Fixed bug causing `Square::is_light` to return the opposite bool.

chessie/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "chessie"
-version = "1.2.0"
+version = "1.2.1"
 edition = "2021"
 build = "build.rs"
 authors = ["Danny Hammer <[email protected]>"]
@@ -13,10 +13,10 @@ keywords = ["chess"]
 [dependencies]
 anyhow = "1.0.86"
 arrayvec = "0.7.4"
-chessie_types = { path = "../chessie_types", version = "1.2.0" }
+chessie_types = { path = "../chessie_types", version = "1.2.1" }
 
 [build-dependencies]
-chessie_types = { path = "../chessie_types", version = "1.2.0" }
+chessie_types = { path = "../chessie_types", version = "1.2.1" }
 
 [dev-dependencies]
 criterion = "0.5.1"

chessie/README.md

@@ -98,13 +98,22 @@ Special thanks in particular to:
 
 ## Changelog
 
+-   `1.2.1`:
+    -   Fixed major bug causing Zobrist keys to not update properly when making moves on positions (thanks @Serdra on the EP discord).
+-   `1.2.0`:
+    -   Added `Position::can_draw_by_insufficient_material`.
+    -   Fixed bug causing `Square::is_light` to return the opposite bool.
+-   `1.1.0`:
+    -   Fixed bug causing illegal positions to crash move generator
+    -   Implemented `FromIterator<Square>` for `Bitboard`.
+    -   Added function for playing null moves, and adjusted how `Game` is printed.
 -   `1.0.0`:
-    -   Massive performance increase (over 200%) in move generation ([benchmarks](https://github.com/dannyhammer/chessie-benchmarks))
-    -   `Game::get_legal_moves` now computes legal moves in bulk rather than relying on `MoveGenIter::collect`
-    -   Some breaking changes with method names in `Bitboard` and other primitives
-    -   General code cleanup (more docs, more tests, etc.)
+    -   Massive performance increase (over 200%) in move generation ([benchmarks](https://github.com/dannyhammer/chessie-benchmarks)).
+    -   `Game::get_legal_moves` now computes legal moves in bulk rather than relying on `MoveGenIter::collect`.
+    -   Some breaking changes with method names in `Bitboard` and other primitives.
+    -   General code cleanup (more docs, more tests, etc.).
     -   Added [`examples/`](./chessie/examples/) and [`benches`](./chessie/benches/).
-    -   Moved `prng.rs` into `chessie-types` so it can be used for magic generation, removing our dependency on `rand`
+    -   Moved `prng.rs` into `chessie-types` so it can be used for magic generation, removing our dependency on `rand`.
     -   Added `#[inline(always)]` to a _lot_ of functions/methods. Seems to have improved efficiency.
     -   Modified `CastlingRights` and it's storage in `Position`. Should make Chess960 integration easier later.
 -   `0.1.0`:

chessie/src/position.rs

@@ -288,7 +288,9 @@ impl Position {
 
     /// Fetch the Zobrist hash key of this position.
     #[inline(always)]
-    pub const fn key(&self) -> ZobristKey {
+    pub fn key(&self) -> ZobristKey {
+        assert_eq!(self.key, ZobristKey::new(&self));
+
         self.key
     }
 
@@ -477,16 +479,16 @@ impl Position {
     /// Applies the move. No enforcement of legality
     pub fn make_move(&mut self, mv: Move) {
         // Remove the piece from it's previous location, exiting early if there is no piece there
-        let Some(mut piece) = self.board_mut().take(mv.from()) else {
+        let Some(mut piece) = self.take(mv.from()) else {
             return;
         };
 
         let color = piece.color();
         let to = mv.to();
         let from = mv.from();
 
-        // Un-hash the piece at `from`.
-        self.key.hash_piece(from, piece);
+        // Un-hash the side-to-move
+        self.key.hash_side_to_move(self.side_to_move());
 
         // Clear the EP square from the last move (and un-hash it)
         if let Some(ep_square) = self.ep_square.take() {
@@ -506,7 +508,7 @@ impl Position {
                 to
             };
 
-            let Some(captured) = self.board_mut().take(captured_square) else {
+            let Some(captured) = self.take(captured_square) else {
                 panic!("Failed to apply {mv:?} to {self}: No piece found at {captured_square}");
             };
             let captured_color = captured.color();
@@ -540,8 +542,8 @@ impl Position {
             let new_rook_square = [Square::D1, Square::F1][castle_index].rank_relative_to(color);
 
             // Move the rook. The King is already handled before and after this match statement.
-            let rook = self.board_mut().take(old_rook_square).unwrap();
-            self.board_mut().place(rook, new_rook_square);
+            let rook = self.take(old_rook_square).unwrap();
+            self.place(rook, new_rook_square);
 
             // Disable castling
             self.clear_castling_rights(color);
@@ -579,10 +581,7 @@ impl Position {
         }
 
         // Place the piece in it's new position
-        self.board_mut().place(piece, to);
-
-        // Hash the piece at `to`.
-        self.key.hash_piece(to, piece);
+        self.place(piece, to);
 
         // Next player's turn
         self.toggle_side_to_move();
@@ -591,6 +590,21 @@ impl Position {
         self.key.hash_side_to_move(self.side_to_move());
     }
 
+    /// Places a piece at the provided square, updating Zobrist hash information.
+    #[inline(always)]
+    fn place(&mut self, piece: Piece, square: Square) {
+        self.board_mut().place(piece, square);
+        self.key.hash_piece(square, piece);
+    }
+
+    /// Removes and returns a piece on the provided square, updating Zobrist hash information.
+    #[inline(always)]
+    fn take(&mut self, square: Square) -> Option<Piece> {
+        let piece = self.board_mut().take(square)?;
+        self.key.hash_piece(square, piece);
+        Some(piece)
+    }
+
     /// Clears the castling rights of `color`
     #[inline(always)]
     fn clear_castling_rights(&mut self, color: Color) {
@@ -1383,12 +1397,20 @@ mod tests {
 
         pos.make_move(Move::from_uci(&pos, "b1a3").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
+
         pos.make_move(Move::from_uci(&pos, "b8a6").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
+
         pos.make_move(Move::from_uci(&pos, "a3b1").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
+
+        // After returning to the original position, they keys should be equal again
         pos.make_move(Move::from_uci(&pos, "a6b8").unwrap());
         assert_eq!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
     }
 
     // There are four cases in which castling rights can be lost:
@@ -1430,6 +1452,7 @@ mod tests {
         // Same for Black
         pos.make_move(Move::from_uci(&pos, "f8e8").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
         assert_ne!(pos.castling_rights(), &original_rights);
         assert_eq!(pos.castling_rights_uci(), "-");
     }
@@ -1482,12 +1505,14 @@ mod tests {
         // Capturing a Rook should disable castling on that side for the side that lost the Rook
         pos.make_move(Move::from_uci(&pos, "a1a8").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
         assert_ne!(pos.castling_rights(), &original_rights);
         assert_eq!(pos.castling_rights_uci(), "Kk"); // White used it's H1 Rook to capture, so they lose their rights on that side as well
 
         // Same for Black, on the other side
         pos.make_move(Move::from_uci(&pos, "h8h1").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
         assert_ne!(pos.castling_rights(), &original_rights);
         assert_eq!(pos.castling_rights_uci(), "-");
     }
@@ -1505,12 +1530,14 @@ mod tests {
         // Performing castling should remove that side's rights altogether
         pos.make_move(Move::from_uci(&pos, "e1g1").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
         assert_ne!(pos.castling_rights(), &original_rights);
         assert_eq!(pos.castling_rights_uci(), "kq");
 
         // Same for Black, on the other side
         pos.make_move(Move::from_uci(&pos, "e8c8").unwrap());
         assert_ne!(pos.key(), original_key);
+        assert_eq!(pos.key(), ZobristKey::new(&pos));
         assert_ne!(pos.castling_rights(), &original_rights);
         assert_eq!(pos.castling_rights_uci(), "-");
     }

chessie_types/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "chessie_types"
-version = "1.2.0"
+version = "1.2.1"
 edition = "2021"
 authors = ["Danny Hammer <[email protected]>"]
 license = "MPL-2.0"