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

Fix #423 (node blunder colors in the default theme) #802

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kaorahi
Copy link
Contributor

@kaorahi kaorahi commented Oct 30, 2020

I'm afraid most users may not notice the feature #413 because of the issue #423. This PR applies the proposed parameters in #423 (comment) if they are empty. It works only once so that users can still set them empty again if they like.

related: #800 #801

kaorahi pushed a commit to kaorahi/lizzie that referenced this pull request Nov 2, 2020
@kaorahi kaorahi mentioned this pull request Nov 2, 2020
@hope366
Copy link

hope366 commented Aug 31, 2021

I have been using lizzie with this PR for quite some time.
Right now, when I put that lizzie in the official lizzie 0.7.4 folder and started it, the winning percentage change node was left blank.
In the official lizzi 0.7.4, the winning percentage change node is blank by default, so I thought this PR would work. Is this recognition wrong?

@kaorahi
Copy link
Contributor Author

kaorahi commented Aug 31, 2021

thx. It seems broken for some reason. Please apply 64dc629 to fix it.

When you test it, you need to delete (or rename) both files config.txt and persist.

kaorahi pushed a commit to kaorahi/lizzie that referenced this pull request Aug 31, 2021
@hope366
Copy link

hope366 commented Aug 31, 2021

I tried the modified version and it seems to work fine. Thank you very much.
When I started lizzie after deleting "config.txt", an error occurred because the part of the engine command that should be "./leelazero/leelaz.exe" is "./leelaz". It worked normally by rewriting it correctly.
無題

@kaorahi
Copy link
Contributor Author

kaorahi commented Sep 1, 2021

I found the above "some reason". The name of the default theme is inconsistent between Config.java ("default") and ConfigDialog.java ("Default" in English, or i18n names in other languages).

So 64dc629 can fail in non-English environments. The correct fix should be cd14ee8. Namely, we do not need to check the default theme here because we never touch theme/yasnaya/theme.txt anyway in checkEmptyBlunderThresholds from validateAndCorrectSettings.

cd14ee8 should work when all the following conditions are satisfied.

  1. "blunder-winrate-thresholds" and "blunder-node-colors" are both empty in config.txt.
  2. "checked-empty-blunder-thresholds" is not true in persist.

kaorahi pushed a commit to kaorahi/lizzie that referenced this pull request Sep 1, 2021
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