Skip to content
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

Fixed "Evolution doesn't happen with initial day set at 30 #33450" #33487

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

ipcyborg
Copy link
Contributor

Summary

SUMMARY: Bugfixes "Evolution doesn't happen with initial day set at 30 #33450"

Purpose of change

Fixes #33450

Describe the solution

Fixed logical error: upgrade_time is in days, not in turns.
When initial_day is set, it affects start_of_cataclysm. Then ``start_of_cataclysmis added toupgrade_time` in turns, which is a bug number for 30 days. So the evolution was really postponed.

Also removed time_point to time_point cast. Or is it required somehow?

Tested for initial_day=30, spawn delay=999. Observed a Kevlar hulk and other evolved monsters immediately.

Describe alternatives you've considered

None

Additional context

I think this PR needs extensive testing.

…#33450".

`upgrade_time` is counted in days, can't add turns to it.
@alkemann alkemann added <Bugfix> This is a fix for a bug (or closes open issue) Time / Turns / Duration / Date Issues concerning any activities being too fast or too slow. Also issues about time and date ingame labels Aug 23, 2019
@ipcyborg
Copy link
Contributor Author

The build continuous-integration/travis-ci/pr failed, but I can't find the reason for it in the log.
Is it possible to re-do this check?

@BevapDin
Copy link
Contributor

BevapDin commented Aug 24, 2019

Error can be seen at https://travis-ci.org/CleverRaven/Cataclysm-DDA/jobs/575868273

/home/travis/build/CleverRaven/Cataclysm-DDA/src/dialogue.h:124:14: error: function 'talk_effect_fun_t::set_u_buy_monster' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name,-warnings-as-errors]
        void set_u_buy_monster( const std::string &monster_id, int cost, int count, bool pacified,
             ^                                     ~~~~~~~~~~
                                                   monster_type_id
/home/travis/build/CleverRaven/Cataclysm-DDA/src/npctalk.cpp:2101:25: note: the definition seen here
void talk_effect_fun_t::set_u_buy_monster( const std::string &monster_type_id, int cost, int count,
                        ^
/home/travis/build/CleverRaven/Cataclysm-DDA/src/dialogue.h:124:14: note: differing parameters are named here: ('monster_id'), in definition: ('monster_type_id')
        void set_u_buy_monster( const std::string &monster_id, int cost, int count, bool pacified,
             ^

Edit: but that's really not the fault of this PR.

@ipcyborg
Copy link
Contributor Author

ipcyborg commented Aug 24, 2019

Error can be seen at

Well, yes. But I have not changed anything about monster_type_id.
So my question is: should I fix this issue and risk merging conflict?
Or should I wait and somehow re-do that check later, when the issue will be fixed already?

@BevapDin
Copy link
Contributor

It's not the fault of this PR, so you have no need to do anything here. If you want, you can fix this in a separate PR, but otherwise, you can just ignore that error.

@jbytheway
Copy link
Contributor

Looks like the upgrade_time value itself ought to be a time_point, rather than an int, to avoid these sorts of confusions.

@ipcyborg
Copy link
Contributor Author

Changing upgrade_time to time_point leads to change these fields too: type.age_grow, type.half_life. All of them are used in monster::next_upgrade_time for upgrade calculation.

Also it could lead to change the save format, increase savegame_version and store all these fields in turns (as integers). Is this the correct way?
Or else we could leave the saving/loading in days (not changing save format, only change the in-game type). Looks kind of bad solution.

Looks complex enough to create another PR for it.

@kevingranade kevingranade merged commit d8f806f into CleverRaven:master Aug 27, 2019
@ipcyborg ipcyborg deleted the fix-33450-evo-calc branch August 27, 2019 06:25
@jbytheway
Copy link
Contributor

A separate PR is certainly reasonable.

misterprimus pushed a commit to misterprimus/Cataclysm-DDA that referenced this pull request Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Time / Turns / Duration / Date Issues concerning any activities being too fast or too slow. Also issues about time and date ingame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evolution doesnt happen with initial day set at 30
5 participants