jordanbray / chess

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

Add from_string function to ChessMove #27

Closed UlisseMini closed 4 years ago

UlisseMini commented 4 years ago
use chess::{ChessMove, Square, Piece};

let mv = ChessMove::new(Square::E7, Square::E8, Some(Piece::Queen));

assert_eq!(ChessMove::from_string("e7e8q".to_owned()).expect("Valid Move"), mv);

This my my first PR to a rust project, I'm not sure if I should bump the minor version in Cargo.toml or not.

jordanbray commented 4 years ago

I'll try to take a look at this over the weekend. I've been pretty busy this week. It doesn't look like it can really break anything, though.

I have a duplicate of this function in a (currently) unpublished repo called chess_uci, which deals with all data structures and formats needed for UCI communication, which I'll get around to publishing at some point. That said, it may be more appropriate for the function to be here rather than there, since squares, FEN strings are all parsed via this library.

UlisseMini commented 4 years ago

Thanks for responding! I'd love for you to make the unfinished chess_uci public so I can contribute.

That being said, I think something as basic as parsing a UCI move should be in this crate, as fen and squares are handled (Square::from_string) I feel like UCI moves should be handled too.

As for the rest of the data structures involved with UCI communication I would love to see that in another crate! And i'll gladly contribute :)

PS: I added some tests for from_string, they should be covering everything but I might have forgotten something /shrug

jordanbray commented 4 years ago

I added this as a FromStr implementation, instead of a static from_string function. Additionally, FromStr has been implemented for most types, and ChessMove also has a from_san(&Board, &str) implementation for parsing SAN moves.

I'm not merging this as is, but I did copy-paste your code directly for the function, just used &str and Result instead of String and Option, and FromStr seems more idiomatic. Also, I had a few local changes for use in the chess_uci crate I was talking about, so I was afraid to hit the merge button.

I will push out a new version (probably 3.2.0) during this week sometime. I think I'm too afraid to check that box at this exact moment.

UlisseMini commented 4 years ago

Alright great :)