Geras1mleo/Chess

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