niklasf / shakmaty

A Rust library for chess and chess variant rules and operations
https://docs.rs/shakmaty
GNU General Public License v3.0
210 stars 41 forks source link

SanPlus::from_move needs a mutable Position? #51

Closed marcusbuffett closed 2 years ago

marcusbuffett commented 2 years ago

I'm confused by the signature of from_move for SanPlus, it takes a mut Position, which doesn't seem necessary, and is tripping me up a bit when using the library.

Looking at the source, I see this:

https://docs.rs/shakmaty/latest/src/shakmaty/san.rs.html#608

Which also confuses me, why is pos.play_unchecked(m); called in there?

My assumption would be that from_move would just take a &Position and a &Move, and then return a SanPlus, not play any moves or require a mutable Position.

Thank you for the excellent library, I wouldn't have been able to write the backend of my chess site in Rust without it.

niklasf commented 2 years ago

The implementation plays the move, to look at the resulting position, and test if the move has given check/checkmate. That's needed for the # in something like Qxf7#.

Since the caller would not expect any moves to be played, the method intentionally consumes pos, instead of taking it by reference. So what happens with the position never becomes visible to the caller. The mut is not part of the publicly visible signature.

Because of this, if (and only if) you still need pos, you have to use pos.clone(). If the method would take a reference and do the pos.clone() internally, there would be no way to avoid the clone in the other cases.

If you intend to play the move yourself anyway, for example in a loop over a sequence of moves, SanPlus::from_move_and_play_unchecked() combines both efficiently.

marcusbuffett commented 2 years ago

That makes a lot of sense why it needs to be like that. Thanks for the clarification, and thanks again for the library :)