From e2ce7782477a9c1649accd46c46cf5375fb96f51 Mon Sep 17 00:00:00 2001 From: Eryn Wells Date: Sat, 7 Jun 2025 20:06:14 -0700 Subject: [PATCH] [core, moves] Improve bounds checking of Square::neighbor Remove the 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 | 108 ++++++++++++++++++--------------- moves/src/generators/knight.rs | 6 +- moves/src/generators/pawn.rs | 28 +++++---- 3 files changed, 80 insertions(+), 62 deletions(-) diff --git a/core/src/coordinates.rs b/core/src/coordinates.rs index d8cb202..aceec8c 100644 --- a/core/src/coordinates.rs +++ b/core/src/coordinates.rs @@ -308,45 +308,57 @@ impl Square { } #[must_use] - pub fn neighbor(self, direction: Direction, n: Option) -> Option { - let index: u8 = self as u8; - - let n = n.unwrap_or(1); - let dir: i8 = direction.to_offset() * n; - + pub fn neighbor(self, direction: Direction) -> Option { match direction { - Direction::North | Direction::NorthEast => { - Square::try_from(index.wrapping_add_signed(dir)).ok() - } - Direction::NorthWest => { + Direction::North => { if self.rank() == Rank::EIGHT { - None - } else { - Square::try_from(index.wrapping_add_signed(dir)).ok() + return None; } } - Direction::West => { - if self.file() == File::A { - None - } else { - 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::NorthEast => { + let (file, rank) = self.file_rank(); + if rank == Rank::EIGHT || file == File::H { + return None; } } Direction::East => { if self.file() == File::H { - None - } else { - Square::try_from(index.wrapping_add_signed(dir)).ok() + return None; + } + } + 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::NorthEast, None), Some(Square::F5)); - assert_eq!(sq.neighbor(Direction::East, None), Some(Square::F4)); - assert_eq!(sq.neighbor(Direction::SouthEast, None), Some(Square::F3)); - assert_eq!(sq.neighbor(Direction::South, None), Some(Square::E3)); - assert_eq!(sq.neighbor(Direction::SouthWest, None), Some(Square::D3)); - assert_eq!(sq.neighbor(Direction::West, None), Some(Square::D4)); - assert_eq!(sq.neighbor(Direction::NorthWest, None), Some(Square::D5)); + assert_eq!(sq.neighbor(Direction::North), Some(Square::E5)); + assert_eq!(sq.neighbor(Direction::NorthEast), Some(Square::F5)); + assert_eq!(sq.neighbor(Direction::East), Some(Square::F4)); + assert_eq!(sq.neighbor(Direction::SouthEast), Some(Square::F3)); + assert_eq!(sq.neighbor(Direction::South), Some(Square::E3)); + assert_eq!(sq.neighbor(Direction::SouthWest), Some(Square::D3)); + assert_eq!(sq.neighbor(Direction::West), Some(Square::D4)); + 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::SouthWest, None).is_none()); - assert!(sq.neighbor(Direction::South, None).is_none()); + assert!(sq.neighbor(Direction::West).is_none()); + assert!(sq.neighbor(Direction::SouthWest).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::SouthEast, None).is_none()); - assert!(sq.neighbor(Direction::South, None).is_none()); + assert!(sq.neighbor(Direction::East).is_none()); + assert!(sq.neighbor(Direction::SouthEast).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::NorthWest, None).is_none()); - assert!(sq.neighbor(Direction::West, None).is_none()); + assert!(sq.neighbor(Direction::North).is_none()); + assert!(sq.neighbor(Direction::NorthWest).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::NorthEast, None).is_none()); - assert!(sq.neighbor(Direction::East, None).is_none()); + assert!(sq.neighbor(Direction::North).is_none()); + assert!(sq.neighbor(Direction::NorthEast).is_none()); + assert!(sq.neighbor(Direction::East).is_none()); } #[test] diff --git a/moves/src/generators/knight.rs b/moves/src/generators/knight.rs index ab8d235..96bd3bd 100644 --- a/moves/src/generators/knight.rs +++ b/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; diff --git a/moves/src/generators/pawn.rs b/moves/src/generators/pawn.rs index 613b6e8..99f5c31 100644 --- a/moves/src/generators/pawn.rs +++ b/moves/src/generators/pawn.rs @@ -118,34 +118,38 @@ impl PawnMoveGenerator { fn calculate_origin_square(&self, target: Square) -> Option { match self.move_type { MoveType::SinglePushes => match self.color { - Color::White => target.neighbor(Direction::South, None), - Color::Black => target.neighbor(Direction::North, None), + Color::White => target.neighbor(Direction::South), + Color::Black => target.neighbor(Direction::North), }, MoveType::DoublePushes => match self.color { - Color::White => target.neighbor(Direction::South, Some(2)), - Color::Black => target.neighbor(Direction::North, Some(2)), + Color::White => target + .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::Black => target.neighbor(Direction::NorthWest, None), + Color::White => target.neighbor(Direction::SouthEast), + Color::Black => target.neighbor(Direction::NorthWest), }, MoveType::RightCaptures => match self.color { - Color::White => target.neighbor(Direction::SouthWest, None), - Color::Black => target.neighbor(Direction::NorthEast, None), + Color::White => target.neighbor(Direction::SouthWest), + 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) } } },