- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 310
 
Implement cloud eval only for broadcast #1469
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
Conversation
| 
               | 
          ||
| typedef EngineGaugeParams = | ||
| ({ | ||
| bool isLocalEngineAvailable, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this field because it didn't seem useful as ref.watch(engineEvaluationProvider).eval will simply have a null value if the local engine is not available and it allows me to use the method currentEval that I created to centralize the logic to choose the current eval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can remove this. Because we want to either:
- always display the gauge and engine lines if the server evals are available
 - and if they are not, display the gauge and line only if local eval is enabled
 
The reason is to avoid showing/hiding the gauge and lines when jumping to another position to avoid any board shift.
a405930    to
    c8e5f65      
    Compare
  
    | }) = _EvaluationOptions; | ||
| } | ||
| 
               | 
          ||
| Eval? pickBestEval({ | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment to explain the logic behind this would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added comments to explain why there are these functions and what are their parameters but the logic is a bit annoying to explain and not necessary I think
| 
               | 
          ||
| typedef EngineGaugeParams = | ||
| ({ | ||
| bool isLocalEngineAvailable, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can remove this. Because we want to either:
- always display the gauge and engine lines if the server evals are available
 - and if they are not, display the gauge and line only if local eval is enabled
 
The reason is to avoid showing/hiding the gauge and lines when jumping to another position to avoid any board shift.
f58ad50    to
    1281f94      
    Compare
  
    1281f94    to
    a1ca1ef      
    Compare
  
    
It should be only reviewed after #1467 as this a follow-up to this PR (only the second commit is new).