Simplify 'Move'

Making 'Move' lightweight sounds like a better idea now that I am
looking at it with fresh eyes...

src/board/chess_board/mod.rs

@@ -136,21 +136,32 @@ impl ChessBoard {
     #[inline(always)]
     pub fn do_move(&mut self, chess_move: Move) -> NonReversibleState {
         let opponent = !self.current_player();
+        let is_capture = !(self.combined_occupancy() & chess_move.destination()).is_empty();
+        let move_piece = Piece::iter()
+            .find(|&p| !(self.piece_occupancy(p) & chess_move.start()).is_empty())
+            .unwrap();
+        let captured_piece = Piece::iter()
+            .skip(1) // No need to check for the king here
+            .find(|&p| !(self.occupancy(p, opponent) & chess_move.destination()).is_empty());
+        let is_double_step = move_piece == Piece::Pawn
+            && chess_move.start().rank() == self.current_player().second_rank()
+            && chess_move.destination().rank() == self.current_player().fourth_rank();
+
         // Save non-revertible state
         let state = NonReversibleState {
             castle_rights: self.castle_rights,
             en_passant: self.en_passant,
             half_move_clock: self.half_move_clock,
-            captured_piece: chess_move.capture(),
+            captured_piece,
         };
 
         // Non-revertible state modification
-        if chess_move.capture().is_some() || chess_move.piece() == Piece::Pawn {
+        if is_capture || move_piece == Piece::Pawn {
             self.half_move_clock = 0;
         } else {
             self.half_move_clock += 1;
         }
-        if chess_move.is_double_step() {
+        if is_double_step {
             let target_square = Square::new(
                 chess_move.destination().file(),
                 self.current_player().third_rank(),
@@ -159,10 +170,10 @@ impl ChessBoard {
         } else {
             self.en_passant = None;
         }
-        if chess_move.is_castling() || chess_move.piece() == Piece::King {
+        if move_piece == Piece::King {
             *self.castle_rights_mut(self.current_player()) = CastleRights::NoSide;
         }
-        if chess_move.piece() == Piece::Rook {
+        if move_piece == Piece::Rook {
             let castle_rights = self.castle_rights_mut(self.current_player());
             *castle_rights = match chess_move.start().file() {
                 File::A => castle_rights.without_queen_side(),
@@ -170,7 +181,7 @@ impl ChessBoard {
                 _ => *castle_rights,
             }
         }
-        if let Some(piece) = chess_move.capture() {
+        if let Some(piece) = captured_piece {
             *self.piece_occupancy_mut(piece) ^= chess_move.destination();
             *self.color_occupancy_mut(opponent) ^= chess_move.destination();
             self.combined_occupancy ^= chess_move.destination();
@@ -179,7 +190,7 @@ impl ChessBoard {
         // Revertible state modification
         self.xor(
             self.current_player(),
-            chess_move.piece(),
+            move_piece,
             chess_move.start() | chess_move.destination(),
         );
         self.total_plies += 1;
@@ -196,8 +207,15 @@ impl ChessBoard {
         self.castle_rights = previous.castle_rights;
         self.en_passant = previous.en_passant;
         self.half_move_clock = previous.half_move_clock;
+
+        let move_piece = Piece::iter()
+            // We're looking for the *destination* as this is *undoing* the move
+            .find(|&p| !(self.piece_occupancy(p) & chess_move.destination()).is_empty())
+            .unwrap();
+
         if let Some(piece) = previous.captured_piece {
             *self.piece_occupancy_mut(piece) ^= chess_move.destination();
+            // The capture affected the *current* player, from our post-move POV
             *self.color_occupancy_mut(self.current_player()) ^= chess_move.destination();
             self.combined_occupancy ^= chess_move.destination();
         }
@@ -206,7 +224,7 @@ impl ChessBoard {
         self.xor(
             // The move was applied at the turn *before* the current player
             !self.current_player(),
-            chess_move.piece(),
+            move_piece,
             chess_move.start() | chess_move.destination(),
         );
         self.total_plies -= 1;
@@ -435,7 +453,6 @@ impl Default for ChessBoard {
 
 #[cfg(test)]
 mod test {
-    use crate::board::MoveBuilder;
     use crate::fen::FromFen;
 
     use super::*;
@@ -729,57 +746,21 @@ mod test {
         // Start from default position
         let mut position = ChessBoard::default();
         // Modify it to account for e4 move
-        position.do_move(
+        position.do_move(Move::new(Square::E2, Square::E4, None));
-            MoveBuilder {
-                piece: Piece::Pawn,
-                start: Square::E2,
-                destination: Square::E4,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: true,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             position,
             ChessBoard::from_fen("rnbqkbnr/pppppppp/8/8/4P3/8/PPPP1PPP/RNBQKBNR b KQkq e3 0 1")
                 .unwrap()
         );
         // And now c5
-        position.do_move(
+        position.do_move(Move::new(Square::C7, Square::C5, None));
-            MoveBuilder {
-                piece: Piece::Pawn,
-                start: Square::C7,
-                destination: Square::C5,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: true,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             position,
             ChessBoard::from_fen("rnbqkbnr/pp1ppppp/8/2p5/4P3/8/PPPP1PPP/RNBQKBNR w KQkq c6 0 2")
                 .unwrap()
         );
         // Finally, Nf3
-        position.do_move(
+        position.do_move(Move::new(Square::G1, Square::F3, None));
-            MoveBuilder {
-                piece: Piece::Knight,
-                start: Square::G1,
-                destination: Square::F3,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: false,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             position,
             ChessBoard::from_fen("rnbqkbnr/pp1ppppp/8/2p5/4P3/5N2/PPPP1PPP/RNBQKB1R b KQkq - 1 2 ")
@@ -792,43 +773,13 @@ mod test {
         // Start from default position
         let mut position = ChessBoard::default();
         // Modify it to account for e4 move
-        let move_1 = MoveBuilder {
+        let move_1 = Move::new(Square::E2, Square::E4, None);
-            piece: Piece::Pawn,
-            start: Square::E2,
-            destination: Square::E4,
-            capture: None,
-            promotion: None,
-            en_passant: false,
-            double_step: true,
-            castling: false,
-        }
-        .into();
         let state_1 = position.do_move(move_1);
         // And now c5
-        let move_2 = MoveBuilder {
+        let move_2 = Move::new(Square::C7, Square::C5, None);
-            piece: Piece::Pawn,
-            start: Square::C7,
-            destination: Square::C5,
-            capture: None,
-            promotion: None,
-            en_passant: false,
-            double_step: true,
-            castling: false,
-        }
-        .into();
         let state_2 = position.do_move(move_2);
         // Finally, Nf3
-        let move_3 = MoveBuilder {
+        let move_3 = Move::new(Square::G1, Square::F3, None);
-            piece: Piece::Knight,
-            start: Square::G1,
-            destination: Square::F3,
-            capture: None,
-            promotion: None,
-            en_passant: false,
-            double_step: false,
-            castling: false,
-        }
-        .into();
         let state_3 = position.do_move(move_3);
         // Now revert each move one-by-one
         position.undo_move(move_3, state_3);
@@ -857,17 +808,7 @@ mod test {
         let expected = ChessBoard::from_fen("3Q3k/8/8/8/8/8/8/K7 b - - 0 1").unwrap();
         let original = position.clone();
 
-        let capture = MoveBuilder {
+        let capture = Move::new(Square::D1, Square::D8, None);
-            piece: Piece::Queen,
-            start: Square::D1,
-            destination: Square::D8,
-            capture: Some(Piece::Queen),
-            promotion: None,
-            en_passant: false,
-            double_step: false,
-            castling: false,
-        }
-        .into();
 
         let state = position.do_move(capture);
         assert_eq!(position, expected);

src/board/move.rs

@@ -1,232 +1,42 @@
 use super::{Piece, Square};
 
-type Bitset = u32;
-
 /// A chess move, containing:
-/// * Piece type.
 /// * Starting square.
 /// * Destination square.
-/// * Optional capture type.
 /// * Optional promotion type.
-/// * Optional captured type.
-/// * Whether the move was an en-passant capture.
-/// * Whether the move was a double-step.
-/// * Whether the move was a castling.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct Move(Bitset);
+pub struct Move {
-
+    start: Square,
-/// A builder for [Move]. This is the prefered and only way of building a [Move].
+    destination: Square,
-pub struct MoveBuilder {
+    promotion: Option<Piece>,
-    pub piece: Piece,
-    pub start: Square,
-    pub destination: Square,
-    pub capture: Option<Piece>,
-    pub promotion: Option<Piece>,
-    pub en_passant: bool,
-    pub double_step: bool,
-    pub castling: bool,
-}
-
-impl From<MoveBuilder> for Move {
-    #[inline(always)]
-    fn from(builder: MoveBuilder) -> Self {
-        Self::new(
-            builder.piece,
-            builder.start,
-            builder.destination,
-            builder.capture,
-            builder.promotion,
-            builder.en_passant,
-            builder.double_step,
-            builder.castling,
-        )
-    }
-}
-
-/// A [Move] is structured as a bitset with the following fields:
-/// | Field       | Size | Range of values | Note                                            |
-/// |-------------|------|-----------------|-------------------------------------------------|
-/// | Piece       | 3    | 0-6             | Can be interpreted as a [Piece] index           |
-/// | Start       | 6    | 0-63            | Can be interpreted as a [Square] index          |
-/// | Destination | 6    | 0-63            | Can be interpreted as a [Square] index          |
-/// | Capture     | 3    | 0-7             | Can be interpreted as a [Piece] index if not 7  |
-/// | Promotion   | 3    | 0-7             | Can be interpreted as a [Piece] index if not 7  |
-/// | En-pasant   | 1    | 0-1             | Boolean value                                   |
-/// | Double-step | 1    | 0-1             | Boolean value                                   |
-/// | Castling    | 1    | 0-1             | Boolean value                                   |
-mod shift {
-    use super::Bitset;
-
-    pub const PIECE: usize = 0;
-    pub const PIECE_MASK: Bitset = 0b111;
-
-    pub const START: usize = 3;
-    pub const START_MASK: Bitset = 0b11_1111;
-
-    pub const DESTINATION: usize = 9;
-    pub const DESTINATION_MASK: Bitset = 0b11_1111;
-
-    pub const CAPTURE: usize = 15;
-    pub const CAPTURE_MASK: Bitset = 0b111;
-
-    pub const PROMOTION: usize = 18;
-    pub const PROMOTION_MASK: Bitset = 0b111;
-
-    pub const EN_PASSANT: usize = 21;
-    pub const EN_PASSANT_MASK: Bitset = 0b1;
-
-    pub const DOUBLE_STEP: usize = 22;
-    pub const DOUBLE_STEP_MASK: Bitset = 0b1;
-
-    pub const CASTLING: usize = 23;
-    pub const CASTLING_MASK: Bitset = 0b1;
 }
 
 impl Move {
     /// Construct a new move.
     #[inline(always)]
-    #[allow(clippy::too_many_arguments)]
+    pub fn new(start: Square, destination: Square, promotion: Option<Piece>) -> Self {
-    fn new(
+        Self {
-        piece: Piece,
+            start,
-        start: Square,
+            destination,
-        destination: Square,
+            promotion,
-        capture: Option<Piece>,
+        }
-        promotion: Option<Piece>,
-        en_passant: bool,
-        double_step: bool,
-        castling: bool,
-    ) -> Self {
-        let mut value = 0;
-        value |= (piece.index() as Bitset) << shift::PIECE;
-        value |= (start.index() as Bitset) << shift::START;
-        value |= (destination.index() as Bitset) << shift::DESTINATION;
-        value |=
-            (capture.map(Piece::index).unwrap_or(Piece::NUM_VARIANTS) as Bitset) << shift::CAPTURE;
-        value |= (promotion.map(Piece::index).unwrap_or(Piece::NUM_VARIANTS) as Bitset)
-            << shift::PROMOTION;
-        value |= (en_passant as Bitset) << shift::EN_PASSANT;
-        value |= (double_step as Bitset) << shift::DOUBLE_STEP;
-        value |= (castling as Bitset) << shift::CASTLING;
-        Self(value)
-    }
-
-    /// Get the [Piece] that is being moved.
-    #[inline(always)]
-    pub fn piece(self) -> Piece {
-        let index = ((self.0 >> shift::PIECE) & shift::PIECE_MASK) as usize;
-        // SAFETY: we know the value is in-bounds
-        unsafe { Piece::from_index_unchecked(index) }
     }
 
     /// Get the [Square] that this move starts from.
     #[inline(always)]
     pub fn start(self) -> Square {
-        let index = ((self.0 >> shift::START) & shift::START_MASK) as usize;
+        self.start
-        // SAFETY: we know the value is in-bounds
-        unsafe { Square::from_index_unchecked(index) }
     }
 
     /// Get the [Square] that this move ends on.
     #[inline(always)]
     pub fn destination(self) -> Square {
-        let index = ((self.0 >> shift::DESTINATION) & shift::DESTINATION_MASK) as usize;
+        self.destination
-        // SAFETY: we know the value is in-bounds
-        unsafe { Square::from_index_unchecked(index) }
-    }
-
-    /// Get the [Piece] that this move captures, or `None` if there are no captures.
-    #[inline(always)]
-    pub fn capture(self) -> Option<Piece> {
-        let index = ((self.0 >> shift::CAPTURE) & shift::CAPTURE_MASK) as usize;
-        if index < Piece::NUM_VARIANTS {
-            // SAFETY: we know the value is in-bounds
-            unsafe { Some(Piece::from_index_unchecked(index)) }
-        } else {
-            None
-        }
     }
 
     /// Get the [Piece] that this move promotes to, or `None` if there are no promotions.
     #[inline(always)]
     pub fn promotion(self) -> Option<Piece> {
-        let index = ((self.0 >> shift::PROMOTION) & shift::PROMOTION_MASK) as usize;
+        self.promotion
-        if index < Piece::NUM_VARIANTS {
-            // SAFETY: we know the value is in-bounds
-            unsafe { Some(Piece::from_index_unchecked(index)) }
-        } else {
-            None
-        }
-    }
-
-    /// Get the whether or not the move is an en-passant capture.
-    #[inline(always)]
-    pub fn is_en_passant(self) -> bool {
-        let index = (self.0 >> shift::EN_PASSANT) & shift::EN_PASSANT_MASK;
-        index != 0
-    }
-
-    /// Get the whether or not the move is a pawn double step.
-    #[inline(always)]
-    pub fn is_double_step(self) -> bool {
-        let index = (self.0 >> shift::DOUBLE_STEP) & shift::DOUBLE_STEP_MASK;
-        index != 0
-    }
-
-    /// Get the whether or not the move is a castling.
-    #[inline(always)]
-    pub fn is_castling(self) -> bool {
-        let index = (self.0 >> shift::CASTLING) & shift::CASTLING_MASK;
-        index != 0
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    #[test]
-    fn builder_simple() {
-        let chess_move: Move = MoveBuilder {
-            piece: Piece::Queen,
-            start: Square::A2,
-            destination: Square::A3,
-            capture: None,
-            promotion: None,
-            en_passant: false,
-            double_step: false,
-            castling: false,
-        }
-        .into();
-        assert_eq!(chess_move.piece(), Piece::Queen);
-        assert_eq!(chess_move.start(), Square::A2);
-        assert_eq!(chess_move.destination(), Square::A3);
-        assert_eq!(chess_move.capture(), None);
-        assert_eq!(chess_move.promotion(), None);
-        assert!(!chess_move.is_en_passant());
-        assert!(!chess_move.is_double_step());
-        assert!(!chess_move.is_castling());
-    }
-
-    #[test]
-    fn builder_all_fields() {
-        let chess_move: Move = MoveBuilder {
-            piece: Piece::Pawn,
-            start: Square::A7,
-            destination: Square::B8,
-            capture: Some(Piece::Queen),
-            promotion: Some(Piece::Knight),
-            en_passant: true,
-            double_step: true,
-            castling: true,
-        }
-        .into();
-        assert_eq!(chess_move.piece(), Piece::Pawn);
-        assert_eq!(chess_move.start(), Square::A7);
-        assert_eq!(chess_move.destination(), Square::B8);
-        assert_eq!(chess_move.capture(), Some(Piece::Queen));
-        assert_eq!(chess_move.promotion(), Some(Piece::Knight));
-        assert!(chess_move.is_en_passant());
-        assert!(chess_move.is_double_step());
-        assert!(chess_move.is_castling());
     }
 }

src/fen.rs

@@ -202,7 +202,7 @@ impl FromFen for ChessBoard {
 
 #[cfg(test)]
 mod test {
-    use crate::board::MoveBuilder;
+    use crate::board::Move;
 
     use super::*;
 
@@ -220,57 +220,21 @@ mod test {
     fn en_passant() {
         // Start from default position
         let mut position = ChessBoard::default();
-        position.do_move(
+        position.do_move(Move::new(Square::E2, Square::E4, None));
-            MoveBuilder {
-                piece: Piece::Pawn,
-                start: Square::E2,
-                destination: Square::E4,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: true,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             ChessBoard::from_fen("rnbqkbnr/pppppppp/8/8/4P3/8/PPPP1PPP/RNBQKBNR b KQkq e3 0 1")
                 .unwrap(),
             position
         );
         // And now c5
-        position.do_move(
+        position.do_move(Move::new(Square::C7, Square::C5, None));
-            MoveBuilder {
-                piece: Piece::Pawn,
-                start: Square::C7,
-                destination: Square::C5,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: true,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             ChessBoard::from_fen("rnbqkbnr/pp1ppppp/8/2p5/4P3/8/PPPP1PPP/RNBQKBNR w KQkq c6 0 2")
                 .unwrap(),
             position
         );
         // Finally, Nf3
-        position.do_move(
+        position.do_move(Move::new(Square::G1, Square::F3, None));
-            MoveBuilder {
-                piece: Piece::Knight,
-                start: Square::G1,
-                destination: Square::F3,
-                capture: None,
-                promotion: None,
-                en_passant: false,
-                double_step: false,
-                castling: false,
-            }
-            .into(),
-        );
         assert_eq!(
             ChessBoard::from_fen("rnbqkbnr/pp1ppppp/8/2p5/4P3/5N2/PPPP1PPP/RNBQKB1R b KQkq - 1 2 ")
                 .unwrap(),

utils/gdb/seer_pretty_printers.py

@@ -186,79 +186,35 @@ class Move(object):
     in memory.
     """
 
-    # Should be kept in sync with the values in `move.rs`
+    def __init__(self, start, destination, promotion):
-    PIECE_SHIFT = 0
+        self._start = Square(start)
-    PIECE_MASK = 0b111
+        self._destination = Square(destination)
-    START_SHIFT = 3
+        self._promotion = Piece(promotion)
-    START_MASK = 0b11_1111
-    DESTINATION_SHIFT = 9
-    DESTINATION_MASK = 0b11_1111
-    CAPTURE_SHIFT = 15
-    CAPTURE_MASK = 0b111
-    PROMOTION_SHIFT = 18
-    PROMOTION_MASK = 0b111
-    EN_PASSANT_SHIFT = 21
-    EN_PASSANT_MASK = 0b1
-    DOUBLE_STEP_SHIFT = 22
-    DOUBLE_STEP_MASK = 0b1
-    CASTLING_SHIFT = 23
-    CASTLING_MASK = 0b1
-
-    def __init__(self, val):
-        self._val = val
 
     @classmethod
     def from_gdb(cls, val):
-        return cls(int(val))
+        start = Square(int(val["start"]))
-
+        destination = Square(int(val["destination"]))
-    @property
+        promotion = optional(Piece.from_gdb, val["promotion"])
-    def piece(self):
+        cls(start, destination, promotion)
-        return Piece(self._val >> self.PIECE_SHIFT & self.PIECE_MASK)
 
     @property
     def start(self):
-        return Square(self._val >> self.START_SHIFT & self.START_MASK)
+        return self._start
 
     @property
     def destination(self):
-        return Square(self._val >> self.DESTINATION_SHIFT & self.DESTINATION_MASK)
+        return self._destination
-
-    @property
-    def capture(self):
-        index = self._val >> self.CAPTURE_SHIFT & self.CAPTURE_MASK
-        if index == 7:
-            return None
-        return Piece(index)
 
     @property
     def promotion(self):
-        index = self._val >> self.PROMOTION_SHIFT & self.PROMOTION_MASK
+        return self._promotion
-        if index == 7:
-            return None
-        return Piece(index)
-
-    @property
-    def en_passant(self):
-        return bool(self._val >> self.EN_PASSANT_SHIFT & self.EN_PASSANT_MASK)
-
-    @property
-    def double_step(self):
-        return bool(self._val >> self.DOUBLE_STEP_SHIFT & self.DOUBLE_STEP_MASK)
-
-    @property
-    def castling(self):
-        return bool(self._val >> self.CASTLING_SHIFT & self.CASTLING_MASK)
 
     def __str__(self):
         KEYS = [
-            "piece",
             "start",
             "destination",
-            "capture",
             "promotion",
-            "en_passant",
-            "double_step",
-            "castling",
         ]
         indent = lambda s: "    " + s