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.

Sign up for GitHub

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

Jump to bottom

Fix empty EvalFile option #3678

Closed
wants to merge 1 commit into from
Closed

Fix empty EvalFile option #3678

wants to merge 1 commit into from

Conversation

vondele
Copy link
Member

@vondele vondele commented Aug 26, 2021

some GUIs send an empty string for EvalFile, in that case explicitly try the default name

fixes #3675

No functional change.

@vondele
Fix empty EvalFile option
d86c821 
some GUIs send an empty string for EvalFile, in that case explicitly try the default name

fixes official-stockfish#3675

No functional change.
@vondele
Copy link
Member Author

vondele commented Aug 26, 2021

This could be an alternative for #3668

@vondele vondele mentioned this pull request Aug 26, 2021
Closed
@Sopel97
Copy link
Member

Sopel97 commented Aug 26, 2021

I like this one better, the semantics here are quite intuitive.

@osteslag
Copy link

Teaser:

With this, it looks like we could have a precedent for using default values?

Such that we could also send an empty string for Thread and Hash and have reasonable/recommended default values computed ;-)

See my earlier suggestion.

@vondele
Copy link
Member Author

vondele commented Aug 27, 2021

we have default values for all of those things. We handle a case where the GUI overwrites the default.

@vondele vondele added the to be merged Will be merged shortly label Aug 27, 2021
@vondele vondele closed this in af0d827 Aug 27, 2021
@vondele vondele mentioned this pull request Aug 27, 2021
Closed
@osteslag
Copy link

You're right. This is about default values, not “recommended” values.

@joergoster
Copy link
Contributor

joergoster commented Aug 27, 2021 •
edited
Loading

we have default values for all of those things. We handle a case where the GUI overwrites the default.

At least, it should be made clear that this is only a workaround for a bug in one GUI. This bug should have been reported to the author of the GUI!
If I set EvalFile to "empty" I'd expect a different behaviour!

@vondele
Copy link
Member Author

vondele commented Aug 27, 2021

what would be the behaviour you expect?

@joergoster
Copy link
Contributor

Isn't that obvious? Switching to classical eval ...

Joachim26 pushed a commit to Joachim26/StockfishNPS that referenced this pull request Mar 28, 2023
@vondele @Joachim26
Fix empty EvalFile option
24d8c03 
some GUIs send an empty string for EvalFile, in that case explicitly try the default name

fixes official-stockfish#3675

closes official-stockfish#3678

No functional change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
to be merged Will be merged shortly
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crashing under SCID
4 participants
@vondele @Sopel97 @osteslag @joergoster