-
Notifications
You must be signed in to change notification settings - Fork 72
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
Parse stats from api.php in Grey You #640
Conversation
Codecov Report
@@ Coverage Diff @@
## main #640 +/- ##
============================================
- Coverage 21.41% 21.40% -0.01%
Complexity 9205 9205
============================================
Files 999 999
Lines 156154 156166 +12
Branches 34713 34714 +1
============================================
- Hits 33433 33431 -2
- Misses 116524 116537 +13
- Partials 6197 6198 +1
Continue to review full report at Codecov.
|
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'm approving this - although I'd like a test - because you're fixing a major issue. And the new code is clearly not going to change anything outside of Grey You.
In your ample free time, consider writing a test for CharSheetRequest.parseStatus(JSON).
long rawmuscle = 0; | ||
long rawmysticality = 0; | ||
long rawmoxie = 0; | ||
if (KoLCharacter.inGreyYou()) { |
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 initially wondered why we had to look at the static global, considering that "path" was right in the JSON, but then I realized that was our convention; when parsing the JSON from api.php, we parse it in AccountRequest first - which sets the ascension path - before CharSheetRequest and CharPaneRequest. I wish that ordering dependency was documented in ApiRequest.parseStatus.
rawmuscle = JSON.getLong("rawmuscle"); | ||
rawmysticality = JSON.getLong("rawmysticality"); | ||
rawmoxie = JSON.getLong("rawmoxie"); | ||
} | ||
|
||
KoLCharacter.setStatPoints(muscle, rawmuscle, mysticality, rawmysticality, moxie, rawmoxie); |
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.
Seems like there could be a test in CharSheetRequestTest for this method. There are tests for parseStatus(String html), but not for parseStatus(JSONObject JSON).
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.
Looks like my next task is to learn how test writing works.
No description provided.