[core, moves] Improve bounds checking of Square::neighbor

Remove the <n> argument from this method. I think it was a bad idea to begin with
but at the time I was looking for an expedient solution for getting neighbor
squares 2 squares away.

Overhaul bounds checking in this method so horizontal (west and east) bounds are
checked in addition to vertical (north and south) bounds. For the diagonal
directions in particular, it was easy to generate some bogus neighbor squares
because this method didn't check for wrapping when calculating horizontal
neighbors.

core/src/coordinates.rs

@@ -308,45 +308,57 @@ impl Square {
     }
 
     #[must_use]
-    pub fn neighbor(self, direction: Direction, n: Option<i8>) -> Option<Square> {
+    pub fn neighbor(self, direction: Direction) -> Option<Square> {
-        let index: u8 = self as u8;
-
-        let n = n.unwrap_or(1);
-        let dir: i8 = direction.to_offset() * n;
-
         match direction {
-            Direction::North | Direction::NorthEast => {
+            Direction::North => {
-                Square::try_from(index.wrapping_add_signed(dir)).ok()
-            }
-            Direction::NorthWest => {
                 if self.rank() == Rank::EIGHT {
-                    None
+                    return None;
-                } else {
-                    Square::try_from(index.wrapping_add_signed(dir)).ok()
                 }
             }
-            Direction::West => {
+            Direction::NorthEast => {
-                if self.file() == File::A {
+                let (file, rank) = self.file_rank();
-                    None
+                if rank == Rank::EIGHT || file == File::H {
-                } else {
+                    return None;
-                    Square::try_from(index.wrapping_add_signed(dir)).ok()
-                }
-            }
-            Direction::SouthEast | Direction::South | Direction::SouthWest => {
-                if self.rank() == Rank::ONE {
-                    None
-                } else {
-                    Square::try_from(index.wrapping_add_signed(dir)).ok()
                 }
             }
             Direction::East => {
                 if self.file() == File::H {
-                    None
+                    return None;
-                } else {
+                }
-                    Square::try_from(index.wrapping_add_signed(dir)).ok()
+            }
+            Direction::SouthEast => {
+                let (file, rank) = self.file_rank();
+                if rank == Rank::ONE || file == File::H {
+                    return None;
+                }
+            }
+            Direction::South => {
+                if self.rank() == Rank::ONE {
+                    return None;
+                }
+            }
+            Direction::SouthWest => {
+                let (file, rank) = self.file_rank();
+                if rank == Rank::ONE || file == File::A {
+                    return None;
+                }
+            }
+            Direction::West => {
+                if self.file() == File::A {
+                    return None;
+                }
+            }
+            Direction::NorthWest => {
+                let (file, rank) = self.file_rank();
+                if rank == Rank::EIGHT || file == File::A {
+                    return None;
                 }
             }
         }
+
+        let index: u8 = self as u8;
+        let direction = direction.to_offset();
+        Square::try_from(index.wrapping_add_signed(direction)).ok()
     }
 }
 
@@ -561,37 +573,37 @@ mod tests {
     fn valid_neighbors() {
         let sq = Square::E4;
 
-        assert_eq!(sq.neighbor(Direction::North, None), Some(Square::E5));
+        assert_eq!(sq.neighbor(Direction::North), Some(Square::E5));
-        assert_eq!(sq.neighbor(Direction::NorthEast, None), Some(Square::F5));
+        assert_eq!(sq.neighbor(Direction::NorthEast), Some(Square::F5));
-        assert_eq!(sq.neighbor(Direction::East, None), Some(Square::F4));
+        assert_eq!(sq.neighbor(Direction::East), Some(Square::F4));
-        assert_eq!(sq.neighbor(Direction::SouthEast, None), Some(Square::F3));
+        assert_eq!(sq.neighbor(Direction::SouthEast), Some(Square::F3));
-        assert_eq!(sq.neighbor(Direction::South, None), Some(Square::E3));
+        assert_eq!(sq.neighbor(Direction::South), Some(Square::E3));
-        assert_eq!(sq.neighbor(Direction::SouthWest, None), Some(Square::D3));
+        assert_eq!(sq.neighbor(Direction::SouthWest), Some(Square::D3));
-        assert_eq!(sq.neighbor(Direction::West, None), Some(Square::D4));
+        assert_eq!(sq.neighbor(Direction::West), Some(Square::D4));
-        assert_eq!(sq.neighbor(Direction::NorthWest, None), Some(Square::D5));
+        assert_eq!(sq.neighbor(Direction::NorthWest), Some(Square::D5));
     }
 
     #[test]
     fn invalid_neighbors() {
         let sq = Square::A1;
-        assert!(sq.neighbor(Direction::West, None).is_none());
+        assert!(sq.neighbor(Direction::West).is_none());
-        assert!(sq.neighbor(Direction::SouthWest, None).is_none());
+        assert!(sq.neighbor(Direction::SouthWest).is_none());
-        assert!(sq.neighbor(Direction::South, None).is_none());
+        assert!(sq.neighbor(Direction::South).is_none());
 
         let sq = Square::H1;
-        assert!(sq.neighbor(Direction::East, None).is_none());
+        assert!(sq.neighbor(Direction::East).is_none());
-        assert!(sq.neighbor(Direction::SouthEast, None).is_none());
+        assert!(sq.neighbor(Direction::SouthEast).is_none());
-        assert!(sq.neighbor(Direction::South, None).is_none());
+        assert!(sq.neighbor(Direction::South).is_none());
 
         let sq = Square::A8;
-        assert!(sq.neighbor(Direction::North, None).is_none());
+        assert!(sq.neighbor(Direction::North).is_none());
-        assert!(sq.neighbor(Direction::NorthWest, None).is_none());
+        assert!(sq.neighbor(Direction::NorthWest).is_none());
-        assert!(sq.neighbor(Direction::West, None).is_none());
+        assert!(sq.neighbor(Direction::West).is_none());
 
         let sq = Square::H8;
-        assert!(sq.neighbor(Direction::North, None).is_none());
+        assert!(sq.neighbor(Direction::North).is_none());
-        assert!(sq.neighbor(Direction::NorthEast, None).is_none());
+        assert!(sq.neighbor(Direction::NorthEast).is_none());
-        assert!(sq.neighbor(Direction::East, None).is_none());
+        assert!(sq.neighbor(Direction::East).is_none());
     }
 
     #[test]

moves/src/generators/knight.rs

@@ -54,9 +54,11 @@ impl Iterator for KnightMoveGenerator {
             if (target_bitboard & self.friends).is_populated() {
                 self.next()
             } else if (target_bitboard & self.enemies).is_populated() {
-                Some(Move::capture(origin, target).into())
+                let ply = Move::capture(origin, target);
+                Some(ply.into())
             } else {
-                Some(Move::quiet(origin, target).into())
+                let ply = Move::quiet(origin, target);
+                Some(ply.into())
             }
         } else {
             self.current_origin = None;

moves/src/generators/pawn.rs

@@ -118,34 +118,38 @@ impl PawnMoveGenerator {
     fn calculate_origin_square(&self, target: Square) -> Option<Square> {
         match self.move_type {
             MoveType::SinglePushes => match self.color {
-                Color::White => target.neighbor(Direction::South, None),
+                Color::White => target.neighbor(Direction::South),
-                Color::Black => target.neighbor(Direction::North, None),
+                Color::Black => target.neighbor(Direction::North),
             },
             MoveType::DoublePushes => match self.color {
-                Color::White => target.neighbor(Direction::South, Some(2)),
+                Color::White => target
-                Color::Black => target.neighbor(Direction::North, Some(2)),
+                    .neighbor(Direction::South)?
+                    .neighbor(Direction::South),
+                Color::Black => target
+                    .neighbor(Direction::North)?
+                    .neighbor(Direction::North),
             },
             MoveType::LeftCaptures => match self.color {
-                Color::White => target.neighbor(Direction::SouthEast, None),
+                Color::White => target.neighbor(Direction::SouthEast),
-                Color::Black => target.neighbor(Direction::NorthWest, None),
+                Color::Black => target.neighbor(Direction::NorthWest),
             },
             MoveType::RightCaptures => match self.color {
-                Color::White => target.neighbor(Direction::SouthWest, None),
+                Color::White => target.neighbor(Direction::SouthWest),
-                Color::Black => target.neighbor(Direction::NorthEast, None),
+                Color::Black => target.neighbor(Direction::NorthEast),
             },
             MoveType::EnPassant => match self.color {
                 Color::White => {
                     if (self.en_passant & self.left_captures).is_populated() {
-                        target.neighbor(Direction::SouthEast, None)
+                        target.neighbor(Direction::SouthEast)
                     } else {
-                        target.neighbor(Direction::SouthWest, None)
+                        target.neighbor(Direction::SouthWest)
                     }
                 }
                 Color::Black => {
                     if (self.en_passant & self.left_captures).is_populated() {
-                        target.neighbor(Direction::NorthWest, None)
+                        target.neighbor(Direction::NorthWest)
                     } else {
-                        target.neighbor(Direction::NorthEast, None)
+                        target.neighbor(Direction::NorthEast)
                     }
                 }
             },