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

[CR] Skip unnecessary weather calculations for root cellar #41324

Merged
merged 1 commit into from
Jul 16, 2020
Merged

[CR] Skip unnecessary weather calculations for root cellar #41324

merged 1 commit into from
Jul 16, 2020

Conversation

int-ua
Copy link
Contributor

@int-ua int-ua commented Jun 15, 2020

Summary

SUMMARY: Performance "Skip unnecessary weather calculations for root cellar"

Describe the solution

Checking for root cellar in item::process_temperature_rot in addition to pos.z and avoiding weather for it.

Additional context

Old description follows:


Summary

SUMMARY: Bugfixes "Equalize root cellar and underground spoilage rates"

Purpose of change

#24083 description gives impression that root cellars must be equivalent to storing food underground yet they were not exactly equal.

Describe the solution

Checking for root cellar in item::process_temperature_rot in addition to pos.z. This change was ported from my local fork of 0.E branch.

Testing

Only on my 0.E local branch. It worked as intended there.

Additional context

https://discourse.cataclysmdda.org/t/spoilage-in-root-cellars-is-faster-than-underground-z-1/23956

@anothersimulacrum anothersimulacrum added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Mechanics: Temperature Freezing, cooling, thawing, heating, etc. mechanics labels Jun 15, 2020
@Hirmuolio
Copy link
Contributor

Hirmuolio commented Jun 15, 2020

The root cellar temperature is applied few lines later at https://github.com/CleverRaven/Cataclysm-DDA/blob/master/src/item.cpp#L8718
So adding this check shouldn't change anything (unless I am missing something). Though it would skip the weather calculations that are not needed for root cellars so maybe minor performance increase.

Root cellar items should rot at same rate as underground items. If that is not the case it should be investigated.

Items spawned from mapgen have some random variance in their initial rot level (IIRC +-20%) so you should test with items spawned from debug menu.
Enabling debug mode displays exact rot value and temperature in the item info.

@int-ua
Copy link
Contributor Author

int-ua commented Jun 15, 2020

applied few lines later

I thought so too at first, but it looks like it's the other way around: underground gets treated differently and therefore has lower total temperature. I tested with wished-for items both before the change and after in the same way. And before the change items spoiled a bit faster in root cellar while after it they spoiled at exactly* the same rate. But yes, I don't fully understand how it works yet.

exact rot value and temperature

Didn't notice it, I'll re-test tomorrow again if it's not yet clear by that point.

[*] I noticed differences of +- several turns only.

@Hirmuolio
Copy link
Contributor

Hirmuolio commented Jun 15, 2020

Edit: Disregard this. My test world had its time set to before start of game which makes items behave in very strange ways.

Edit 2: Tested with proper world. Yeh root cellar did nothing. Item on ground and item in root cellar had same rot value after 2d off map.

@int-ua
Copy link
Contributor Author

int-ua commented Jun 15, 2020

Oh, I forgot that my fork that I tested it on had your ice lab fix that I had to backport because it was omitted in 0.E-# updates for some reason despite me mentioning only that change in request for 0.E updates on Discord. Not sure if it affected anything. Can't test right now but I'm on #CDDA-Dev IRC channel if you have some interesting details to share real-time.

@Hirmuolio
Copy link
Contributor

Found the problem

When map is loaded (player returns from far away location) the item gets processed without the temperature flag.

@int-ua
Copy link
Contributor Author

int-ua commented Jun 16, 2020

I'll re-purpose this PR as just an optimization.

@int-ua int-ua reopened this Jun 16, 2020
@int-ua int-ua changed the title Equalize root cellar and underground spoilage rates [CR] Skip unnecessary weather calculations for root cellar Jun 16, 2020
@Maleclypse
Copy link
Member

Is this PR ready for review?

@kevingranade kevingranade merged commit 552ccba into CleverRaven:master Jul 16, 2020
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) [C++] Changes (can be) made in C++. Previously named `Code` Mechanics: Temperature Freezing, cooling, thawing, heating, etc. mechanics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants