lichess-org / mobile

Lichess mobile app v2
GNU General Public License v3.0
1.1k stars 150 forks source link

Clear engine suggestion arrows after reaching checkmate or stalemate #724

Closed timmcca-be closed 1 month ago

timmcca-be commented 1 month ago

Summary

This ensures that we emit an evaluation from UCIProtocol once checkmate or stalemate has been reached, addressing https://github.com/lichess-org/mobile/issues/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

Screenshots

Before

After

veloce commented 1 month ago

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?