jordanbray / chess Goto Github PK

Hi, recently I started another general purpose chess lib (https://github.com/niklasf/shakmaty) and found your crate only now. Now I wonder how to move forward without duplicating too much work.

Your crate is superior here:

• Your perft is twice as fast (probably mostly due to the compact board representation, avoiding lazy_static for attack tables and unchecked indexing).
• You have selective move generation.

Some missing features:

• Shakmaty parses moves in SAN and UCI notation.
• Shakmaty supports a bunch of chess variants from lichess.org: Standard chess, Chess960, Crazyhouse, King Of The Hill, Giveaway, Three-Check, Horde, Atomic, and Racing Kings.
• Arguably safer/cleaner APIs.

Now ... you may or may not want (all of) these features, which certainly come at the cost of some complexity. Before I continue with this essay ... what's your stance on this?

Best,
Niklas

At present, there are two ways to generate legal moves.
The first is to use board.enumerate_moves(...), which is the fastest, but requires you to pass in an empty array of 256 ChessMove's to mutate, and also does not give you an iterator or anything nice to work with.
The second way is to use the MoveGen object, which has a lot of nice features, but is slightly slower for raw perft tests (it's actually probably faster in a chess engine, though, which is the only time performance will really matter).
So, the question is, do I delete enumerate_moves(...) and just let other people be faster, or do I leave in the code duplication for the sake of speed?
It's an open question for now.

I'm planning on writing a chess engine in rust myself, and looking for a move generator to use for it. I've narrowed it down to your library and Pleco. Do you have any benchmarks between them? Looking at this benchmark result (https://github.com/niklasf/shakmaty#benchmarks), it seems like yours is a lot faster than Stockfish, and since Pleco is a clone of Stockfish, I'm assuming yours is also faster than Pleco? Thanks for your help in advance!

I would like to use the library with my own Bitboard (and maybe even different bitboard layouts), .

Would it be possible to make Bitboard a trait?

Or is the representation you are using fully tied to the algorithm implementations?

Hello, im trying to wrap my head how i should accuire the material for Black, Respective White using a given board?

Removing en passant moves doesn't work:

fn main() {
    let board = "8/8/5k2/8/3Pp3/8/2K5/8 b - d3 0 1".parse().unwrap();
    let mut moves = chess::MoveGen::new_legal(&board);
    let en_passant = "e4d3".parse().unwrap();
    moves.remove_move(en_passant);
    assert!(moves.find(|&mv| mv == en_passant).is_none()); //Assertion fails
}

The reason is because the code for removal makes the assumption that each SquareAndBitBoard has a unique source square, as it masks out the move and then returns:

EDIT: This is all wrong :/. Take a look at the last comment.

Hi, I'm trying to use your library but it seems to stall whenever I attempt to compile:

wssmts@laptop:~/Programs/chess-ai > cargo build
   Compiling chess v3.1.1 (https://github.com/jordanbray/chess.git#a09032e3)
    Building [===================================================>     ] 35/38: chess(build)

It just get's stuck there... When I run strace on it it seems to be sitting in some sort of deadlock:

wssmts@laptop:~/Programs/chess-ai > strace cargo build
<SNIP>
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
futex(0x5636fae9da98, FUTEX_WAIT_PRIVATE, 0, {tv_sec=0, tv_nsec=499991284}) = -1 ETIMEDOUT (Connection timed out)
futex(0x5636fae9da20, FUTEX_WAKE_PRIVATE, 1) = 0
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
futex(0x5636fae9da98, FUTEX_WAIT_PRIVATE, 0, {tv_sec=0, tv_nsec=499998831}) = -1 ETIMEDOUT (Connection timed out)
futex(0x5636fae9da20, FUTEX_WAKE_PRIVATE, 1) = 0
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
ioctl(2, TIOCGWINSZ, {ws_row=57, ws_col=212, ws_xpixel=1908, ws_ypixel=1026}) = 0
... <CONTINUES FOREVER>

I've tried a bunch of different versions, including pointing directly to your repo, and found that I can compile 0.3.4 but 0.4.0 introduces the stalling behavior for me. I've also tried compiling with the nightly compiler and the stable compiler, both exibit the same behavior.

The strange thing is that I tried cloning your repo and building it standalone, and this works:

wssmts@laptop:~/Programs/chess > cargo build
   Compiling arrayvec v0.5.2
   Compiling nodrop v0.1.14
   Compiling chess v3.1.1 (/home/ANT.AMAZON.COM/wssmts/Programs/chess)
warning: `#[inline]` is ignored on function prototypes
  --> src/movegen/piece_type.rs:17:5
   |
17 |     #[inline(always)]
   |     ^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_attributes)]` on by default

    Finished dev [optimized + debuginfo] target(s) in 7.56s

Any ideas? (The cargo verbose flag isn't very useful, just spits out the same info)

Environment details:

wssmts@laptop:~ > rustc -V
rustc 1.41.0 (5e1a79984 2020-01-27)
wssmts@laptop:~ > rustup show
Default host: x86_64-unknown-linux-gnu
rustup home:  /home/ANT.AMAZON.COM/wssmts/.rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu
1.38.0-x86_64-unknown-linux-gnu
1.41.0-x86_64-unknown-linux-gnu (default)

installed targets for active toolchain
--------------------------------------

aarch64-unknown-linux-musl
x86_64-unknown-linux-gnu
x86_64-unknown-linux-musl

active toolchain
----------------

1.41.0-x86_64-unknown-linux-gnu (default)
rustc 1.41.0 (5e1a79984 2020-01-27)

Hey there, thanks for creating and maintaining this crate. I picked it to experiment with classical AI algorithms and practice rust. I have faced some usability issues, which might be in part due to me not understanding how best to handle it:

1. Iterating over pieces: Board::pieces() method is what I would expect to give an iterator over all pieces present on the board. I want to implement a heuristic value of the board based on sum of weighted pieces for each side. This method currently gives BitBoard, and I have no idea how to use it. Would you be interested to add a method Board::pieces_iter( &self ) -> Iterator<(Piece, Color, Square)> ?

My current workaround is to map from characters in Board::to_string() to (Piece, Color, Square), you can guess it is kind of ugly :)

2. Not sure if it is sensible to expect this, but MoveGen does not implement choose to get a random turn. Again, my purpose of practicing rust can be served by adding this if you would merge such PR.

i tryed to copy the fen example from https://de.wikipedia.org/wiki/Forsyth-Edwards-Notation

let board = Board::from_fen("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1".to_string()).expect("build board from fen");

i expected that this should just work, but instead the code panics.

Dear chess maintainers, hi @jordanbray,

Thanks for the chess crate, I found it at the Rust docs at https://docs.rs/chess/3.2.0/chess/.

I did have to guess that the Rust docs are generated from this crate, which is the problem I suggest to solve: add a link to this GitHub repo in the documentation. In that way, any user can find the source code of the crate easily.

For me, I wanted to find the source code to see an example of how Rust is done properly. I think a chess game is of nice intermediate complexity :-)

I hope you will help out me and my fellow rustlings. Thanks and cheers, Richel Bilderbeek

Since Board.status only needs to check if there are legal moves or not, it is slow and pointless to calculate all legal moves, especially since this would often be in hot code areas (search/eval functions).

Perf says Board.status takes around a third of my engine's running time, so this is an issue.

Edit: Board.status takes around 20% of my engine's running time with this change.

The documentation of MaybeUninit::as_mut_ptr mentions that

Reading from this pointer or turning it into a reference is undefined behavior unless the MaybeUninit<T> is initialized.

This is exactly what happens in all usages of MaybeUninit throughout the library:

Hello,

Currently the square returned by Board::en_passant is on the fifth or the fourth rank, which is not standard.

What is expected to be returned by the Board::en_passant is the en passant target square which is on the third or sixth rank (in the example given by the docs it should be e6 instead of e5).

This change would also make checking if a given move is en passant easier.

The en passant target square is the way en passant is set by FEN, I think we should follow the standard.

Of course that would be a breaking (albeit small) change.

Thanks for your time developing and maintaining this library.

There is a bug in Game::can_declare_draw()

#[test]
pub fn test_can_declare_draw(){
    let moves = 
               "1. Nc3 d5 2. e3 Nc6 3. Nf3 Nf6 4. Bb5 a6 5. Bxc6+ bxc6 6. Ne5 Qd6 7. d4 Nd7
                8. f4 Nxe5 9. dxe5 Qg6 10. O-O Bf5 11. e4 Bxe4 12. Nxe4 Qxe4 13. Re1 Qb4
                14. e6 f6 15. Be3 g6 16. Qd4 Qxd4 17. Bxd4 Bh6 18. g3 g5 19. f5 g4 20. Rad1
                Rg8 21. b3 Rb8 22. c4 dxc4 23. bxc4 Rd8 24. Kg2 Rc8 25. Bc5 Rg5 26. Rd7 Bf8
                27. Rf1 a5 28. Kg1 a4 29. Bb4 Rh5 30. Rf4 Rg5 31. Rf1 Rh5 32. Rf4 Rg5 33.
                Ba5";
    let game = moves.split_whitespace()
        .filter(|s| !s.ends_with("."))
        .fold(Game::new(), |mut g, m| {g.make_move(san_move_to_move(&g.current_position(),m)); g});
    assert!(!game.can_declare_draw());
}

It thinks a draw can be declared after 33. Ba5, but that is not the case.

When I want to set up a certain position, the only way I can see to do this, is to use Board::from_fen(). However it would be nice to be able to set up a position without creating a FEN string first (since this is slow and tedious). I would suggest a function like the following:

pub fn setup(
    pieces: impl Iterator<Item=(Square, Piece)>,
    side_to_move: Color,
    white_castle: CastleRights,
    black_castle: CastleRights,
    en_passant: Option<Square>)
    -> Option<Board> {}

The implementation could be the same as for Board::from_fen (except for the string handling). Do you agree that this is a good idea? Would you accept a pull request for this?

The Board.legal() function fails at recognizing legal moves/pins after Board.null_move() is called. Take the following example position. The positions start exactly the same way except for the side to move. In the white example, it considers Ra2 illegal as it should. However in the exact same position, with white to move (after a null move from black), it considers Ra2 legal.

extern crate chess;

use chess::*;

fn main() {

	let pos_white     = Board::from_fen("4k3/3r4/8/8/8/8/3R4/3K4 w - - 0 1".to_owned()).unwrap();
	let mut pos_black = Board::from_fen("4k3/3r4/8/8/8/8/3R4/3K4 b - - 0 1".to_owned()).unwrap();

	let testmove = ChessMove::new(
		Square::from_string("d2".to_owned()).unwrap(),
		Square::from_string("a2".to_owned()).unwrap(),
		None);

	// Ra2 should fail because of the pin
	assert!(pos_white.legal(testmove) == false);

	// Same exact position, but black has the move.
	// Let's change the side to move to white via null_move
	pos_black = pos_black.null_move().unwrap();

	// Make sure white is the side to move
	assert!(pos_black.side_to_move() == Color::White);

	// Ra2 should be illegal, because the position is exactly the same as pos_white
	assert!(pos_black.legal(testmove) == false); // assertion failed
}

The reference for type representations states that

Nominal types without a repr attribute have the default representation. Informally, this representation is also called the rust representation.

There are no guarantees of data layout made by this representation.

However, the Rank and File types rely on a defined layout due to the use of std::mem::transmute:

Looks like MoveGen misses moves when a mask is set.
Here is my test code:

let board = Board::from_str("r1bqkb1r/pp3ppp/5n2/2ppn1N1/4pP2/1BN1P3/PPPP2PP/R1BQ1RK1 w kq - 0 9").unwrap();

let mut capture_moves = MoveGen::new_legal(&board);
let targets = *board.color_combined(!board.side_to_move());
capture_moves.set_iterator_mask(targets);

for m in capture_moves{
    print!("{}, ", m);
}

It only prints this move:

f4e5,

lib version is 3.2.0, which is not reflected by the repo

There appears to be a bug in move generation. This piece of code panics:

let board = Board::from_fen("rnbqkbnr/ppp2pp1/4p3/3N4/3PpPp1/8/PPP3PP/R1B1KBNR b KQkq f3 0 1".to_string()).unwrap();

let move_gen = MoveGen::new_legal(&board);

For the first example in README.md, we have this code:

// lets iterate over targets.
let targets = board.color_combined(!board.side_to_move());
iterable.set_iterator_mask(targets);

Notice that the target should be a &BitBoard type. However, set_iterator_mask accepts mask: BitBoard which is not a pointer. Therefore, building the project gives me this error:

mismatched types [E0308] expected `BitBoard`, found `&BitBoard` 

If I dereference target using *, it works.
Can you recheck if the example is correct?

Sorry for the empty description originally, I fat-fingered the return key before I had the chance to type anything.

Really appreciate the hard-work you and others have put into this crate.

There were a couple of features added after the last release; of particular interest to me would be the implementation of ForStr for ChessMove, but commit 6693cac in particular looks packed with improvements as well.

Is there a target date for the next release?

remove_move and remove_mask truncate moves in the move generator:

fn main() {
    let board = "8/8/5k2/8/3Pp3/8/2K5/8 b - d3 0 1".parse().unwrap();
    let mut moves = chess::MoveGen::new_legal(&board);
    let len = moves.len();
    moves.remove_move("e4e3".parse().unwrap());
    assert_eq!(moves.len(), len - 1); //Assertion fails with 0 != 8
}

fn main() {
    let board = "8/8/5k2/8/3Pp3/8/2K5/8 b - d3 0 1".parse().unwrap();
    let mut moves = chess::MoveGen::new_legal(&board);
    let len = moves.len();
    moves.remove_mask(chess::BitBoard::from_square(chess::Square::E3));
    assert_eq!(moves.len(), len - 1); //Assertion fails with 0 != 8
}

This is because remove_move and remove_mask fail to uphold the invariant where no non-empty SquareAndBitBoards may come after an empty one. This invariant is mentioned here:

Why is it allowed to pass in a number greater than 63 to make a new Square? Is this the reason why it is unsafe and thats contaminates everything else? Is it a lot faster compared to just defaulting to make_square instead? If so, how big is the speed difference?

Just some questions I had if anyone can help me understand.

It produces a lot of errors, starting with:

~/.cargo/bin/cargo check --color=always --all --all-targets
   Compiling chess v2.0.2
error[E0433]: failed to resolve: use of undeclared type or module `gen_tables`
 --> ~/.cargo/registry/src/github.com-1ecc6299db9ec823/chess-2.0.2/src/gen_tables/bmis.rs:6:5
  |
6 | use gen_tables::magic_helpers::{magic_mask, questions_and_answers, NUM_MOVES};
  |     ^^^^^^^^^^ use of undeclared type or module `gen_tables`

That code would build on Rust 2015, but version 2.0.2 is marked as Rust 2018.

Version 2.0.1 (the latest version committed to this repo) is still Rust 2015 and it works fine.

I need to implement a move sorter for chess engine developers.
This will need to take in a PV, a list of killer moves, a function to sort captures and a fuction to sort quiet moves.
It will need to iterate in the following order:

• PV (iff PV is legal, passed in by user)
• Good Captures (sorted by user supplied function, determined good capture by same function)
• Killers (iff legal, passed in by user)
  ** Remove all killers from move list!
• Good Quiets (sorted by user supplied function, determined good by user supplied function)
• Bad Moves

It needs the ability to stop at an arbitrary point (Such as only iterating over captures for qsearch).

Hello,

First thanks for the fantastic job ! You make me able to almost (work still in progress) create an UCI engine quite rapidly.

is there a clean way to unmake a move ? The only way I could see is to truncate the game.actions() list to remove last MakeMove(ChessMove) but I think it should be a simpler way ?

The need is to implement the ponder uci feature. when entering in ponder search mode, the last move played is the ponder move (note: when playing the move on the game, we don't know yet it will be a ponder move) . As we don't have to only check for the pondered move (UIC protocol allow that), we need to unmake the move before searching.

Any advice how to implement this in other way is welcome.

Hello,

To ease the task of analysing a game (after it is played using this crate), displaying a Game using PGN could be interesting, so that it can be plugged into the Lichess analysis board for example (https://lichess.org/analysis).

I have started working on this but I am a very new Rust dev so mistakes are bound to be made. Would this project still be interested in a PR for this?

Since the Game struct is already not as efficient as Board and not recommended for chess engines, I went for something very simple, but not so efficient: implementing Display for the Game struct, iterating over all the actions, ignoring every MakeMove except ChessMove, applying the move in a Board, copied from starting_pos (so that I can easily get the Piece that is in each Square from a ChessMove), and with this creating the full PGN string.

If another approach should be preferred, I would be interested in details and I'll try to do it!

Thanks a lot,
yzoug

fn main() {
    chess::MoveGen::new_legal(&"3k4/8/PPPPPPPP/8/PPPPPPPP/8/PPPPPPPP/3K4 w - - 0 1".parse().unwrap());
}

leads to

thread 'main' panicked at 'assertion failed: len < A::CAPACITY', C:\Users\analog_hors\.cargo\registry\src\github.com-1ecc6299db9ec823\arrayvec-0.5.2\src\lib.rs:265:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\chess-test.exe` (exit code: 101)

on debug mode. Compiling for release disables the assertion and causes undefined behavior instead.
Should probably either prevent non-standard boards from getting created (which sounds tricky since I doubt this is the only way to get one) or account for this during move generation.

I'm using your crate (thank you for writing it!) for a chess engine and encountered the following problem: The Zobrist hash does not take into account which side is to move. This is vital information though, for instance this fails:

use chess::Board;
assert_ne!(
    Board::from_fen("8/q7/1q6/2k5/5K2/6Q1/7Q/8 w - - 0 1".into()).unwrap().get_hash(),
    Board::from_fen("8/q7/1q6/2k5/5K2/6Q1/7Q/8 b - - 0 1".into()).unwrap().get_hash());

However the first position is winning for white, the other is winning for black.

But–since both positions have the same hash–the transposition table gave nonsensical results, which confused my engine a lot.

(While we're at it: Is there a reason that from_fen takes a String and not a &str?)

fn main() {
    println!("{}", chess::get_king_moves(chess::EMPTY.to_square()));
}

prints garbage:

. . . . . . X X 
X X X . X . . .
X X . X X X X .
. . . . X . X .
. X X . X X X X
X X X X X X X .
. . . . . . . .
. . . . . . . .

BitBoard::to_square creates an out of bounds Square(64) on an empty bitboard. This can be used to access out of bounds memory.
Another somewhat irrelevant thing that can be considered is making Square an enum instead. This would allow the compiler to automatically elide all bounds checks when indexing by Squares. It would allow it to be used for niche space optimizations.

Hello,

Rust 1.50 panics when running this simple example (chess version: 3.1.1)

use chess::{Board, ChessMove, Square};

fn main() {
    let board = Board::default();
    board.make_move_new(ChessMove::new(Square::E2, Square::E4, None));
}

Traceback:

thread 'main' panicked at 'attempted to leave type `chess::Board` uninitialized, which is invalid'
stack backtrace:
   0: rust_begin_unwind
             at /rustc/1c389ffeff814726dec325f0f2b0c99107df2673/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/1c389ffeff814726dec325f0f2b0c99107df2673/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/1c389ffeff814726dec325f0f2b0c99107df2673/library/core/src/panicking.rs:50:5
   3: core::mem::uninitialized
             at /rustc/1c389ffeff814726dec325f0f2b0c99107df2673/library/core/src/mem/mod.rs:659:9
   4: chess::board::Board::make_move_new
             at /home/lucas/.cargo/registry/src/github.com-1ecc6299db9ec823/chess-3.1.1/src/board.rs:857:35
   5: simple_engine::main
             at ./src/main.rs:6:13
   6: core::ops::function::FnOnce::call_once
             at /rustc/1c389ffeff814726dec325f0f2b0c99107df2673/library/core/src/ops/function.rs:227:5

Apparently, it is not happy with the mem::uninitialized() in board.rs

pub fn make_move_new(&self, m: ChessMove) -> Board {
    let mut result = unsafe { mem::uninitialized() };
    self.make_move(m, &mut result);
    result
}

Edit: probably fixed by 6693cac but not released yet.

Introduction

I've noticed that in all my games the FEN string always prints 0 and 1 for the half-move clock and full move counter respectively. Here's an example of a game where each rounds' FEN string is printed. Notice that each of the FEN strings has 0 1 at the end. Here's a snip of the first couple of board states FEN representation:

rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1
rnbqkbnr/pppppppp/8/8/2P5/8/PP1PPPPP/RNBQKBNR b KQkq - 0 1
rnbqkbnr/pp1ppppp/8/2p5/2P5/8/PP1PPPPP/RNBQKBNR w KQkq - 0 1
rnbqkbnr/pp1ppppp/8/2p5/2P5/7P/PP1PPPP1/RNBQKBNR b KQkq - 0 1
rnb1kbnr/pp1ppppp/8/q1p5/2P5/7P/PP1PPPP1/RNBQKBNR w KQkq - 0 1
rnb1kbnr/pp1ppppp/8/q1p5/P1P5/7P/1P1PPPP1/RNBQKBNR b KQkq - 0 1

The issue

Looking through the code, I can see the Board's Display trait implementation is just a thin wrapper around the BoardBuilder's Display trait implementation. The half-move clock and the full-move counter is hard-coded to display as 0 1. Neither the Board or BoardBuilder structs have counters to keep track of the half-move clock or full-move counter.

The consequences

My immediate reaction was to wonder how you managed to implement the can_declare_draw function on the Game struct. You seem to keep a list of Actions and essentially calculate the half-move clock from history.

Neat! So no need to keep an actual counter of the half-move clock (and full-move counter, although I don't think there's any chess logic depending on it) since you can derive it from the history you keep for other reasons. Although, this does pose a problem for writing a correct FEN string from the context available in the Board struct.

Possible solutions

I'd suggest adding counters to the Board and BoardBuilder structs to keep track of the half-move clock and full-move counter, then also refactor the can_declare_draw function to make use of these counters to improve performance.

I'm quite keen to make these changes and submit a PR myself. I'm however not sure when I'll get to it so in the mean time I'm just writing this issue to hear your thoughts.

Can ChessMove be marked to derive the Hash trait?

A common idiom in engines is something like this

fn eval(board: chess::Board) {
  // ... do stuff ...

  let color = if board.side_to_move() == Color::White { 1 } else { -1 };
  return evaluation * color;
}

It would be convenient if there was a method to get the number version of the side to move, perhaps a method on Color, such as

fn to_num(self) -> i32 {
  match self {
    Color::White => 1,
    Color::Black => -1,
  }
}

(name is debatable, but this is the idea)

The implementation of the Hash trait uses only the field 'hash' which is only affected by the pieces positions. On the other hand, the implementation of PartialEq compare all fields of the struct.
This causes unnecessary collisions in hashmap/hashset.
To solve it, the implementation of the Hash trait could simply use the get_hash() func.

I want to use a board state (represented by the Board struct in this library) as a key to a HashMap. This is for a chess engine I am writing for fun.

std::collections::Hashmap requires that its key be Eq and Hash. Both should be doable, yet are not in this library. Checking the struct definition and cross-referencing with cargo doc --open in my project reveals that the following:

• the only type which needs Eq and Hash as well is Bitboard
• Color, and Square are already Hash and Eq
• CastleRights has Hash, but no Eq (only PartialEq)

As Bitboard is just a wrapper around a u64, it seems as simple as adding Hash, Eq into the #[derive]. Once that is done, it is probably a similar matter for Board and CastleRights.

To-do List / Summary (in no particular order)

• Hash for Bitboard
• Eq for Bitboard
• Hash for Board
• Eq for CastleRights
• Eq for Board

Currently this fails:

let board = Board::from_fen("rnbqkbnr/pppp2pp/8/4pP2/8/8/PPPP1PPP/RNBQKBNR w KQkq e6 0 0".into()) .unwrap();
assert!(board.null_move().unwrap().is_sane());

The reason is that the en-passant square is left untouched by the null_move() function but should be reset.

CastleRights::from_index and CastleRights::rook_square_to_castle_rights are both unsound; They can cause undefined behavior on certain inputs while not marked as unsafe. The documentation is also misleading, instead saying that it will panic.

In the documentation I think this is supposed to be?

let mut iterable = MoveGen::new_legal(board, true);
...
let mut iterable = MoveGen::new_legal(&board);
....

It would make debugging way easier if there was a way to convert a Board into a human-readable string or at least a FEN string (which is somewhat human-readable). I think the library once contained a Display implementation for Board but that seems to have been removed.

I would suggest adding a method Board::to_fen(&self) -> String that converts the board to a FEN string but I'm open to other proposals.

   Compiling chess v3.1.1
error[E0432]: unresolved import `std::arch::x86_64`
 --> /home/pi/.cargo/registry/src/github.com-1ecc6299db9ec823/chess-3.1.1/src/gen_tables/bmis.rs:1:16
  |
1 | use std::arch::x86_64::_pext_u64;
  |                ^^^^^^ could not find `x86_64` in `arch`

If I have time i'll PR a fix, from my limited understanding I think this is only there for efficiency so it should be as simple as not importing for arm.

edit: Maybe consider using https://docs.rs/bitintr

At the moment our only option to check if a user's input move (not sanitized) is valid is to invoke the greedy board's

pub fn legal(&self, m: ChessMove) -> bool {
    MoveGen::new_legal(&self).find(|x| *x == m).is_some()
}

Are there any plans to support a safe_legal_quick validator without enumerating every possible valid move but only evaluating the provided source-destination pair?

cargo build takes ~2 minutes to run on my 2019 macbook. rust-analyzer (running in my editor — I believe on any given file-save) also wreaks havoc on my CPU when trying to edit files here. Worse yet, it appears to spin up multiple build-script async processes eagerly...

I'd love to contribute more to this project, but these costly build steps make it hard. (FWIW, I'm specifically interested in improving fn legal in board.rs)

Perhaps there's room to better separate the table generation from other source files into a core or table sub-crate?
(I was toying with Board and BoardBuilder at the time)

The experience pushed me to a composition model instead (preferable anyhow) wherein I consume the chess crate and hack up my own stuff (though there's still some code duplication), for Bughouse

The crate currently takes a very long time to build. It would probably be a good idea to feature-guard the most compile-time-consuming parts of the crate and make it possible to disable their compilation. (opt-out) or possibly even move to opt-in in 4.x.

Does marking the functions with the #[inline] attribute help with performance?
From what I understand, the compiler does not inline functions from external crates unless they are marked with #[inline], and I feel like inlining many of the functions in the library can have a noticeable impact on performance. I'm thinking mostly of the functions in magic.rs, and Board and BitBoard functions.

With 3.2.0, the following case prints test successful:

use chess::{Board, BoardStatus, ChessMove, Square};
let mut board = Board::default();

assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e4").expect("invalid: e4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e5").expect("invalid: e5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d4").expect("invalid: d4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "h5").expect("invalid: h5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d5").expect("invalid: d5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "c5").expect("invalid: c5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::new(Square::D5, Square::C6, None)); // en passant
assert_eq!(board.status(), BoardStatus::Ongoing);

println!("test successful");

whereas this case panics with invalid: dxc6: InvalidSanMove:

use chess::{Board, BoardStatus, ChessMove, Square};
let mut board = Board::default();

assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e4").expect("invalid: e4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e5").expect("invalid: e5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d4").expect("invalid: d4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "h5").expect("invalid: h5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d5").expect("invalid: d5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "c5").expect("invalid: c5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "dxc6").expect("invalid: dxc6")); // same en passant!
assert_eq!(board.status(), BoardStatus::Ongoing);

println!("test successful");

As far as I can tell, this is the same move, and valid SAN? (SAN comes from the beginning of https://www.chess.com/game/live/9695070671)

yep i see it now