harden engine rules, position invariants, and undo history

- make Position represent only valid board squares and use checked offsets
- make move endpoints fallible and remove unchecked coordinate arithmetic
- validate moves against generated legal moves, including mandatory capture
- track pending multi-captures and restrict continuation to the same piece
- derive pending state after undo/redo and preserve redo on invalid moves
- fix circular buffer wraparound, mixed iteration, and element drops
- adapt GTK and WASM to fallible endpoints and apply errors
- add rule, promotion, chain, history, and coordinate regression tests

README.md

@@ -21,6 +21,7 @@ The rules implemented by the engine are an 8x8 draughts variant:
 
 - Each player starts with twelve men.
 - Captures are mandatory.
+- A multi-capture sequence must be completed with the same piece.
 - Men move one square diagonally forward.
 - Men cannot capture backwards in this implementation.
 - Kings move one square diagonally in any direction and can capture in any

rdraught-gtk/src/rdraught_application.rs

@@ -433,7 +433,9 @@ fn create_game_window(
             let am = available_moves.borrow();
             if !am.is_empty() {
                 for mv in am.iter() {
-                    let end_pos = mv.get_end_position();
+                    let Some(end_pos) = mv.get_end_position() else {
+                        continue;
+                    };
                     let screen_position = match current_player {
                         Player::White => (8 - 1 - end_pos.row(), end_pos.col()),
                         Player::Red => (end_pos.row(), 8 - 1 - end_pos.col()),
@@ -519,10 +521,12 @@ fn create_game_window(
                     if !am.is_empty() {
                         if let Ok(pos) = position {
                             for mv in am.into_iter() {
-                                if mv.get_end_position() == pos {
+                                if mv.get_end_position() == Some(pos) {
                                     let mut rd_app = rd.borrow_mut();
                                     // println!("Applied move: {:?}", mv);
-                                    rd_app.apply_move(mv).unwrap();
+                                    if rd_app.apply_move(mv).is_err() {
+                                        continue;
+                                    }
                                     // let game_copy = rd_app.game().clone();
                                     // thread::spawn(move || {
                                     //     if let (Some(mv), analyzed_moves) =

rdraught-wasm/src/main.rs

@@ -252,7 +252,7 @@ impl App {
         let available_moves: Vec<Position> = self
             .available_moves
             .iter()
-            .map(Move::get_end_position)
+            .filter_map(Move::get_end_position)
             .collect();
         for i in 0..DraughtsBoard::rows() {
             for j in 0..DraughtsBoard::columns() {
@@ -274,7 +274,8 @@ impl App {
                 ctx.fill_rect(cell.tl().x(), cell.tl().y(), cell.width(), cell.height());
                 if (i + j) % 2 != 0 {
                     let current_player = Player::White;
-                    let position = Position::from_index((map_row(current_player, j), i));
+                    let position = Position::new(map_row(current_player, j) as u8, i as u8)
+                        .expect("board loop only visits valid squares");
                     let piece = self.game.piece_at(position);
                     if piece.is_present() {
                         piece_drawer.draw_piece(ctx, &self.xform, cell.clone(), piece)?;
@@ -412,13 +413,14 @@ fn main() -> Result<(), JsValue> {
                     let selected_move = app
                         .available_moves
                         .iter()
-                        .find(|mv| mv.get_end_position() == position)
+                        .find(|mv| mv.get_end_position() == Some(position))
                         .map(Move::clone);
                     if app.selected_piece.is_some() && selected_move.is_some() {
                         let mv = selected_move.unwrap();
-                        app.game.check_and_apply_move(&mv).unwrap();
+                        if app.game.check_and_apply_move(&mv).is_ok() {
-                        app.selected_piece = None;
+                            app.selected_piece = None;
-                        app.available_moves.clear();
+                            app.available_moves.clear();
+                        }
                     } else {
                         app.selected_piece = Some(position);
                         let mut available_moves = Vec::<Move>::new();

rdraught/src/circular_buffer.rs

@@ -4,8 +4,6 @@ use core::result::Result;
 #[derive(Debug, PartialEq)]
 pub enum Error {
     BufferIsFull,
-    BufferIsEmpty,
-    BufferHasZeroSize,
 }
 
 pub struct CircularBuffer<T: Sized, const SIZE: usize> {
@@ -17,6 +15,7 @@ pub struct CircularBuffer<T: Sized, const SIZE: usize> {
 
 impl<T: Sized, const SIZE: usize> CircularBuffer<T, SIZE> {
     pub fn new() -> CircularBuffer<T, SIZE> {
+        assert!(SIZE > 0, "CircularBuffer size must be non-zero");
         CircularBuffer {
             buffer: [const { MaybeUninit::uninit() }; SIZE],
             front: 0,
@@ -49,7 +48,7 @@ impl<T: Sized, const SIZE: usize> CircularBuffer<T, SIZE> {
     }
 
     pub fn pop_front(&mut self) -> Option<T> {
-        if self.len() == 0 {
+        if self.is_empty() {
             None
         } else {
             let mut result = MaybeUninit::uninit();
@@ -61,12 +60,13 @@ impl<T: Sized, const SIZE: usize> CircularBuffer<T, SIZE> {
     }
 
     pub fn pop_back(&mut self) -> Option<T> {
-        if self.len() == 0 {
+        if self.is_empty() {
             None
         } else {
+            let index = (self.rear + SIZE - 1) % SIZE;
             let mut result = MaybeUninit::uninit();
-            core::mem::swap(&mut result, &mut self.buffer[self.rear - 1]);
+            core::mem::swap(&mut result, &mut self.buffer[index]);
-            self.rear = (self.rear - 1) % SIZE;
+            self.rear = index;
             self.fill -= 1;
             Some(unsafe { result.assume_init() })
         }
@@ -101,7 +101,7 @@ impl<T: Sized, const SIZE: usize> CircularBuffer<T, SIZE> {
     }
 
     pub fn first(&self) -> Option<&T> {
-        if self.len() > 0 {
+        if !self.is_empty() {
             unsafe { Some(self.buffer[self.front].assume_init_ref()) }
         } else {
             None
@@ -109,14 +109,21 @@ impl<T: Sized, const SIZE: usize> CircularBuffer<T, SIZE> {
     }
 
     pub fn last(&self) -> Option<&T> {
-        if self.len() > 0 {
+        if !self.is_empty() {
-            unsafe { Some(self.buffer[self.rear - 1].assume_init_ref()) }
+            let index = (self.rear + SIZE - 1) % SIZE;
+            unsafe { Some(self.buffer[index].assume_init_ref()) }
         } else {
             None
         }
     }
 }
 
+impl<T: Sized, const SIZE: usize> Drop for CircularBuffer<T, SIZE> {
+    fn drop(&mut self) {
+        while self.pop_front().is_some() {}
+    }
+}
+
 pub struct CircularBufferIterator<T: Sized, const SIZE: usize> {
     buffer: CircularBuffer<T, SIZE>,
 }
@@ -161,7 +168,7 @@ impl<'a, T: Sized, const SIZE: usize> DoubleEndedIterator
     for CircularBufferIteratorRef<'a, T, SIZE>
 {
     fn next_back(&mut self) -> Option<Self::Item> {
-        if self.count_back == self.cb.len() {
+        if self.count + self.count_back == self.cb.len() {
             None
         } else {
             let i = (SIZE + self.cb.rear - 1 - self.count_back) % SIZE;
@@ -244,7 +251,11 @@ impl<'a, T: Sized, const SIZE: usize> IntoIterator for &'a mut CircularBuffer<T,
 
 #[cfg(test)]
 mod tests {
+    extern crate std;
+
     use super::*;
+    use std::cell::Cell;
+    use std::rc::Rc;
 
     #[test]
     fn it_works() {
@@ -252,7 +263,7 @@ mod tests {
         let mut cb = CircularBuffer::<i32, SZ>::new();
         for i in 0..SZ {
             let result = cb.push(i as i32);
-            assert_eq!(true, result.is_ok());
+            assert!(result.is_ok());
         }
 
         //Check the buffer is full
@@ -264,7 +275,7 @@ mod tests {
         }
         let result = cb.push(10);
         //Cannot push in a full buffer
-        assert_eq!(true, result.is_err());
+        assert!(result.is_err());
 
         //Check push_evict returns the first element
         let result = cb.push_evict(10);
@@ -294,4 +305,78 @@ mod tests {
             assert_eq!((cb.len() - n - 1) as i32, *i);
         }
     }
+
+    #[test]
+    fn pop_back_on_full_buffer() {
+        let mut cb = CircularBuffer::<i32, 4>::new();
+        for i in 0..4 {
+            cb.push(i).unwrap();
+        }
+        assert_eq!(Some(3), cb.pop_back());
+        assert_eq!(Some(2), cb.pop_back());
+        assert_eq!(Some(1), cb.pop_back());
+        assert_eq!(Some(0), cb.pop_back());
+        assert_eq!(None, cb.pop_back());
+    }
+
+    #[test]
+    fn last_on_full_buffer() {
+        let mut cb = CircularBuffer::<i32, 4>::new();
+        for i in 0..4 {
+            cb.push(i).unwrap();
+        }
+        assert_eq!(Some(&3), cb.last());
+    }
+
+    #[test]
+    fn mixed_front_and_back_iteration() {
+        let mut cb = CircularBuffer::<i32, 5>::new();
+        for i in 0..5 {
+            cb.push(i).unwrap();
+        }
+        let mut it = cb.iter();
+        assert_eq!(Some(&0), it.next());
+        assert_eq!(Some(&4), it.next_back());
+        assert_eq!(Some(&1), it.next());
+        assert_eq!(Some(&3), it.next_back());
+        assert_eq!(Some(&2), it.next());
+        assert_eq!(None, it.next());
+        assert_eq!(None, it.next_back());
+    }
+
+    #[test]
+    fn pop_back_after_evict() {
+        let mut cb = CircularBuffer::<i32, 4>::new();
+        for i in 0..4 {
+            cb.push(i).unwrap();
+        }
+        assert_eq!(Some(0), cb.push_evict(4));
+        assert_eq!(Some(4), cb.pop_back());
+    }
+
+    #[test]
+    fn drops_initialized_elements() {
+        struct DropCounter(Rc<Cell<usize>>);
+
+        impl Drop for DropCounter {
+            fn drop(&mut self) {
+                self.0.set(self.0.get() + 1);
+            }
+        }
+
+        let drops = Rc::new(Cell::new(0));
+        {
+            let mut cb = CircularBuffer::<DropCounter, 4>::new();
+            for _ in 0..3 {
+                cb.push(DropCounter(drops.clone())).unwrap();
+            }
+        }
+        assert_eq!(3, drops.get());
+    }
+
+    #[test]
+    #[should_panic]
+    fn zero_sized_buffer_panics() {
+        let _ = CircularBuffer::<i32, 0>::new();
+    }
 }

rdraught/src/constants.rs

@@ -1,3 +1,5 @@
 pub const BITS_PER_POSITION: usize = 3;
-pub const POSITIONS: usize = 32;
+pub const ROWS: usize = 8;
-pub const POSITIONS_PER_ROW: usize = 4;
+pub const COLUMNS: usize = 8;
+pub const POSITIONS_PER_ROW: usize = COLUMNS / 2;
+pub const POSITIONS: usize = ROWS * POSITIONS_PER_ROW;

rdraught/src/draughts.rs

@@ -1,5 +1,5 @@
 use super::circular_buffer::CircularBuffer;
-use super::constants::{BITS_PER_POSITION, POSITIONS, POSITIONS_PER_ROW};
+use super::constants::{BITS_PER_POSITION, COLUMNS, POSITIONS, POSITIONS_PER_ROW, ROWS};
 use super::movement::{LoggedMove, Move, MoveDirection};
 use super::position::Position;
 use crate::piece::Piece;
@@ -35,6 +35,7 @@ impl DraughtsBoard {
     }
 
     pub fn get(&self, position: Position) -> Piece {
+        debug_assert!((position.row() as usize) < ROWS && (position.col() as usize) < COLUMNS);
         let offset_bits = DraughtsBoard::offset_bits(position);
         let index = offset_bits / 8;
         let remainder = offset_bits % 8;
@@ -76,60 +77,18 @@ impl DraughtsBoard {
                 }
                 false
             })
-            .map(|(pos, _)| Position::new(pos.row(), pos.col()).unwrap())
+            .map(|(pos, _)| pos)
     }
 
-    fn is_position_valid(position: Position) -> bool {
+    fn undo_move(&mut self, mv: &LoggedMove) -> Result<(), Error> {
-        let position = position.to_index();
+        let end_pos = mv.get_end_position().ok_or(Error::InvalidMove)?;
-        position.0 < DraughtsBoard::rows() && position.1 < DraughtsBoard::columns()
-    }
-
-    fn get_piece(&self, p: &Position) -> Piece {
-        self.get(*p)
-    }
-
-    fn check_move_valid(&self, mv: &Move) -> Result<(), Error> {
-        let start = mv.get_start_position();
-        if mv.is_movement() {
-            let mut move_is_possible = false;
-            for possible_move in self.moves_for_piece(start, false) {
-                if possible_move.is_capture() {
-                    // Capture is mandatory
-                    return Err(Error::MovementInsteadOfCapture);
-                }
-                if possible_move == *mv {
-                    move_is_possible = true;
-                }
-            }
-            if move_is_possible {
-                Ok(())
-            } else {
-                Err(Error::InvalidMove)
-            }
-        } else {
-            let mut move_is_possible = false;
-            for possible_move in self.moves_for_piece(start, true) {
-                if possible_move == *mv {
-                    move_is_possible = true;
-                }
-            }
-            if move_is_possible {
-                Ok(())
-            } else {
-                Err(Error::InvalidMove)
-            }
-        }
-    }
-
-    fn undo_move(&mut self, mv: &LoggedMove) {
-        let end_pos = mv.get_end_position();
         let p = self.get(end_pos);
-        //Revert promotion if any
+        let player = p.player().ok_or(Error::InvalidMove)?;
+        // Revert promotion, if any.
         let initial_piece = if mv.pawn_promoted() {
-            match p.player() {
+            match player {
-                Some(Player::White) => Piece::SimpleWhitePawn,
+                Player::White => Piece::SimpleWhitePawn,
-                Some(Player::Red) => Piece::SimpleRedPawn,
+                Player::Red => Piece::SimpleRedPawn,
-                None => Piece::NoPiece,
             }
         } else {
             p
@@ -137,8 +96,9 @@ impl DraughtsBoard {
         self.set(end_pos, Piece::NoPiece);
         self.set(mv.start_position(), initial_piece);
         if mv.is_capture() {
-            let captured_position = (mv.start_position() + mv.get_end_position()) / (2, 2);
+            let captured_position =
-            let captured_piece = match p.player().unwrap() {
+                Position::midpoint(mv.start_position(), end_pos).ok_or(Error::InvalidMove)?;
+            let captured_piece = match player {
                 Player::Red => {
                     if mv.crowned_captured() {
                         Piece::CrownedWhitePawn
@@ -156,87 +116,61 @@ impl DraughtsBoard {
             };
             self.set(captured_position, captured_piece);
         }
+        Ok(())
     }
 
-    fn redo_move(&mut self, mv: &LoggedMove) {
+    fn redo_move(&mut self, mv: &LoggedMove) -> Result<(), Error> {
         let start = mv.start_position();
-        let piece = self.get_piece(&start);
+        let end = mv.get_end_position().ok_or(Error::InvalidMove)?;
+        let piece = self.get(start);
+        let player = piece.player().ok_or(Error::InvalidMove)?;
+        if self.get(end) != Piece::NoPiece {
+            return Err(Error::PositionIsOccupied);
+        }
         let final_piece = if mv.pawn_promoted() {
-            match piece.player() {
+            match player {
-                Some(Player::Red) => Piece::CrownedRedPawn,
+                Player::Red => Piece::CrownedRedPawn,
-                Some(Player::White) => Piece::CrownedWhitePawn,
+                Player::White => Piece::CrownedWhitePawn,
-                None => Piece::NoPiece,
             }
         } else {
             piece
         };
-        if mv.is_movement() {
+        if mv.is_capture() {
-            let end = mv.get_end_position();
+            let captured_pos = Position::midpoint(start, end).ok_or(Error::InvalidMove)?;
-            self.set(start, Piece::NoPiece);
+            if self.get(captured_pos) == Piece::NoPiece {
-            self.set(end, final_piece);
+                return Err(Error::NoPieceCaptured);
-        } else {
+            }
-            let end = mv.get_end_position();
-            let captured_pos = (start + end) / (2, 2);
-            self.set(start, Piece::NoPiece);
-            self.set(end, final_piece);
             self.set(captured_pos, Piece::NoPiece);
-        };
+        }
+        self.set(start, Piece::NoPiece);
+        self.set(end, final_piece);
+        Ok(())
     }
 
     fn apply_move(&mut self, mv: &Move) -> Result<(), Error> {
         let start = mv.get_start_position();
-        let piece = self.get_piece(&start);
+        let piece = self.get(start);
-        if let Piece::NoPiece = piece {
+        let player = piece.player().ok_or(Error::NoPiece)?;
-            Err(Error::InvalidMove)
+        let end = mv.get_end_position().ok_or(Error::InvalidMove)?;
+        if self.get(end) != Piece::NoPiece {
+            return Err(Error::PositionIsOccupied);
+        }
+        if mv.is_movement() {
+            self.set(start, Piece::NoPiece);
+            self.set(end, piece);
         } else {
-            let player = piece.player().unwrap();
+            let captured_pos = Position::midpoint(start, end).ok_or(Error::InvalidMove)?;
-            if mv.is_movement() {
+            match self.get(captured_pos).player() {
-                let end = mv.get_end_position();
+                None => return Err(Error::NoPieceCaptured),
-                // Make sure the move ends in a valid position
+                Some(captured_player) if captured_player != player => {
-                if !DraughtsBoard::is_position_valid(end) {
-                    return Err(Error::InvalidMove);
-                }
-                let piece_at_destination = self.get(mv.get_end_position());
-                // Make sure there is no piece at destination
-                if let Piece::NoPiece = piece_at_destination {
                     self.set(start, Piece::NoPiece);
                     self.set(end, piece);
-                } else {
+                    self.set(captured_pos, Piece::NoPiece);
-                    return Err(Error::InvalidMove);
                 }
-            } else {
+                Some(_) => return Err(Error::FriendlyPieceCaptured),
-                let end = mv.get_end_position();
+            }
-                // Make sure the move ends in a valid position
-                if !DraughtsBoard::is_position_valid(end) {
-                    return Err(Error::InvalidMove);
-                }
-                let piece_at_destination = self.get(mv.get_end_position());
-                // Make sure there is no piece at destination
-                if let Piece::NoPiece = piece_at_destination {
-                    let captured_pos = (start + end) / (2, 2);
-                    let captured = self.get(captured_pos);
-                    // Make sure there is a piece to be captured
-                    if let Piece::NoPiece = captured {
-                        return Err(Error::InvalidMove);
-                    }
-                    // Make sure the captured piece belongs to the opposite player
-                    if let Some(captured_piece_player) = captured.player() {
-                        if captured_piece_player != player {
-                            self.set(start, Piece::NoPiece);
-                            self.set(end, piece);
-                            self.set(captured_pos, Piece::NoPiece);
-                        } else {
-                            return Err(Error::InvalidMove);
-                        }
-                    } else {
-                        return Err(Error::InvalidMove);
-                    }
-                } else {
-                    return Err(Error::InvalidMove);
-                }
-            };
-            Ok(())
         }
+        Ok(())
     }
 
     pub fn moves_for_piece<'a>(
@@ -261,11 +195,11 @@ impl DraughtsBoard {
 
 impl RectangularBoard for DraughtsBoard {
     fn rows() -> usize {
-        8
+        ROWS
     }
 
     fn columns() -> usize {
-        8
+        COLUMNS
     }
 }
 
@@ -299,14 +233,12 @@ pub struct BoardIterator<'a> {
 
 impl<'a> Iterator for BoardIterator<'a> {
     fn next(&mut self) -> Option<Self::Item> {
-        let res = if self.index < (POSITIONS as u8) {
+        if self.index >= (POSITIONS as u8) {
-            let pos = Position::from(self.index);
+            return None;
-            Some((pos, self.board.get(pos)))
+        }
-        } else {
+        let pos = Position::try_from(self.index).expect("board index is a valid position");
-            None
-        };
         self.index += 1;
-        res
+        Some((pos, self.board.get(pos)))
     }
 
     type Item = (Position, Piece);
@@ -316,26 +248,44 @@ impl<'a> Iterator for BoardIterator<'a> {
 pub struct DraughtsGame {
     board: DraughtsBoard,
     next_move: Player,
+    pending_capture: Option<Position>,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Error {
     WrongPlayer,
     NoPiece,
     InvalidMove,
     InvalidPosition,
-    InvalidPiece,
     NoPieceCaptured,
-    FirendlyPieceCaptured,
+    FriendlyPieceCaptured,
     PositionIsOccupied,
     MovementInsteadOfCapture,
 }
 
+impl core::fmt::Display for Error {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Error::WrongPlayer => write!(f, "it is not this player's turn"),
+            Error::NoPiece => write!(f, "there is no piece at the starting position"),
+            Error::InvalidMove => write!(f, "the move is not legal"),
+            Error::InvalidPosition => write!(f, "the position is not a playable square"),
+            Error::NoPieceCaptured => write!(f, "there is no piece to capture"),
+            Error::FriendlyPieceCaptured => write!(f, "a player cannot capture their own piece"),
+            Error::PositionIsOccupied => write!(f, "the destination square is occupied"),
+            Error::MovementInsteadOfCapture => write!(f, "a capture is mandatory"),
+        }
+    }
+}
+
+impl core::error::Error for Error {}
+
 impl Default for DraughtsGame {
     fn default() -> Self {
         DraughtsGame {
             board: DraughtsBoard::default(),
             next_move: Player::Red,
+            pending_capture: None,
         }
     }
 }
@@ -345,6 +295,7 @@ impl DraughtsGame {
         DraughtsGame {
             board: DraughtsBoard::new(cb),
             next_move,
+            pending_capture: None,
         }
     }
 
@@ -352,34 +303,75 @@ impl DraughtsGame {
         self.board.get(p)
     }
 
-    pub fn moves_for_piece(&self, position: Position) -> MoveIterator {
+    pub fn next_move(&self) -> Player {
-        let player_opt = self.piece_at(position).player();
+        self.next_move
-        match player_opt {
+    }
-            Some(player) => {
+
-                if player == self.next_move {
+    pub fn pending_capture(&self) -> Option<Position> {
-                    let mut needle = Vec::<Piece, 2>::new();
+        self.pending_capture
-                    match player {
+    }
-                        Player::Red => {
+
-                            needle.push(Piece::SimpleRedPawn).unwrap();
+    fn pieces_for_player(player: Player) -> Vec<Piece, 2> {
-                            needle.push(Piece::CrownedRedPawn).unwrap();
+        let mut pieces = Vec::<Piece, 2>::new();
-                        }
+        match player {
-                        Player::White => {
+            Player::Red => {
-                            needle.push(Piece::SimpleWhitePawn).unwrap();
+                pieces.push(Piece::SimpleRedPawn).unwrap();
-                            needle.push(Piece::CrownedWhitePawn).unwrap();
+                pieces.push(Piece::CrownedRedPawn).unwrap();
-                        }
-                    }
-                    let mut capture_available = false;
-                    for pos in self.board.pieces(needle) {
-                        for _ in self.board.moves_for_piece(pos, true) {
-                            capture_available = true;
-                        }
-                    }
-                    self.board.moves_for_piece(position, capture_available)
-                } else {
-                    MoveIterator::empty_iterator(&self.board)
-                }
             }
-            None => MoveIterator::empty_iterator(&self.board),
+            Player::White => {
+                pieces.push(Piece::SimpleWhitePawn).unwrap();
+                pieces.push(Piece::CrownedWhitePawn).unwrap();
+            }
+        }
+        pieces
+    }
+
+    fn has_capture(&self, player: Player) -> bool {
+        for pos in self.board.pieces(Self::pieces_for_player(player)) {
+            if self.board.moves_for_piece(pos, true).next().is_some() {
+                return true;
+            }
+        }
+        false
+    }
+
+    pub fn moves_for_piece(&self, position: Position) -> MoveIterator<'_> {
+        if let Some(pending) = self.pending_capture {
+            if position != pending {
+                return MoveIterator::empty_iterator(&self.board);
+            }
+            return self.board.moves_for_piece(position, true);
+        }
+
+        let Some(player) = self.piece_at(position).player() else {
+            return MoveIterator::empty_iterator(&self.board);
+        };
+        if player != self.next_move {
+            return MoveIterator::empty_iterator(&self.board);
+        }
+        let capture_available = self.has_capture(player);
+        self.board.moves_for_piece(position, capture_available)
+    }
+
+    pub fn validate_move(&self, mv: &Move) -> Result<(), Error> {
+        let start = mv.get_start_position();
+        let piece = self.board.get(start);
+        let player = piece.player().ok_or(Error::NoPiece)?;
+        if player != self.next_move {
+            return Err(Error::WrongPlayer);
+        }
+        if let Some(pending) = self.pending_capture {
+            if start != pending {
+                return Err(Error::InvalidMove);
+            }
+        }
+        if mv.is_movement() && self.has_capture(player) {
+            return Err(Error::MovementInsteadOfCapture);
+        }
+        if self.moves_for_piece(start).any(|legal| legal == *mv) {
+            Ok(())
+        } else {
+            Err(Error::InvalidMove)
         }
     }
 
@@ -402,49 +394,42 @@ impl DraughtsGame {
         }
     }
 
-    fn undo_move(&mut self, mv: &LoggedMove) {
+    fn undo_move(&mut self, mv: &LoggedMove) -> Result<(), Error> {
-        self.board.undo_move(mv);
+        self.board.undo_move(mv)?;
         if !mv.is_capture() || !mv.multi_capture() {
             self.next_turn();
         }
+        self.pending_capture = None;
+        Ok(())
     }
 
-    fn redo_move(&mut self, mv: &LoggedMove) {
+    fn redo_move(&mut self, mv: &LoggedMove) -> Result<(), Error> {
-        self.board.redo_move(mv);
+        self.board.redo_move(mv)?;
         if !mv.is_capture() || !mv.multi_capture() {
             self.next_turn();
         }
+        self.pending_capture = None;
+        Ok(())
     }
 
     pub fn check_and_apply_move(&mut self, mv: &Move) -> Result<(), Error> {
+        self.validate_move(mv)?;
+        self.apply_validated_move(mv)
+    }
+
+    fn apply_validated_move(&mut self, mv: &Move) -> Result<(), Error> {
         let start = mv.get_start_position();
-        let piece = self.board.get_piece(&start);
+        let piece = self.board.get(start);
-        if let Some(player) = piece.player() {
+        let player = piece.player().ok_or(Error::NoPiece)?;
-            if mv.is_movement() {
+        let end = mv.get_end_position().ok_or(Error::InvalidMove)?;
-                if self.next_move == player {
+        self.board.apply_move(mv)?;
-                    self.board.apply_move(mv)?;
+
-                    self.next_turn();
+        let continues = mv.is_capture() && self.board.moves_for_piece(end, true).next().is_some();
-                } else {
+        if continues {
-                    return Err(Error::WrongPlayer);
+            self.pending_capture = Some(end);
-                }
+        } else {
-            } else if self.next_move == player {
+            self.pending_capture = None;
-                self.board.apply_move(mv)?;
+            if !piece.is_crowned() && Self::is_last_row(&end, player) {
-                // Check if more captures are available for the current piece
-                if self
-                    .board
-                    .moves_for_piece(mv.get_end_position(), true)
-                    .next()
-                    .is_some()
-                {
-                    return Ok(());
-                }
-                self.next_turn();
-            } else {
-                return Err(Error::WrongPlayer);
-            };
-            let end = mv.get_end_position();
-            //Promote pawns that reach the last row
-            if !piece.is_crowned() && DraughtsGame::is_last_row(&end, player) {
                 self.board.set(
                     end,
                     match player {
@@ -453,32 +438,16 @@ impl DraughtsGame {
                     },
                 );
             }
-            Ok(())
+            self.next_turn();
-        } else {
-            Err(Error::NoPiece)
         }
+        Ok(())
     }
 
     pub fn score_for_player(&self, p: Player) -> u32 {
         let md = MoveDirection::NE;
         let mut score = 0u32;
-        let pieces = {
+        for pos in self.board.pieces(Self::pieces_for_player(p)) {
-            let mut v = Vec::<Piece, 2>::new();
+            let piece = self.board.get(pos);
-            match p {
-                Player::White => {
-                    v.push(Piece::SimpleWhitePawn).unwrap();
-                    v.push(Piece::CrownedWhitePawn).unwrap();
-                    v
-                }
-                Player::Red => {
-                    v.push(Piece::SimpleRedPawn).unwrap();
-                    v.push(Piece::CrownedRedPawn).unwrap();
-                    v
-                }
-            }
-        };
-        for pos in self.board.pieces(pieces) {
-            let piece = self.board.get_piece(&pos);
             if piece.is_crowned() {
                 score += 20;
             } else {
@@ -510,7 +479,8 @@ impl DraughtsGame {
         depth: u32,
         analyzed_moves: &mut usize,
     ) -> f32 {
-        self.check_and_apply_move(mv).unwrap();
+        self.apply_validated_move(mv)
+            .expect("available move must be legal");
         if depth != 0 {
             let mut best_score = None;
             for mv in self.available_moves() {
@@ -531,20 +501,9 @@ impl DraughtsGame {
         }
     }
 
-    fn available_moves(&self) -> impl Iterator<Item = Move> {
+    fn available_moves(&self) -> impl Iterator<Item = Move> + '_ {
-        let mut pieces = Vec::<Piece, 2>::new();
-        match self.next_move {
-            Player::White => {
-                pieces.push(Piece::SimpleWhitePawn).unwrap();
-                pieces.push(Piece::CrownedWhitePawn).unwrap();
-            }
-            Player::Red => {
-                pieces.push(Piece::SimpleRedPawn).unwrap();
-                pieces.push(Piece::CrownedRedPawn).unwrap();
-            }
-        }
         self.board
-            .pieces(pieces)
+            .pieces(Self::pieces_for_player(self.next_move))
             .flat_map(|pos| self.moves_for_piece(pos))
     }
 
@@ -578,6 +537,9 @@ impl DraughtsGame {
     }
 
     pub fn winner(&self) -> Option<Player> {
+        if self.pending_capture.is_some() {
+            return None;
+        }
         if self.available_moves().next().is_none() {
             Some(match self.next_move {
                 Player::White => Player::Red,
@@ -642,27 +604,24 @@ impl<'a> Iterator for MoveIterator<'a> {
                 continue;
             }
             let movement = Move::movement(self.position, direction);
-            let next_pos = movement.get_end_position();
+            let Some(next_pos) = movement.get_end_position() else {
-            if DraughtsBoard::is_position_valid(next_pos) {
+                continue;
-                let piece_at_next_position = self.board.get(next_pos);
+            };
-                match piece_at_next_position {
+            let piece_at_next_position = self.board.get(next_pos);
-                    Piece::NoPiece => {
+            match piece_at_next_position {
-                        if self.capture_only {
+                Piece::NoPiece => {
-                            continue;
+                    if !self.capture_only {
-                        } else {
+                        return Some(movement);
-                            return Some(movement);
-                        }
                     }
-                    _ => {
+                }
-                        if piece.player() != piece_at_next_position.player() {
+                _ => {
-                            let capture = Move::capture(self.position, direction);
+                    if piece.player() != piece_at_next_position.player() {
-                            let next_position = capture.get_end_position();
+                        let capture = Move::capture(self.position, direction);
-                            if DraughtsBoard::is_position_valid(next_position) {
+                        let Some(next_position) = capture.get_end_position() else {
-                                let piece_at_end_position = self.board.get(next_position);
+                            continue;
-                                if piece_at_end_position == Piece::NoPiece {
+                        };
-                                    return Some(capture);
+                        if self.board.get(next_position) == Piece::NoPiece {
-                                }
+                            return Some(capture);
-                            }
                         }
                     }
                 }
@@ -695,85 +654,81 @@ impl RDraughtApplication {
         self.cursor > 0
     }
 
-    pub fn undo(&mut self) -> Result<(), Error> {
+    fn pending_capture_from_history(&self) -> Option<Position> {
-        let mut it = self.moves.iter().rev();
+        let applied = self.moves.len().checked_sub(self.cursor)?;
-        let mut count = 0;
+        if applied == 0 {
-        loop {
+            return None;
-            let mv = it.next();
-            if let Some(mv) = mv {
-                if count == self.cursor {
-                    self.game.undo_move(&mv);
-                    self.cursor += 1;
-                    break;
-                }
-                count += 1;
-            } else {
-                return Err(Error::InvalidMove);
-            }
         }
+        let last = self.moves.iter().nth(applied - 1)?;
+        if last.is_capture() && last.multi_capture() {
+            last.get_end_position()
+        } else {
+            None
+        }
+    }
+
+    pub fn undo(&mut self) -> Result<(), Error> {
+        if !self.can_undo() {
+            return Err(Error::InvalidMove);
+        }
+        let index = self.moves.len() - 1 - self.cursor;
+        let mv = *self.moves.iter().nth(index).ok_or(Error::InvalidMove)?;
+        self.game.undo_move(&mv)?;
+        self.cursor += 1;
+        self.game.pending_capture = self.pending_capture_from_history();
         Ok(())
     }
 
     pub fn redo(&mut self) -> Result<(), Error> {
-        if self.cursor == 0 {
+        if !self.can_redo() {
-            Err(Error::InvalidMove)
+            return Err(Error::InvalidMove);
-        } else {
-            let mut it = self.moves.iter().rev();
-            let mut count = 0;
-            loop {
-                let mv = it.next();
-                if let Some(mv) = mv {
-                    if count + 1 == self.cursor {
-                        self.game.redo_move(&mv);
-                        self.cursor -= 1;
-                        break;
-                    }
-                    count += 1;
-                } else {
-                    return Err(Error::InvalidMove);
-                }
-            }
-            Ok(())
         }
+        let index = self.moves.len() - self.cursor;
+        let mv = *self.moves.iter().nth(index).ok_or(Error::InvalidMove)?;
+        self.game.redo_move(&mv)?;
+        self.cursor -= 1;
+        self.game.pending_capture = self.pending_capture_from_history();
+        Ok(())
     }
 
     pub fn apply_move(&mut self, mv: Move) -> Result<(), Error> {
-        while self.cursor > 0 {
+        self.game.validate_move(&mv)?;
-            self.moves.pop_back();
+
-            self.cursor -= 1;
+        let start = mv.get_start_position();
-        }
+        let initial_piece = self.game.piece_at(start);
-        let initial_piece = self.game.piece_at(mv.get_start_position());
+        let end_position = mv.get_end_position().ok_or(Error::InvalidMove)?;
-        let end_position = mv.get_end_position();
+        let crowned_capture = if mv.is_capture() {
-        let crowned_capture = if let Move::Capture {
+            let captured_position =
-            start: _,
+                Position::midpoint(start, end_position).ok_or(Error::InvalidMove)?;
-            direction: _,
+            self.game.piece_at(captured_position).is_crowned()
-        } = mv
-        {
-            self.game
-                .piece_at((mv.get_start_position() + end_position) / (2, 2))
-                .is_crowned()
         } else {
             false
         };
-        let next_player_move = self.game.next_move;
+
-        self.game.check_and_apply_move(&mv)?;
+        self.game.apply_validated_move(&mv)?;
+
+        while self.cursor > 0 {
+            self.moves
+                .pop_back()
+                .expect("cursor implies a redo history entry");
+            self.cursor -= 1;
+        }
+
         let final_piece = self.game.piece_at(end_position);
         let is_pawn_promoted =
             initial_piece != final_piece && !initial_piece.is_crowned() && final_piece.is_crowned();
+        let multi_capture = mv.is_capture() && self.game.pending_capture.is_some();
         let logged_move = match mv {
             Move::Movement { start, direction } => {
                 LoggedMove::movement(start, direction, is_pawn_promoted)
             }
-            Move::Capture { start, direction } => {
+            Move::Capture { start, direction } => LoggedMove::capture(
-                let multi_capture = self.game.next_move == next_player_move;
+                start,
-                LoggedMove::capture(
+                direction,
-                    start,
+                is_pawn_promoted,
-                    direction,
+                crowned_capture,
-                    is_pawn_promoted,
+                multi_capture,
-                    crowned_capture,
+            ),
-                    multi_capture,
-                )
-            }
         };
         self.moves.push_evict(logged_move);
         Ok(())
@@ -804,9 +759,20 @@ mod tests {
     extern crate std;
     use crate::{LoggedMove, MoveDirection};
 
-    use super::{DraughtsBoard, Piece, Position};
+    use super::{
+        DraughtsBoard, DraughtsGame, Error, Move, Piece, Player, Position, RDraughtApplication,
+    };
     use std::collections::HashMap;
 
+    fn pos(row: u8, col: u8) -> Position {
+        Position::new(row, col).unwrap()
+    }
+
+    fn game_from(pieces: &[(Position, Piece)], next: Player) -> DraughtsGame {
+        let map = pieces.iter().copied().collect::<HashMap<Position, Piece>>();
+        DraughtsGame::new(|p| map.get(&p).copied().unwrap_or(Piece::NoPiece), next)
+    }
+
     #[test]
     fn test_undo_redo() {
         let pieces = [
@@ -845,9 +811,9 @@ mod tests {
         ];
         for mv in moves {
             let mut board_clone = board.clone();
-            board_clone.redo_move(&mv);
+            board_clone.redo_move(&mv).unwrap();
             assert_ne!(board, board_clone);
-            board_clone.undo_move(&mv);
+            board_clone.undo_move(&mv).unwrap();
             assert_eq!(board, board_clone);
         }
     }
@@ -888,6 +854,225 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn error_implements_std_error() {
+        fn assert_error<E: std::error::Error>() {}
+        assert_error::<Error>();
+    }
+
+    #[test]
+    fn rejects_backward_pawn_move() {
+        let mut game = game_from(&[(pos(3, 3), Piece::SimpleRedPawn)], Player::Red);
+        let mv = Move::movement(pos(3, 3), MoveDirection::NW);
+        assert_eq!(Err(Error::InvalidMove), game.check_and_apply_move(&mv));
+    }
+
+    #[test]
+    fn mandatory_capture_rejects_quiet_move() {
+        let mut game = game_from(
+            &[
+                (pos(3, 3), Piece::SimpleRedPawn),
+                (pos(2, 4), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+        let quiet = Move::movement(pos(3, 3), MoveDirection::NW);
+        assert_eq!(
+            Err(Error::MovementInsteadOfCapture),
+            game.check_and_apply_move(&quiet)
+        );
+
+        let capture = Move::capture(pos(3, 3), MoveDirection::SE);
+        assert_eq!(Ok(()), game.check_and_apply_move(&capture));
+    }
+
+    #[test]
+    fn rejects_backward_pawn_capture() {
+        let mut game = game_from(
+            &[
+                (pos(3, 3), Piece::SimpleRedPawn),
+                (pos(4, 2), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+        let capture = Move::capture(pos(3, 3), MoveDirection::NE);
+        assert_eq!(Err(Error::InvalidMove), game.check_and_apply_move(&capture));
+    }
+
+    #[test]
+    fn multi_capture_chain_is_locked_to_one_piece() {
+        let mut game = game_from(
+            &[
+                (pos(5, 3), Piece::SimpleRedPawn),
+                (pos(4, 4), Piece::SimpleWhitePawn),
+                (pos(2, 6), Piece::SimpleWhitePawn),
+                (pos(7, 3), Piece::SimpleRedPawn),
+                (pos(6, 4), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+
+        let first = Move::capture(pos(5, 3), MoveDirection::SE);
+        assert_eq!(Ok(()), game.check_and_apply_move(&first));
+        assert_eq!(Some(pos(3, 5)), game.pending_capture());
+
+        let other_piece = Move::capture(pos(7, 3), MoveDirection::SE);
+        assert_eq!(
+            Err(Error::InvalidMove),
+            game.check_and_apply_move(&other_piece)
+        );
+        assert_eq!(0, game.moves_for_piece(pos(7, 3)).count());
+
+        let quiet = Move::movement(pos(3, 5), MoveDirection::NE);
+        assert_eq!(
+            Err(Error::MovementInsteadOfCapture),
+            game.check_and_apply_move(&quiet)
+        );
+
+        let second = Move::capture(pos(3, 5), MoveDirection::SE);
+        assert_eq!(Ok(()), game.check_and_apply_move(&second));
+        assert_eq!(None, game.pending_capture());
+        assert_eq!(Player::White, game.next_move());
+        assert_eq!(Piece::SimpleRedPawn, game.piece_at(pos(1, 7)));
+        assert_eq!(Piece::NoPiece, game.piece_at(pos(4, 4)));
+        assert_eq!(Piece::NoPiece, game.piece_at(pos(2, 6)));
+    }
+
+    #[test]
+    fn promotion_is_reverted_by_undo() {
+        let game = game_from(&[(pos(1, 1), Piece::SimpleRedPawn)], Player::Red);
+        let mut app = RDraughtApplication::new(game);
+        let mv = Move::movement(pos(1, 1), MoveDirection::SE);
+        app.apply_move(mv).unwrap();
+        assert_eq!(Piece::CrownedRedPawn, app.game().piece_at(pos(0, 2)));
+
+        app.undo().unwrap();
+        assert_eq!(Piece::SimpleRedPawn, app.game().piece_at(pos(1, 1)));
+        assert_eq!(Piece::NoPiece, app.game().piece_at(pos(0, 2)));
+
+        app.redo().unwrap();
+        assert_eq!(Piece::CrownedRedPawn, app.game().piece_at(pos(0, 2)));
+    }
+
+    #[test]
+    fn capture_promotion_is_reverted_by_undo() {
+        let game = game_from(
+            &[
+                (pos(2, 2), Piece::SimpleRedPawn),
+                (pos(1, 3), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+        let mut app = RDraughtApplication::new(game);
+        app.apply_move(Move::capture(pos(2, 2), MoveDirection::SE))
+            .unwrap();
+        assert_eq!(Piece::CrownedRedPawn, app.game().piece_at(pos(0, 4)));
+        assert_eq!(Piece::NoPiece, app.game().piece_at(pos(1, 3)));
+
+        app.undo().unwrap();
+        assert_eq!(Piece::SimpleRedPawn, app.game().piece_at(pos(2, 2)));
+        assert_eq!(Piece::SimpleWhitePawn, app.game().piece_at(pos(1, 3)));
+    }
+
+    #[test]
+    fn multi_capture_pending_state_survives_undo_and_redo() {
+        let game = game_from(
+            &[
+                (pos(5, 3), Piece::SimpleRedPawn),
+                (pos(4, 4), Piece::SimpleWhitePawn),
+                (pos(2, 6), Piece::SimpleWhitePawn),
+                (pos(7, 3), Piece::SimpleRedPawn),
+                (pos(6, 4), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+        let mut app = RDraughtApplication::new(game);
+
+        app.apply_move(Move::capture(pos(5, 3), MoveDirection::SE))
+            .unwrap();
+        assert_eq!(Some(pos(3, 5)), app.game().pending_capture());
+        assert_eq!(None, app.game().winner());
+
+        app.apply_move(Move::capture(pos(3, 5), MoveDirection::SE))
+            .unwrap();
+        assert_eq!(None, app.game().pending_capture());
+        assert_eq!(Player::White, app.game().next_move());
+
+        app.undo().unwrap();
+        assert_eq!(Some(pos(3, 5)), app.game().pending_capture());
+        assert_eq!(Player::Red, app.game().next_move());
+        assert_eq!(1, app.game().moves_for_piece(pos(3, 5)).count());
+        assert_eq!(0, app.game().moves_for_piece(pos(7, 3)).count());
+
+        app.undo().unwrap();
+        assert_eq!(None, app.game().pending_capture());
+        assert_eq!(Player::Red, app.game().next_move());
+        assert_eq!(Piece::SimpleRedPawn, app.game().piece_at(pos(5, 3)));
+        assert_eq!(Piece::SimpleWhitePawn, app.game().piece_at(pos(4, 4)));
+
+        app.redo().unwrap();
+        assert_eq!(Some(pos(3, 5)), app.game().pending_capture());
+        app.redo().unwrap();
+        assert_eq!(None, app.game().pending_capture());
+        assert_eq!(Player::White, app.game().next_move());
+        assert_eq!(Piece::SimpleRedPawn, app.game().piece_at(pos(1, 7)));
+    }
+
+    #[test]
+    fn invalid_move_keeps_redo_history() {
+        let mut app = RDraughtApplication::new(DraughtsGame::default());
+        let mv = app.game().moves_for_piece(pos(5, 1)).next().unwrap();
+        app.apply_move(mv).unwrap();
+        app.undo().unwrap();
+        assert!(app.can_redo());
+
+        let invalid = Move::movement(pos(3, 3), MoveDirection::NE);
+        assert_eq!(Err(Error::NoPiece), app.apply_move(invalid));
+        assert!(app.can_redo());
+
+        app.redo().unwrap();
+        assert!(!app.can_redo());
+    }
+
+    #[test]
+    fn apply_move_after_undo_on_full_history() {
+        let mut app = RDraughtApplication::new(DraughtsGame::default());
+        for _ in 0..128 {
+            app.moves
+                .push_evict(LoggedMove::movement(pos(0, 0), MoveDirection::NE, false));
+        }
+        app.cursor = 1;
+
+        app.apply_move(Move::movement(pos(5, 1), MoveDirection::SW))
+            .unwrap();
+        assert_eq!(128, app.moves.len());
+        assert_eq!(0, app.cursor);
+    }
+
+    #[test]
+    fn kings_move_and_capture_backwards() {
+        let mut game = game_from(&[(pos(4, 4), Piece::CrownedRedPawn)], Player::Red);
+        assert_eq!(
+            Ok(()),
+            game.check_and_apply_move(&Move::movement(pos(4, 4), MoveDirection::NE))
+        );
+        assert_eq!(Piece::CrownedRedPawn, game.piece_at(pos(5, 5)));
+
+        let mut game = game_from(
+            &[
+                (pos(4, 4), Piece::CrownedRedPawn),
+                (pos(5, 5), Piece::SimpleWhitePawn),
+            ],
+            Player::Red,
+        );
+        assert_eq!(
+            Ok(()),
+            game.check_and_apply_move(&Move::capture(pos(4, 4), MoveDirection::NE))
+        );
+        assert_eq!(Piece::CrownedRedPawn, game.piece_at(pos(6, 6)));
+        assert_eq!(Piece::NoPiece, game.piece_at(pos(5, 5)));
+    }
 }
 
 #[cfg(test)]

rdraught/src/movement.rs

@@ -12,6 +12,15 @@ pub enum MoveDirection {
 }
 
 impl MoveDirection {
+    pub(crate) fn delta(self) -> (i8, i8) {
+        match self {
+            MoveDirection::NE => (1, 1),
+            MoveDirection::SE => (-1, 1),
+            MoveDirection::SW => (-1, -1),
+            MoveDirection::NW => (1, -1),
+        }
+    }
+
     pub fn is_forward(&self, piece: Piece) -> Result<bool, Error> {
         match piece.player() {
             Some(Player::Red) => Ok(matches!(self, Self::SE) || matches!(self, Self::SW)),
@@ -25,26 +34,13 @@ pub fn compute_end_position(
     start_position: Position,
     direction: MoveDirection,
     is_capture: bool,
-) -> Position {
+) -> Option<Position> {
-    if is_capture {
+    let (dr, dc) = direction.delta();
-        let direction = direction;
+    let step = if is_capture { 2 } else { 1 };
-        match direction {
+    start_position.offset(dr * step, dc * step)
-            MoveDirection::NE => start_position + (2, 2),
-            MoveDirection::SE => start_position + (-2, 2),
-            MoveDirection::SW => start_position + (-2, -2),
-            MoveDirection::NW => start_position + (2, -2),
-        }
-    } else {
-        match direction {
-            MoveDirection::NE => start_position + (1, 1),
-            MoveDirection::SE => start_position + (-1, 1),
-            MoveDirection::SW => start_position + (-1, -1),
-            MoveDirection::NW => start_position + (1, -1),
-        }
-    }
 }
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub struct LoggedMove {
     data: u16,
 }
@@ -82,7 +78,7 @@ impl LoggedMove {
     }
 
     pub fn start_position(&self) -> Position {
-        Position::from((self.data & 31) as u8)
+        Position::try_from((self.data & 31) as u8).expect("logged move contains a valid position")
     }
 
     pub fn direction(&self) -> MoveDirection {
@@ -109,12 +105,8 @@ impl LoggedMove {
         (self.data & 1024) != 0
     }
 
-    pub fn get_end_position(&self) -> Position {
+    pub fn get_end_position(&self) -> Option<Position> {
-        compute_end_position(
+        compute_end_position(self.start_position(), self.direction(), self.is_capture())
-            self.start_position().clone(),
-            self.direction(),
-            self.is_capture(),
-        )
     }
 }
 
@@ -160,15 +152,15 @@ impl Move {
             Move::Movement {
                 start,
                 direction: _,
-            } => start.clone(),
+            } => *start,
             Move::Capture {
                 start,
                 direction: _,
-            } => start.clone(),
+            } => *start,
         }
     }
 
-    pub fn get_end_position(&self) -> Position {
+    pub fn get_end_position(&self) -> Option<Position> {
         compute_end_position(
             self.get_start_position(),
             self.get_direction(),
@@ -181,22 +173,21 @@ impl Move {
             Move::Movement {
                 start: _,
                 direction,
-            } => direction.clone(),
+            } => *direction,
             Move::Capture {
                 start: _,
                 direction,
-            } => direction.clone(),
+            } => *direction,
         }
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{LoggedMove, MoveDirection, Piece};
+    use super::{LoggedMove, Move, MoveDirection, Piece};
     use crate::Position;
 
     struct MoveData {
-        piece: Piece,
         start: Position,
         dir: MoveDirection,
         capture: bool,
@@ -205,12 +196,15 @@ mod tests {
         multi_capture: bool,
     }
 
+    fn pos(row: u8, col: u8) -> Position {
+        Position::new(row, col).unwrap()
+    }
+
     #[test]
     fn test_from_into() {
         let move_datas = [
             MoveData {
-                piece: Piece::SimpleRedPawn,
+                start: pos(0, 0),
-                start: Position::new(0, 0).unwrap(),
                 dir: super::MoveDirection::NE,
                 capture: false,
                 crowned_capture: false,
@@ -218,8 +212,7 @@ mod tests {
                 multi_capture: false,
             },
             MoveData {
-                piece: Piece::SimpleWhitePawn,
+                start: pos(3, 5),
-                start: Position::new(3, 5).unwrap(),
                 dir: super::MoveDirection::SW,
                 capture: false,
                 crowned_capture: false,
@@ -227,8 +220,7 @@ mod tests {
                 multi_capture: false,
             },
             MoveData {
-                piece: Piece::SimpleWhitePawn,
+                start: pos(6, 6),
-                start: Position::new(6, 6).unwrap(),
                 dir: super::MoveDirection::NW,
                 capture: false,
                 crowned_capture: false,
@@ -236,8 +228,7 @@ mod tests {
                 multi_capture: false,
             },
             MoveData {
-                piece: Piece::SimpleWhitePawn,
+                start: pos(5, 5),
-                start: Position::new(5, 5).unwrap(),
                 dir: super::MoveDirection::NE,
                 capture: true,
                 crowned_capture: true,
@@ -266,4 +257,44 @@ mod tests {
             assert_eq!(md.multi_capture, mv.multi_capture());
         }
     }
+
+    #[test]
+    fn test_end_positions() {
+        assert_eq!(
+            Some(pos(1, 1)),
+            Move::movement(pos(0, 0), MoveDirection::NE).get_end_position()
+        );
+        assert_eq!(
+            None,
+            Move::movement(pos(0, 0), MoveDirection::NW).get_end_position()
+        );
+        assert_eq!(
+            Some(pos(2, 2)),
+            Move::capture(pos(0, 0), MoveDirection::NE).get_end_position()
+        );
+        assert_eq!(
+            None,
+            Move::capture(pos(7, 7), MoveDirection::SE).get_end_position()
+        );
+        assert_eq!(
+            None,
+            LoggedMove::movement(pos(0, 0), MoveDirection::NW, false).get_end_position()
+        );
+    }
+
+    #[test]
+    fn test_piece_direction() {
+        assert!(
+            MoveDirection::NE
+                .is_forward(Piece::SimpleWhitePawn)
+                .unwrap()
+        );
+        assert!(!MoveDirection::NE.is_forward(Piece::SimpleRedPawn).unwrap());
+        assert!(
+            MoveDirection::NW
+                .is_forward(Piece::CrownedWhitePawn)
+                .unwrap()
+        );
+        assert!(!MoveDirection::NW.is_forward(Piece::CrownedRedPawn).unwrap());
+    }
 }

rdraught/src/position.rs

@@ -1,12 +1,12 @@
 use crate::Error;
 use core::fmt::Display;
-use core::ops::Add;
-use core::ops::Div;
-use core::ops::Mul;
-use core::ops::Sub;
 
-use crate::constants::POSITIONS_PER_ROW;
+use crate::constants::{COLUMNS, POSITIONS, POSITIONS_PER_ROW, ROWS};
 
+/// A playable square on the board.
+///
+/// The type is deliberately limited to squares that exist and that have the
+/// correct colour: `row < 8`, `col < 8` and `(row + col) % 2 == 0`.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub struct Position((u8, u8));
 
@@ -17,16 +17,9 @@ impl Display for Position {
 }
 
 impl Position {
-    pub fn to_index(&self) -> (usize, usize) {
-        (self.0.1 as usize, self.0.0 as usize)
-    }
-
-    pub fn from_index(index: (usize, usize)) -> Position {
-        Position((index.0 as u8, index.1 as u8))
-    }
-
     pub fn new(row: u8, column: u8) -> Result<Position, Error> {
-        if (row + column) % 2 == 0 {
+        if (row as usize) < ROWS && (column as usize) < COLUMNS && (row + column).is_multiple_of(2)
+        {
             Ok(Position((row, column)))
         } else {
             Err(Error::InvalidPosition)
@@ -36,106 +29,62 @@ impl Position {
     pub fn row(&self) -> u8 {
         self.0.0
     }
+
     pub fn col(&self) -> u8 {
         self.0.1
     }
-}
 
-impl Add<Position> for Position {
+    /// Returns the square reached by moving `dr` rows and `dc` columns, if it
-    type Output = Position;
+    /// is a playable square.
+    pub fn offset(self, dr: i8, dc: i8) -> Option<Position> {
+        let row = i16::from(self.row()) + i16::from(dr);
+        let col = i16::from(self.col()) + i16::from(dc);
+        if row < 0 || row >= ROWS as i16 || col < 0 || col >= COLUMNS as i16 {
+            return None;
+        }
+        Position::new(row as u8, col as u8).ok()
+    }
 
-    fn add(self, rhs: Position) -> Self::Output {
+    /// Returns the square halfway between `start` and `end`, if the midpoint
-        Position((self.0.0 + rhs.0.0, self.0.1 + rhs.0.1))
+    /// is a playable square.
+    pub fn midpoint(start: Position, end: Position) -> Option<Position> {
+        let row = i16::from(start.row()) + i16::from(end.row());
+        let col = i16::from(start.col()) + i16::from(end.col());
+        if row % 2 != 0 || col % 2 != 0 {
+            return None;
+        }
+        Position::new((row / 2) as u8, (col / 2) as u8).ok()
     }
 }
 
-impl Sub<Position> for Position {
+impl TryFrom<u8> for Position {
-    type Output = Position;
+    type Error = Error;
 
-    fn sub(self, rhs: Position) -> Self::Output {
+    fn try_from(n: u8) -> Result<Self, Self::Error> {
-        Position((self.0.0 - rhs.0.0, self.0.1 - rhs.0.1))
+        if n as usize >= POSITIONS {
-    }
+            return Err(Error::InvalidPosition);
-}
+        }
-
+        let row = (ROWS - 1 - n as usize / POSITIONS_PER_ROW) as u8;
-impl Div<Position> for Position {
+        let offset = if row.is_multiple_of(2) { 0u8 } else { 1u8 };
-    type Output = Position;
+        let col = (n as usize % POSITIONS_PER_ROW * 2 + offset as usize) as u8;
-
+        Position::new(row, col)
-    fn div(self, rhs: Position) -> Self::Output {
-        Position((self.0.0 / rhs.0.0, self.0.1 / rhs.0.1))
-    }
-}
-
-impl Mul<Position> for Position {
-    type Output = Position;
-
-    fn mul(self, rhs: Position) -> Self::Output {
-        Position((self.0.0 * rhs.0.0, self.0.1 * rhs.0.1))
-    }
-}
-
-impl Add<(i32, i32)> for Position {
-    type Output = Position;
-
-    fn add(self, rhs: (i32, i32)) -> Self::Output {
-        Position((
-            (self.0.0 as i32 + rhs.0) as u8,
-            (self.0.1 as i32 + rhs.1) as u8,
-        ))
-    }
-}
-
-impl Sub<(i32, i32)> for Position {
-    type Output = Position;
-
-    fn sub(self, rhs: (i32, i32)) -> Self::Output {
-        Position((
-            (self.0.0 as i32 - rhs.0) as u8,
-            (self.0.1 as i32 - rhs.1) as u8,
-        ))
-    }
-}
-
-impl Div<(i32, i32)> for Position {
-    type Output = Position;
-
-    fn div(self, rhs: (i32, i32)) -> Self::Output {
-        Position((
-            (self.0.0 as i32 / rhs.0) as u8,
-            (self.0.1 as i32 / rhs.1) as u8,
-        ))
-    }
-}
-
-impl Mul<(i32, i32)> for Position {
-    type Output = Position;
-
-    fn mul(self, rhs: (i32, i32)) -> Self::Output {
-        Position((
-            (self.0.0 as i32 * rhs.0) as u8,
-            (self.0.1 as i32 * rhs.1) as u8,
-        ))
     }
 }
 
 impl From<Position> for u8 {
     fn from(pos: Position) -> Self {
-        (8 - 1 - pos.row()) * (POSITIONS_PER_ROW as u8) + pos.col() / 2
+        (ROWS as u8 - 1 - pos.row()) * (POSITIONS_PER_ROW as u8) + pos.col() / 2
-    }
-}
-
-impl From<u8> for Position {
-    fn from(n: u8) -> Self {
-        // println!("{}", n);
-        let row = 8 - 1 - n / (POSITIONS_PER_ROW as u8);
-        let offset = if row % 2 == 0 { 0u8 } else { 1u8 };
-        let col = n % (POSITIONS_PER_ROW as u8) * 2 + offset;
-        Position::new(row, col).unwrap()
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::Position;
+    use crate::Error;
+
+    fn pos(row: u8, col: u8) -> Position {
+        Position::new(row, col).unwrap()
+    }
 
     #[test]
     fn test_from_into() {
@@ -154,8 +103,38 @@ mod tests {
         for p in positions {
             let p = p.unwrap();
             let n = <Position as Into<u8>>::into(p);
-            let p2 = <Position as From<u8>>::from(n);
+            let p2 = <Position as TryFrom<u8>>::try_from(n).unwrap();
             assert_eq!(p, p2);
         }
     }
+
+    #[test]
+    fn test_invalid_positions() {
+        assert_eq!(Err(Error::InvalidPosition), Position::new(0, 1));
+        assert_eq!(Err(Error::InvalidPosition), Position::new(8, 0));
+        assert_eq!(Err(Error::InvalidPosition), Position::new(0, 8));
+        assert_eq!(Err(Error::InvalidPosition), Position::try_from(32u8));
+        assert_eq!(Err(Error::InvalidPosition), Position::try_from(u8::MAX));
+    }
+
+    #[test]
+    fn test_index_round_trip() {
+        for n in 0..32u8 {
+            let p = Position::try_from(n).unwrap();
+            assert_eq!(n, u8::from(p));
+        }
+    }
+
+    #[test]
+    fn test_offset_and_midpoint() {
+        assert_eq!(Some(pos(1, 1)), pos(0, 0).offset(1, 1));
+        assert_eq!(Some(pos(0, 2)), pos(0, 0).offset(0, 2));
+        assert_eq!(None, pos(0, 0).offset(-1, 1));
+        assert_eq!(None, pos(0, 0).offset(0, 1));
+        assert_eq!(None, pos(7, 7).offset(1, 1));
+
+        assert_eq!(Some(pos(1, 1)), Position::midpoint(pos(0, 0), pos(2, 2)));
+        assert_eq!(Some(pos(1, 3)), Position::midpoint(pos(0, 2), pos(2, 4)));
+        assert_eq!(None, Position::midpoint(pos(0, 0), pos(1, 1)));
+    }
 }