ParseToSan and TryParseToSan seem to return null for valid Moves
instantiator opened this issue · 2 comments
Describe the bug
I've been having some difficulty with the Chessboard.ParseToSan
and .TryParseToSan
methods. Despite the Move
provided passing ChessBoard.IsValidMove
, the .ParseToSan
methods return null
for the SAN string (.TryParseToSan
returns true
which leads me to believe the string shouldn't be null
).
To Reproduce
I wrote a quick unit test for it (edit: which now also contains the fix I'm proposing because I didn't link to a specific commit)
Expected behaviour
I'm expecting the returned string to contain the SAN equivalent for the move, based on the state of the current board.
Additional context
Hey - hope all's well. I love the library, and I've adopted it for my current project: instantiator/consensus-chess-engine as it's lightweight, and does everything I need (notwithstanding this particular issue). I'd be glad to dig a little deeper and see if I can help figure out what's happening, but I suspect you'd be able to go faster...
Ok, I've debugged and I think I understand. To a degree it's my misunderstanding, but I think there's an opportunity here to do the expected thing. The last 2 lines of SanBuilder.TryParse are relevant:
move.San = builder.ToString();
return (true, null);
It sets move.San
(which makes sense), but I think if it also set out san
to builder.ToString()
then the SAN would bubble up through the other *ParseToSan
methods, too.
I'll PR a small change. Hope that's ok. 🤞
Here's the PR: #6