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 bugs and inconveniences around blunder thresholds table #801

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

Conversation

kaorahi
Copy link
Contributor

@kaorahi kaorahi commented Oct 30, 2020

See the commit logs for details. As this PR includes various changes, it will be better to test it for a while before merging.

@hope366
Copy link

hope366 commented Oct 31, 2020

Thank you for the detailed correction.
I didn't really understand how to use blunder color, and I shied away from it, but in the process of applying and validating your PR, I came to understand how interesting this feature is.
Now, I want to check the following two items

  • Fix IndexOutOfBoundsException when one try to empty blunder thresholds
  • Quick & dirty patch for NumberFormatException in blunder thresholds

Before application : If you leave a threshold blank or enter a double-byte value, the next time you check, the value will not be displayed, but the value that was previously set will be displayed.

After applying: If you leave the threshold blank or enter a double-byte value, the next time you check, it will be displayed as "-777.0".

Is it correct that the above PR is intended to be this kind of content?

When I enter a threshold in full-width characters, it looks like it's accepted at the time, but hitting the space bar doesn't turn the analysis on or off. This may confuse users who are not familiar with computers.
I think it would be easier to understand if an error message is displayed to let the user know that it is an incorrect input when you enter a threshold in full-width characters and click "ok".

@kaorahi
Copy link
Contributor Author

kaorahi commented Oct 31, 2020

Thanks for testing. Yes, I intend to convert invalid thresholds into a nonsense value -777.0.

Your proposal is desirable of course. But it needs additional coding that I'm not willing to do. So I made this "Quick & dirty patch". Without this patch, even one mistake make you lose ALL the changes in the table when you click the OK button.

Would you post a new issue for your above request if it is important for you? Though I don't have enough energy now, I or someone may possibly implement it in future.

@hope366
Copy link

hope366 commented Oct 31, 2020

What I suggested is very trivial and I don't think it's important enough to raise any new issues.

By the way, I have noticed something in the course of this examination of blunder color.
I experimented with the official release of Lizzie-v0.7.4 with the following configuration
2
1
Black's first move is -4.9% compared to the best move, so the node should be green in color. Similarly, white's second move should be red at -14.7% and black's third move should be yellow at -5.2%.
However, any of the moves are light blue, indicating -1.0% to -3.0%.
This problem seems to have been fixed after applying Fix missing marks and color in the variation tree.

@hope366
Copy link

hope366 commented Nov 2, 2020

Mistakes were made due to lack of verification.
#800 is probably not relevant.
The key to determining whether it would work or not seemed to be the order in which the thresholds were entered.
The thresholds must be entered in order of decreasing, and if you specify -10 followed by -20, then -20 is ignored.
I mean...
tiguhagu
In this case, orange and red are ignored.

matigai
With this setting, everything but the light blue will be ignored.

Sort blunder thresholds before recording applies, the thresholds you enter are automatically sorted in decreasing order, so it seems to work no matter what order you enter them in.

@kaorahi
Copy link
Contributor Author

kaorahi commented Nov 2, 2020

I read the document of JTable and noticed that we can simply store Double into it. So I pushed another commit for better behavior.

@kaorahi kaorahi mentioned this pull request Nov 2, 2020
@hope366
Copy link

hope366 commented Nov 2, 2020

If you enter a character that is not a half-width number, the cells become red borders and you cannot enter blunder color.
This makes it easier to see that there was a problem with the input method.😄
無題

@hope366
Copy link

hope366 commented Nov 3, 2020

It seems a bit underwhelming that the best move, or a move close to it, is represented by colorless = white. I think it's good that the best moves have the appropriate color on them.
Is there a better way to do this?
無題1無題2

@kaorahi
Copy link
Contributor Author

kaorahi commented Nov 3, 2020

I pushed yet another commit so that you can use threshold=0.0 to specify the color for "normal" moves.

@hope366
Copy link

hope366 commented Nov 3, 2020

Thank you very much.
I wasn't very interested in the variation tree, but now I can enjoy it a lot more because of the various improvements!😄

kaorahi pushed a commit to kaorahi/lizzie that referenced this pull request Nov 14, 2020
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