-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Rework the waiting menu and fix 'calendar::print_duration()' #17116
Conversation
I agree, that's dubious, but what if it's noon, and player wants to wait till noon of the next day? |
@Night-Pryanik: I don't think it's a valid use case and it would require reworking the menu to have the option to wait for the next noon. |
@Zireael07 Example: It's 12:00, noon. I want to wait exactly 24 hours, till next noon, but now it's not possible. |
I know what you want. The old menu didn't enable this and I think it's out of scope for this PR. |
I agree with @Zireael07 that it's hardly the case. Prior to #17106 the waiting (till the noon in particular) was broken (due to an erroneous formula), but nobody complained about it (which proves that people don't like waiting :) . Even properly working, the old wait menu (prior to #17106) uses the same logic, but in a more obscure way: it doesn't grey out the menu item, but stops waiting immediately if the current hour and the desired hour are equal. In case of noon you had to use another option if the time was between 12:00pm and 01:00pm. Greyed out menu items signal that the desired time is now. I can even add an appropriate note, something like Waiting till the next noon doesn't require any significant reworking, just dropping a couple of lines maybe. If the community decides that this limitation is annoying, I could: reduce the time of disabling (say, to 10 minutes), or even drop it entirely, or replace with a pair
Now is an incorrect word in the sentence. Have you ever tried that? That wasn't possible in the previous menu either (see above), and you had to wait an hour instead of a half. |
#include "calendar.h" | ||
#include "output.h" | ||
#include "options.h" | ||
#include "translations.h" | ||
#include "game.h" | ||
#include "debug.h" | ||
|
||
// Divided by 100 to prevent overflowing when converted to turns | ||
const calendar calendar::INDEFINITELY_LONG( int( std::numeric_limits<int>::max() / 100 ) ); |
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.
Why the cast to int, shouldn't it already be an int? Also, the comment is not clear. The calendar constructor takes turns as input, there is no conversion there, did you mean moves (as in Creature::moves
)?
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.
Yes to both questions.
Needs a rebase |
It was putting bare turns into "fraction parts" of time instead of the right time values.
So that it could read durations from the helper function and optionally display them (if player has a watch). Also, introduced calendar::INDEFINITELY_LONG (self explanatory).
It enforces unnecessary coupling: switch in the function and menu numbers should be kept consistent manually.
Rebased against |
So that it:
Disables certain menu items if the target time is reached, but not yet passed (for 30 minutes).Also, this PR fixes
calendar::print_duration()
that was broken. It was putting bare turns into the "fraction parts" of the time instead of the right time values, so that you would see e.g.:12 hours and 476 minutes
. Also, it could never show you round units of time (1 hour
,1 minute
e.t.c).Before:

After:
