jordanbray / chess

A rust library to manage chess move generation
https://jordanbray.github.io/chess/
MIT License
243 stars 55 forks source link

Joining forces #6

Closed niklasf closed 7 years ago

niklasf commented 7 years ago

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:

Some missing features:

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

jordanbray commented 7 years ago
  1. We ran into each other once on github via the python-chess libray. Hello again!

  2. The whole reason I developed this crate was to prevent duplication of work. As a result, I definitely think we should join forces on some of these things.

  3. I would be completely fine with adding support for other chess variants, so long as it didn't affect performance on the main variant. I haven't looked at your implementation of how you did that yet.

  4. I noticed that your implementation had UCI code in it. I was planning a separate chess engine communication library, that depends on the 'chess' crate. I definitely want to keep all the chess things organized, and as a result I definitely would want to keep the UCI/XBoard stuff separate.

niklasf commented 7 years ago

Cool. How do you feel about switching to nightly Rust and adding some #[bench] tests like these, so we can make sure that patches don't decrease performance?

Edit: associated_consts would be another nice unstable feature to have. But if you need stable support that's ok, of course.

jordanbray commented 7 years ago

There are actually a few things from unstable that I want, but I'm trying to stick with stable for now. To take #[bench] as an example, the entire framework has been unstable since rust 1.1, and has had very little work done on it since. I'd be fine with using something like bencher, however. I'm not sure exactly where I'd use associated_consts. Edit: I saw you were using this: #[cfg(target_feature="bmi2")] I will definitely be using that too once I can safely do it on stable.

jordanbray commented 7 years ago

Some general thoughts:

  1. I think 960 should probably have first-class support in the current Board struct. The only thing that I think would need to change is the castling rules. IIRC, if a 960 game is started with the normal board position, the rules of the game are exactly the same.
  2. I think there should probably be some sort of BoardTrait (named something better than that), that handles chess variants. Then, each variant can implement the same trait. At the end, if needed, we could have an enum AnyVariant { Chess(x), ThreeCheck(x) ... } that impls BoardTrait, and variant users could have easier access with that.
  3. Perhaps variants could go into another crate, and have them have chess as a dependency?
  4. UCI/XBoard stuff should go into another crate. I started writing some code there, but XBoard was a terrible protocol and so I'm stuck in analysis paralysis. Longer Term Goals (of the project, from my perspective)
  5. chess_syzygy crate.
  6. chess_pgn crate (maybe fork chess_pgn_parser, which already exists?)
  7. chess_network crate(s) (eg. lichess.org public API, FICS, ICC, or whatever else has an open API).
  8. Full support for UCI and XBoard.
  9. A simple interface to write a chess GUI.
  10. A simple interface to write a chess engine.

Edit: Thoughts?

niklasf commented 7 years ago

That looks like a solid plan.

There's one thing that worries me: I just tried (as an experiment) to factor out the very basics (Square, Bitboard, ...) into their own crate: https://github.com/niklasf/rust-chess/commit/8ab2084561b20e6830d5af612a70da022fd027e4. This was a huge performance hit (6x slower). It seems that many optimizations don't work across crate boundaries. Not sure if there's anything that can be done about it ...

niklasf commented 7 years ago

Update: Looks like #[inline] includes code in the crate metadata to allow inlining across crates. In my experimental benchmark it achieves the same performance again. We just potentially have to put #[inline] on a lot of stuff. (As opposed to #[inline(always)] the compiler still can decide not to actually inline).

So with that I am happy to move forward ... first steps for me would be to polish Square and Bitboard and add Chess960 support.

jordanbray commented 7 years ago

It almost has to be non-inlined functions that is causing problems. I'll run valgrind when I get home on that branch and see if I can pinpoint the issue.

jordanbray commented 7 years ago

I'm not sure I like the structure of the chess_base module that has been created. One of the problems I immediately note is that many of the modules are not generic enough to support some chess variants. (For example, some variants are on an 8x10 board). I'm not sure that separating out the squares/ranks/files/bitboards/etc from move generation for the base game is really gonna be helpful. I think a better method may be to add lots of traits to the existing module. I think the best place to start will be some sort of Board trait, and some sort of MoveGen trait. I will work on this some this weekend I think. I'm really aiming to write a MoveGen trait very soon to get rid of some ugly code there right now.

niklasf commented 7 years ago

Yeah, the chess_base module was just an experiment to see how such a thing would perform, not intended to actually merge.

Btw. I am only intrested in variants played on an 8x8 board. It's ok if you want to be even more general, but then we might have to switch to a move generation method other than bitboards.

niklasf commented 7 years ago

Ok ... I think I can close this as we agree on the general direction. I'll post a patch every now and then.