Skip to content

New date format - #504

Open
MatthewForrester wants to merge 81 commits into
jamespetts:masterfrom
MatthewForrester:64-second-minute
Open

New date format#504
MatthewForrester wants to merge 81 commits into
jamespetts:masterfrom
MatthewForrester:64-second-minute

Conversation

@MatthewForrester

Copy link
Copy Markdown

New display setting for DATE_FMT_INTERNAL_MINUTE. Explanation at https://forum.simutrans.com/index.php/topic,21433.msg199126.html.

Comment thread dataobj/environment.h Outdated
DATE_FMT_INTERNAL_MINUTE = 8,
DATE_FMT_JAPANESE_INTERNAL_MINUTE = 9
DATE_FMT_JAPANESE_INTERNAL_MINUTE = 9,
DATE_FMT_64_SECOND_MINUTE = 10

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add DATE_FMT_LENGTH = 11 or something like that. (See below)

Comment thread gui/display_settings.cc Outdated
uint8 old_show_month = env_t::show_month;
sint32 current_tick = world()->get_ticks();
for( env_t::show_month = 0; env_t::show_month<10; env_t::show_month++ ) {
for( env_t::show_month = 0; env_t::show_month<11; env_t::show_month++ ) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use hardcoded 11, use DATE_FMT_LENGTH instead (see above)

Comment thread gui/display_settings.h Outdated
{
private:
char time_str[10][64];
char time_str[11][64];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use DATE_FMT_LENGTH instead of 11

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% agree. I started writing a separate test program to make sure I understood how to use maximum values in enums correctly before I changed anything in Simutrans, so it's good you have done this.

Comment thread simworld.h Outdated
// Long minutes are used by DATE_FMT_64_SECOND_MINUTE
// DBG_DEBUG("sprintf_time_secs()", "Current date format is %u", env_t::show_month);
// DBG_DEBUG("sprintf_timesecs()", "Current status of long_minutes is %d", long_minutes);
unsigned int minutes = seconds;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simply declare a variable without explicit initialisation:
unsigned int minutes;

Comment thread simworld.h Outdated
Comment on lines +2675 to +2687
unsigned int minutes = seconds;
if (long_minutes) {
minutes = seconds / 64;
}
else {
minutes = seconds / 60;
}
unsigned int hours = minutes / 60;
seconds %= 60;
if (long_minutes) {
seconds %= 64;
} else {
seconds %= 60;
}

@mariculousNyan mariculousNyan Feb 17, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those conditionals seem bloated and you kind of repeat yourself.

Better do this:

unsigned int minutes;
if (long_minutes) {
	minutes = seconds / 64;
	seconds %= 64			
}
else {
	minutes = seconds / 60;
	seconds %=60;
}
unsigned int hours = minutes / 60;
minutes %= 60;

Still, you repeat yourself. So finally get rid of that repetition:

const unsigned int seconds_per_minute = long_minutes ? 64 : 60;
unsigned int minutes = seconds / seconds_per_minute;
seconds %= seconds_per_minute;

unsigned int hours = minutes / 60;
minutes %= 60;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants