-
Notifications
You must be signed in to change notification settings - Fork 1
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
Amateur golden point #76
base: master
Are you sure you want to change the base?
Conversation
Removed commented line
Renamed TotalDeuces -> totalDeuces
I forgot one if...else section in incP1 function
Added the Golden status to see when the golden happen, so both scores are G - G
Update to have Golden status available
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 added a visual way to know when there's a Golded point, I know that maybe all will change, but you know, better to have, in this way is not possible to forget
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.
finally looking again at your PR, sorry for taking this huge amount of time!
i merged the current master branch into your PR, and also added unit tests to help you. you can see the changed I added here https://github.com/pedrorijo91/garmin-padel/pull/76/commits
We just need to sort the thing with the scoreIdx whenever you have some time please
@@ -57,6 +61,10 @@ class MatchStatus { | |||
self.p1ScoreIdx++; | |||
} | |||
|
|||
function getP1Idx() { |
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 think this is somehow new. the p1Idx and p2Idx were not exposed on purpose. it's an internal data structure that should not be used by anyone else other than the matchStatus class. callers should look into the score only
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 think this change happened because we want to display the G-G score when in golden point right? i feel we should revert it. instead of doing this, can we infer we are in the G-G state by looking at the score and at the number of deuces?
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 think this is somehow new. the p1Idx and p2Idx were not exposed on purpose. it's an internal data structure that should not be used by anyone else other than the matchStatus class. callers should look into the score only
How come not here? That's the file where I found all the functions, it seemed more practical to draw from here since you have an index with all the scores.
So if I understand you correctly, you would rather create a second function that does the calculations and just returns the value? How were you planning to proceed?
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 think this change happened because we want to display the G-G score when in golden point right? i feel we should revert it. instead of doing this, can we infer we are in the G-G state by looking at the score and at the number of deuces?
Trying this feature, I realized that a visual indication is better, because it happens that the point lasts a very long time, so long that you don't remember accurately the score, which is then one of the purposes of the application. I think it's happened to you playing as well, points of a long duration that at the conclusion you don't know the score. So the ploy, you run out of deuces available, the application points to G-G which reminds you that you are at the Golden Point and the point is decisive. The same thing happens with A if you think about it.
I would take advantage of the dedicated array of scores and "play" by moving the index time by time.
How would you do that?
How do you use the tests you put in? I usually to do the tests I would run the debug and test the functions, without doing anything else. |
I update file by file manually, so I hope I modify all the files, let me know if is working, on my side is working but because I have all the file of course
closes #69