Cataclysm-dda: Perishable food is good forever

[BevapDin]

Describe the bug

Item info shows various items (e.g. flour) as being "perishable", but having a nominal shelf life of "about forever".

These seem to be mutually exclusive properties. Perishable means it will eventually go bad, but shelf life of forever means it will never go bad. So what is it?

Steps To Reproduce

Start game, spawn flour item, look at it's information.

Expected behavior

Perishable food has a shelf life of less than forever, or food with a shelf life of forever is not classified as perishable.

[TechyBen]

There is an english word for this. "Indefinite". As in, long without a defined ending. "lasting for an unknown or unstated length of time."

No idea if it would apply. But would basically mean "a long time, but undefined on the exact timescale".

Or would "almost forever" be closer?

[JayThirtySeven]

I've also seen it display "less than forever". Looking in calendar.cpp, the text descriptor "forever" is applied when the time in turns is greater than INDEFINITELY_LONG which is defined as 1% of the maximum value of an int.

As an experiment I commented out the cases where it returns "forever" and the flour showed a shelf life of 3 seasons. I think this is probably a result of the 1 second turn change; there are now six times more turns in three seasons and the code wasn't originally tested that way.

Doubly confusing, INDEFINITELY_LONG has a comment saying that it is defined as 1% of max int so that it doesn't overflow when converted to turns... but the comparison that selects "forever" treats INDEFINITELY_LONG as though it is already in turns.

[l29ah]

I've seen more than forever even.

[JayThirtySeven]

On second examination I think I was confusing "turns" and "moves". I don't know enough about the time keeping in the game to figure out exactly what is going on, but it seems like all the relevant code is in calendar.cpp. I wonder if it would make sense to just remove the "forever" result... if the code is being called to display the duration that means it's a finite amount of time. Why would we ever want to report "forever" when we could report an actual amount of time? Even if it's 250 years or something I don't know why the player would mind seeing the real number.

[FulcrumA]

Visually, I'd prefer if game treated items lasting above certain time as non-perishables. Far easier to manage at a glance what's what, it's certain such items will stack and so on.

[brannamm]

I looked through the code and it seems that the root of the problem (which several others have already alluded to) is that calendar::INDEFINITELY_LONG is only 248 days after the 1 second turns change, and it used to be 1488 days. That means that anything with a 360 day spoils_in tag will now display forever in various places where it didn't before.

I am going to look into updating calendar::INDEFINITELY_LONG to be a time_duration instead of an int. There are only 9 usages of calendar::INDEFINITELY_LONG and most just convert it to a time_duration anyway or convert the other time_duration into turns.

Also, as I am writing this comment out I found a code comment that says // TODO: change INDEFINITELY_LONG to time_duration, so I'm glad that someone else had the same idea as me.

[brannamm]

During implementation of the change I found that there are 2 types of usages of this. One is intending the constant to be a large time_duration and the other is using it as a large number of turns. The latter is used widely by assign_activity and in order to entirely convert calendar::INDEFINITELY_LONG to a time_duration I would have to change a parameter of assign_activity which has many usages from an int to a time_duration. Instead of fully converting calendar::INDEFINITELY_LONG, there will now be 2 constants - one representing a large duration for use in comparing other durations, and the other for representing a large number of turns.

I think this implementation introduces the least risk by changing the least code and clarifies the intended usage of the constants.