Skip to content

Conversation

@johndoknjas
Copy link
Contributor

@johndoknjas johndoknjas commented Feb 26, 2025

This PR is intended to only affect the evaluation shown by the running engine, and temporarily the evals given to moves in the notation (until refreshing). I could be wrong though.

For why we might want to consider this:

  • Open a game you've played in the analysis board, and request a computer analysis.
  • Once it's done, toggle on the SF 16 engine, and set the number of lines to at least 2.
  • For many moves, you should see the main bold evaluation stay where it is for a few seconds, and then suddenly switch to the evaluation of the top line.

The reason for this is that the SF 17 server analysis is used for the main bold eval, until the client engine reaches 4 million nodes. But for the user this feels confusing; especially if they're using SF 16 on their end, the abrupt change in evaluation can be large.

This isn't as much of a problem when the number of lines is just 1, since in that case no evaluation is displayed for the line. However, it's still not fully candid, since initially the main evaluation in bold won't necessarily correspond with the current line given.

@ornicar ornicar requested a review from Copilot February 27, 2025 09:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR updates the evaluation logic used in the client engine to avoid displaying the server evaluation. Key changes include:

  • Using optional chaining to simplify the conditional check in the threat button.
  • Replacing the complex getBestEval logic with a simplified one-liner that prioritizes the client evaluation.

Reviewed Changes

File Description
ui/ceval/src/view/main.ts Simplified evaluation selection and cleanup of unused serverNodes.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

ui/ceval/src/view/main.ts:119

  • The new implementation of getBestEval always returns evs.client if it is truthy, bypassing the previously detailed logic that considered mate values and node counts. Confirm that this simplified behavior is intended for the client engine evaluation.
export const getBestEval = (evs: NodeEvals): EvalScore | undefined => evs.client || evs.server;

ui/ceval/src/view/main.ts:116

  • The variable serverNodes is now unused after simplifying getBestEval. Consider removing it to keep the code clean.
const serverNodes = 4e6;

@ornicar ornicar merged commit 5f5d02f into lichess-org:master Feb 27, 2025
3 checks 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