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]Eating code rewrite #15057

Merged
merged 9 commits into from
Feb 1, 2016

Conversation

Coolthulhu
Copy link
Contributor

Motivation:

  • player::eat is huge, ugly, full of giant nearly-identical if blocks, hard to maintain, and didn't get a solid cleanup in a while
  • Eating code is just plain hard to read at times - it is understandable "locally", but when something doesn't work, you probably won't be able to tell why and where it breaks until you scan the function from the top to the bottom
  • The player is not informed about food being "bad" until they actually make an attempt to consume it
  • The code could be moved to Character with a relatively low amount of work (compared to the rewrite itself). NPCs will want to eat some time, so that distant player/npc split should keep eating code accessible for both.
  • A lot of the code - all allergies, for example - could be easily yanked out to jsonable structures. Moving them to arrays on top of the file will make it more obvious and easier.

Changes to code itself:

  • Moved the eating code to a new file: consumption.cpp
  • Changed all the if( carnivore && eaten->made_of(fruit) && eaten->made_of(veggy) ... ) blocks to use vectors of banned and allowed foodstuffs. Much easier to maintain
  • Changed all the big allergy block into an array: array contains trait, food type and morale id. Adding new allergies would be much cleaner (although currently the morale penalties are hardcoded)
  • Cleaned up cannibal code. Removed redundant trait checks, sorted ifs by bonus amount (most positive on the top)
  • Untangled some food capacity code. Instead of special-casing it per trait, traits only set the capacity which is enforced later
  • Split the eating code into 2 sections: first one checks and asks the player, the second one eats food confirmed edible or forced to be eaten
  • Moved capacity if blocks to a function: stomach_capacity

Changes to game mechanics:

  • Removed some usages of digestion bionics allowing ignoring mutations. It is good enough without letting you skip trait stuff
  • Allowed sapiovores to feel happy from cannibalism
  • Prevented gourmand and the like from affecting move cost of drug consuption
  • Made human flesh not edible for herbivores

Problems:

  • Code is still tangled, many mechanics are in wrong places (slimespawner)
  • Can't move it to Character yet because of morale effects and addictions
  • Hibernation. Everything related to this gimmicky post-thresh mutation is just a problem. I think I'm going to rip out most of the code related to it and just make it slow down metabolism while sleeping.
  • Slimespawner is implemented hackily, retaining old functionality would be a bad idea here

TODO:

  • Get this stuff to compile and work
  • Add food coloring in UI based on edibility
  • Fix hibernation
  • Fix slimespawner

@chaosvolt
Copy link
Contributor

Hmm, interesting. Some of the inefficiency could also be mitigated by allowing for traits/mutations to specify in their JSON entries what materials are forbidden.

That doesn't simplfy the many glorious interactions between traits like for how human flesh is handled though, or for "picking out the inedible bits" sorta things. Plus even then it's beyond the scope of this PR, which looks like a hefty amount of changing already. ^^"

interactive = false;
}

const char *itname = food.tname().c_str();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tname returns a temporary, c_str does as well. Both are per invalid after the full expression. In other words: don't store the result of c_str, store the result of tname as string (not reference).

@kevingranade
Copy link
Member

If one or a few of these mechanics REALLY get in the way of cleaning things up, I'm on board to just ditch them.

@Coolthulhu
Copy link
Contributor Author

I managed to unwrap it, looks like it was just scattered all over the place rather than actually complex.

@Zireael07
Copy link
Contributor

What the heck is slimespawner doing in eating code?

@kevingranade
Copy link
Member

kevingranade commented Jan 29, 2016 via email

@Coolthulhu Coolthulhu changed the title [WiP][CR]Eating code rewrite [CR]Eating code rewrite Jan 29, 2016
@Coolthulhu
Copy link
Contributor Author

I think it's ready. I tested what I expected to go wrong and the new code seemed to work correctly.

Removed double prompt for "you're full, eat it?". Before, you could get this message once for being full (over-nutrition) and once for going over capacity (over-nutrition or over-slaking).
Now answering "yes" to the first one skips the second and gourmand-type (gourmand, hibernation, slimesplitter and sometimes hyperactive metabolism) mutations will prevent the first prompt in many cases.

Added (or restored?) a message on pizza scraping.

I fixed some minor bugs with original code:

  • Hunger threshold thing (integer rounding made it far too quantized)
  • Capacity oddities with gourmand+hibernation and plant+fertilizer

Minor balancing changes:

  • Herbivores can't use meat/eggs to induce vomiting. That was a very minor "feature"
  • Herbivores can't use stuff with meat/eggs to get morale or positive effects. That's way less minor, but it mirrors carnivore. Herbivore is still less crippling than carnivore. Plus, it makes sense from the lore point of view: herbivore says "you can't keep this revolting stuff down", so it shouldn't give a morale bonus
  • Vomiting chance on overfilling is a bit different due to different order
  • Uncapped bad health from unhealthy food - it should only affect heavy drug users, but it SHOULD affect them
  • Prevented vomiting from removing not actually consumed food from stomach. For example, in master, if you take a bite out of an ant egg, can't eat more, try to eat another and vomit, you will lose the entire nutrition for both eggs as if you ate them and vomited them up, resulting in "very hungry"

There is some unfortunate code duplication that could (should?) be yanked out in some later PRs:

  • Determining whether food is eaten, drunk or "consumed" - solids and thick soups are eaten, liquids drunk, rest is drugs. This should probably be an item method.
  • Defining overeating in one place
  • Hibernation thresholds. Currently hardcoded to -60
  • Gourmand-type checks for vomit prevention
  • Two places for nutrition calculation: nutrition_for and consume_effects. nutrition_for is shown in item description and it could get pizza scraping modifier at the very least

There are some minor inconsistencies in handling specific "food"stuffs: fertilizer and alcohol metabolism are handled in iuse, making them skip some checks. Not a problem, but player won't be warned that some of that extra nutrition will go to waste on overconsumption. Also won't be lost on vomiting.

@@ -7,6 +7,7 @@ src/basecamp.cpp
src/cata_utility.cpp
src/char_validity_check.cpp
src/clzones.cpp
src/consumption.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants