-
Notifications
You must be signed in to change notification settings - Fork 42
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
Set 'handicap' from GoEngine.computeScore to the handicap bonus #146
Set 'handicap' from GoEngine.computeScore to the handicap bonus #146
Conversation
This commit changes `GoEngine.computeScore()` to return `0` for `handicap` when it does not contribute to the score. Indirectly, this changes the scoring summary in the web-client to not show the handicap in this case. This fixes an inconsistency in the API/return, which forwarded to the web-client and caused user confusion. Previously it was neither consistently equal to the handicap (see AGA) nor the handicap scoring bonus (see Japanese/Korean/New Zealand). - For Chinese/Ing, it was both the handicap scoring bonus and the handicap, which happen to be equal. - For AGA, it was the handicap scoring bonus, which is `handicap-1`. - For Japanese/Korean/New Zealand, it was the handicap (scoring bonus is 0). Now, for all rulesets, it's the handicap scoring bonus (if any). For a client computing the score, this is what's relevant. This is similar to how `stones` is returned for rulesets where it's relevant to the score. It might be nice for the web-client to show the number of handicap stones somewhere in the main view, but (a) it wasn't doing that previously and (b) `computeScore()` isn't the right way to get that info. Fixes online-go/online-go.com#1469
8524a18
to
7019ead
Compare
Oops; forgot to |
This sounds great! Maybe the English on that line should say "Handicap bonus" to reduce confusion. |
Maybe... seems like it'd be better to consider separately, since the only user-visible changes in this commit are to hide the line when it doesn't apply. (In any case, that'd be a change in the web-client.) |
Hmm, if I'm recalling correctly we had that in there for rules that it didn't matter because some vocal users wanted to quickly see the handicap stones given in the game. |
Well, it's the wrong number for AGA, so it's not serving that purpose well. Maybe we can add the number of handicap stones somewhere? |
Adapt to Goban change in online-go/goban#146 and show the actual handicap stones for black separately from the handicap bonus. To make it clear that this handicap field is not part of the score: - Use a `<hr/>` to separate it - Add `+` in front of scores that are supposed to sum together. Also move white's handicap bonus to the end, after komi. - This means that there's always a `+` in front of it, even at the beginning of the game, so it's implicitly clear that there's a bonus. - I considered changing the text to `Handicap Bonus` but that wrapped. Relates to online-go#1469
|
Thanks, good work @dexonsmith |
Behavior changed in online-go#146 (Set 'handicap' from GoEngine.computeScore to the handicap bonus)
This commit changes
GoEngine.computeScore()
to return0
forhandicap
when it does not contribute to the score. Indirectly, this changes the scoring summary in the web-client to not show the handicap in this case.This fixes an inconsistency in the API/return, which forwarded to the web-client and caused user confusion.
Previously it was neither consistently equal to the handicap (see AGA) nor the handicap scoring bonus (see Japanese/Korean/New Zealand).
handicap-1
.Now, for all rulesets, it's the handicap scoring bonus (if any). For a client computing the score, this is what's relevant. This is similar to how
stones
is returned for rulesets where it's relevant to the score.It might be nice for the web-client to show the number of handicap stones somewhere in the main view, but (a) it wasn't doing that previously and (b)
computeScore()
isn't the right way to get that info.Fixes online-go/online-go.com#1469