Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear engine suggestion arrows after reaching checkmate or stalemate #724

Merged
merged 1 commit into from
May 28, 2024

Conversation

timmcca-be
Copy link
Contributor

@timmcca-be timmcca-be commented May 27, 2024

Summary

This ensures that we emit an evaluation from UCIProtocol once checkmate or stalemate has been reached, addressing #717.

Stockfish sends the message info depth 0 score mate 0 when checkmate is reached and info depth 0 score cp 0 when stalemate is reached. Before this change, these messages were being filtered out (by an explicit check for checkmate and because nodes and time were missing for stalemate), so no evaluation was emitted. This led to the analysis board retaining a stale list of "best moves" and suggesting nonsense.

An alternative approach would be to allow for the stale "best moves" list, and just hide the arrows if the game is over. I prefer the approach taken in this PR, however, because it respects Stockfish as the source of truth for the suggestions.

This assumes that there are no cases in which it is important to filter out a message from Stockfish because it did not include nodes or time. If such cases do exist, please let me know.

Repro steps

  • Go to Tools > Analysis board
  • Click Start
  • Play Fool's Mate (1. f3 e6 2. g4 Qh4#)

Screenshots

Before

After

@veloce
Copy link
Contributor

veloce commented May 28, 2024

Thanks for this PR!

This assumes that there are no cases in which it is important to filter out a message from Stockfish because it did not include nodes or time. If such cases do exist, please let me know.

I'm not able to answer this question. Not sure Stockfish output is a 100% consistent across versions also.

I'd say your fix is more logic, so let's give it a go?

@veloce veloce merged commit 78d936a into lichess-org:main May 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants