-
Notifications
You must be signed in to change notification settings - Fork 173
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
Add method to modify game time properly #190
Conversation
gameMinute += minutes; | ||
gameHour += gameMinute / 60 - (backwards ? 1 : 0); | ||
|
||
// Black mathgic? No. If the value is negative we want to wrap it around |
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.
magic?
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.
maths + magic
The unit tests require GameWorld and GameState, which are currently only available when RW_TEST_WITH_DATA is set (which it's not on the CI servers), so I've had to move the tests inside the #if -- even though they don't require any data. |
This is good. Only 1 issue I can see: pressing [ doesn't roll the clock back an hour, it only alters the minutes. It looks like the test cases are correct and pick up this issue too. |
@danhedron I don't understand what you mean. [ will move the clock back 30 minutes. If the clock is less than xx:30, the hour will be decremented. This is shown to work in the tests. |
I get this output from the tests:
|
@danhedron I force-pushed, please see if it works now? The previous code shouldn't have worked anywhere so I'm not sure why my tests passed. |
Works for me. Tests are passing locally. +1 |
BOOST_CHECK_EQUAL(0, gw.getHour()); | ||
BOOST_CHECK_EQUAL(0, gw.getMinute()); | ||
|
||
gw.offsetGameTime(8*60 + 25); |
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.
Note that this is the only test case which goes forward by non-30 minute increments.
As we have 2 directions (+ and -) this is not enough to test all cases (do we know that -25 works?).
We should have more cases which give a remainder if 60 is divided by the minutes and we should have that in both directions (+ and -). I didn't think about this very much tho, so maybe it's useless.
Only found nits (see diff comments above). Tested this locally - works fine. I'd like to have an option to reset or control the seconds too, but it'd be beyond the scope of this PR and I'm not sure how one could implement it easily either. LGTM. |
...and make the time-cheaty debug keys not under/overflow the clock.