Skip to content

/Cataclysm-DDA

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

Convert more code to use new time classes #23327

Merged
merged 31 commits into from Apr 9, 2018

Conversation

Reviewers

@ZhilkinSerg ZhilkinSerg

Assignees
No one assigned
Labels
(P2 - High) Code: Astyle / Optimization [C++]
Projects
None yet
Milestone
No milestone
2 participants
@BevapDin @ZhilkinSerg
@BevapDin
Copy link
Contributor

commented Mar 30, 2018

Beside changing the type of some variables to time_point / time_duration, this changes calendar::print_time:

  • renamed to to_string (as it does not print like the curses functions).
  • moved the part that was printing in special format into "weather.cpp", as it's only used there.

@ZhilkinSerg ZhilkinSerg self-assigned this Mar 31, 2018

@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 31, 2018

Something is bad with time display, when using any watches:

game start:

image

3 steps north:

image

more steps:

image

image

@ZhilkinSerg ZhilkinSerg removed their assignment Mar 31, 2018

@ZhilkinSerg ZhilkinSerg added the [C++] label Apr 2, 2018

@BevapDin BevapDin force-pushed the BevapDin:ryt branch Apr 2, 2018

@ZhilkinSerg ZhilkinSerg added <Enhancement / Feature> Code: Astyle / Optimization (P2 - High) and removed <Enhancement / Feature> labels Apr 2, 2018

@ZhilkinSerg ZhilkinSerg self-assigned this Apr 3, 2018

@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 3, 2018

I'm pretty sure there was another commit with fixes to display of seconds after I provided screenshots. Have you removed it?

@BevapDin BevapDin force-pushed the BevapDin:ryt branch Apr 4, 2018

@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 4, 2018

Seconds are now displayed as multiples of 10, but there were multiples of 6 previously:

image

Was it intended?

BevapDin added some commits Jan 1, 2018

@BevapDin
Change submap::turn_last_touched to be a time_point. 
Also change its name to just `last_touched` as one does not need to know that it is (currently) based on the turn number.
dd9f318
@BevapDin
Replace usage of calendar::turn_of_year with time_past_new_year
796c541
@BevapDin
Change weather_generator::get_weather to take a time_point
741a5a0
@BevapDin
Change weather_generator::get_weather_conditions to take a time_point
c4a8eac
@BevapDin
Move include of "calendar.h" from "weather.h" into the cpp file. 
Add required forward declarations instead.
21c7b63
@BevapDin
Change get_rot_since to take time_point values.
a0a0253
@BevapDin
Use time_point and time_duration in messages.cpp
e056ed6
@BevapDin
Split calendar::print_time into two functions. 
It was nearly always called with the default value for the parameter, so we now have a function that behaves the same, but does not take any parameter.
And the one place that called it with a different value now calls the other function instead.
a7d75ee
@BevapDin
Move calendar::print_time_just_hour as free function into weather.cpp 
Change the parameter to be `time_point`.

Removed references to the game options as this is *only* used in the weather report which follows a strict format.
e20c012
@BevapDin
Change map actualizing code to use time_duration as parameters.
3f13be3
@BevapDin
Change map::fill_funnels to take a time_point
c04fed6
@BevapDin
Change retroactively_fill_from_funnel to take time_point
6bdf4e4
@BevapDin
Change musical_instrument_actor::description_frequency to time_duration.
7940306
@BevapDin
Change islot_comestible::spoils to be time_duration. 
Note that the `assign` function takes care of the conversion from a plain number of hours in JSON to `time_duration` via the `factor` parameter (1 hours in this case).
93264c8
@BevapDin
Change item::rot to be a time_duration
55c929e
@BevapDin
Change item::last_rot_check to be a time_point
f33a740
@BevapDin
Change get_rot_since to return time_duration
45d7fe9

@BevapDin BevapDin force-pushed the BevapDin:ryt branch to c06b59a Apr 7, 2018

@BevapDin

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Apr 8, 2018

Was it intended?

It wasn't. I'm switching between two notebooks, one older than the other (a full build takes some hours), and was mixing up the branches.
@ZhilkinSerg

ZhilkinSerg approved these changes Apr 9, 2018
View changes

@ZhilkinSerg ZhilkinSerg removed their assignment Apr 9, 2018

@ZhilkinSerg ZhilkinSerg merged commit b4a409f into CleverRaven:master Apr 9, 2018
3 checks passed

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 23.602%
Details
gorgon-ghprb Build finished.
Details

@BevapDin BevapDin deleted the BevapDin:ryt branch Apr 10, 2018

@ZhilkinSerg

ZhilkinSerg reviewed Apr 11, 2018
View changes

src/weather_gen.cpp
@@ -42,8 +42,7 @@ w_point weather_generator::get_weather( const tripoint &location, const calendar
double A( raw_noise_4d( x, y, z, modSEED ) * 8.0 );
double W;

const double now( double( t.turn_of_year() + to_turns<int>( calendar::season_length() ) / 2 ) /
to_turns<int>( calendar::year_length() ) ); // [0,1)
const double now( time_past_new_year( t ) / calendar::year_length() ); // [0,1)

This comment has been minimized.

Sign in to view
Copy link
@ZhilkinSerg

ZhilkinSerg Apr 11, 2018

Contributor

@BevapDin: People say weather became somewhat colder (https://discourse.cataclysmdda.org/t/15260) and this line is likely candidate.

Shouldn't it be const double now( ( time_past_new_year( t ) + calendar::season_length() / 2 ) / calendar::year_length() ), actually?

@Moumix3 Moumix3 referenced this pull request Apr 17, 2018

Merged

Fix for "temperature too low" #23518

@acidia acidia referenced this pull request May 25, 2018

Merged

[DONE] Carrion monster growing and reproduction #23833

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.