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

All cooked food turns rotten immediately #4805

Closed
ianestrachan opened this issue Dec 1, 2013 · 8 comments

Comments

Projects
None yet
4 participants
@ianestrachan
Copy link
Contributor

commented Dec 1, 2013

Reported and confirmed many times in: http://smf.cataclysmdda.com/index.php?topic=4584.0

It seems to be some collision between the (hot) and (rotten) tags.

@EricFedrowisch

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2013

This could be a pretty cool feature if it were from a catastrophic cooking skill check failure.

@ianestrachan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 2, 2013

I think you'd just end up with a lump of charcoal in that case.

@EricFedrowisch

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2013

Yeah, that makes more sense. Or another flag (Disgusting) . Perhaps it is burnt, maybe it is too salty, or too poorly spiced. The food itself IS edible and won't make you sick, but it tastes just AWFUL and is bad for your morale than usual (or worse if it was already bad for morale).

@EricFedrowisch

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2013

That is probably just the normal revivification process as the goo revives the dead meat. Lol.

@ianestrachan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 2, 2013

As I mentioned, and others said in the forum thread, it appears to treat hot food as being rotten.

@tung

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

In game::complete_craft() food items are given a "fuzzy birthday", which sets the newly-created item's bday to the next 6-hour clock time in the future, e.g. any food made after 6 AM up to midday will have their bday set to midday for stacking purposes. This means that freshly-made food items often have their bday set in the future.

Meanwhile, item::rotten() works roughly like this:

int expiry;
it_comest* food = dynamic_cast<it_comest*>(type);
if (food->spoils != 0) {
    expiry = (int)g->turn - bday;
    return (expiry > food->spoils * 600);
}

With future bday, this causes expiry here to be negative. If we look at the definition of it_comest, we see this:

struct it_comest : public itype
{
    // ...
    unsigned int spoils;   // How long it takes to spoil (hours / 600 turns)
    // ...
}

This causes a signed-unsigned comparison with item::rotten(), which "promotes" expiry into an unsigned int, which turns it into a huge positive number in the comparison when expiry is actually negative. While expiry is negative, item::rotten() incorrectly returns true. Once bday is no longer in the future, expiry is positive and item::rotten() works as intended. I've confirmed all of this in gdb.

The quick fix to this bug is this:

return (expiry > (int)food->spoils * 600);

The better fix would be to remove -Wno-sign-compare from the build flags and fix all of the suppressed warnings in the code, since this was what was covering this bug up. Alternately, avoid the use of unsigned types unless absolutely necessary, since they tend to lead to bugs like this.

@ianestrachan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2013

I'm not entirely sure that 'spoils' needs to be an unsigned int, either, given that a signed int would hold enough hours for 89 million days, plus change.

Awesome analysis, and good bug-hunting, @tung.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Dec 11, 2013

This has been fixed.

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.