Crafting menu shows range of possible nutrients for food crafts #36172

jbytheway · 2019-12-17T01:23:45Z

Summary

SUMMARY: Interface "Crafting menu shows range of possible nutrients for food crafts"

Purpose of change

Currently when crafting a comestible the crafting GUI declares a particular calorie and vitamin content for it. But the true value that will result depends on the ingredients used. This is unhelpful and can be confusing for players.

Describe the solution

This turns out to be quite tricky in practice, for a few reasons:

The nutrient calculation is recursive, meaning that we need to know the possible content of ingredients in order to calculate the range for each product.
It's possible for there to be loops in the recipe graph such that the potential nutrient content is unbounded. Even in more reasonable loops which preserve nutrient content, it can still be the case that the code needs to search arbitrarily deep through item components in order to calculate the nutrient content. I decided to consider any loops to be a bug. Loops can be broken either by editing recipes or by marking one item in the loop with NUTRIENT_OVERRIDE.
The nutrient content depends on player traits (primarily various mutations that affect diet). In some cases the trait effect depends on the materials involved. In turn, that means that you cannot simply calculate the "true" overall nutrient content and apply traits to the final value. To get consistent results requires applying the traits at the 'leaves' of the component tree that is searched. In particular, this means that it's not practical to cache the results of this calculation; it needs to be calculated afresh each time.

The solution has a number of components:

At startup (when finalizing game data):
- Cache a list of non-obsolete recipes that can be used for each item.
- Search for loops involving comestibles in the recipe graph. Mark any such comestibles as in a loop (there are no loops in the core game data any more, but mods could add them, so the code still needs to handle the possibility). Report any loops via debugmsg so they can be dealt with.
New player::compute_nutrient_range functions. One overload for a specific recipe, and another for a generic item intended to take into account all recipes.
Update item::foodinfo to display the new values. To avoid infinite recursion we don't try to calculate the ranges for any items which were earlier detected to lie on problematic recipe loops.
Add some tests for the above.

Describe alternatives you've considered

When the player has specific ingredients on hand, it might be nice to see the (range of) actual values that would result from those, rather than the complete range of all possibilities. I decided not to do that because we need to support the more generic situation (where not all ingredients are present) anyway, so that made sense to implement first. The other feature can be added later (but it won't be trivial!).

We could display the "true" nutritional content rather than the effective content based on player traits. That would allow simplification, but it would be a change from prior behaviour, and probably less helpful.

We could make it clearer that the displayed values are dependent on player traits; I suspect most players don't realise this.

Currently the nutrient values displayed are always per-charge, whereas most of the values displayed in this are increase with charges. This can be a bit deceptive, and we might want to clarify or change this, but I think that should be a separate PR.

Testing

New unit tests, and lots of staring at recipe values. Found and fixed various unreasonable-looking values. Some issues were a result of poor recipe design, others were algorithmic issues in my code. I've fixed all the ones I found, but I would not be surprised to hear that I missed some corner cases.

@DaviBones would appreciate you taking a look at this since you've also been working on & finding bugs in this part of the code recently.

Additional context

KorGgenT · 2019-12-17T03:01:19Z

as we are trying to deprecate player, i humbly request the scope of player::compute_nutrient_range() be moved to the appropriate class, probably Character here.

Davi-DeGanne · 2019-12-17T05:44:05Z

This looks great, I've been looking forward to this change! It is not only good for players, but also will be helpful for debugging/balancing.

The only thing I have to say is that you are not taking account for the RAW flag, which is actually a regression in some instances. To see what I mean, spawn some wild vegetables and cook them -- notice that the crafting preview says they will have 25 calories, but they end up with 34 calories. Let me briefly explain how it works:

If an item has the RAW flag, it is worth 25% less calories, unless it also has the COOKED flag.
Upon crafting, if recipe.hot_result() returns true, then COOKED is added to the result and all components via item::set_flag_recursive.

It's not trivial to fix, unfortunately. The only quick fix I can think of is to take comest as a copy in player::compute_nutrient_range so you can recursively add COOKED without modifying the original object, but that is an ugly performance hit that doesn't seem worth it at all.

src/consumption.cpp

jbytheway · 2019-12-18T02:46:50Z

as we are trying to deprecate player, i humbly request the scope of player::compute_nutrient_range() be moved to the appropriate class, probably Character here.

It depends on other functions that are still in player. I'd rather not move those other functions in this same PR, because it will make the changes less clear. I think it will be easy to move these along with the other nutrient functions when the time comes.

jbytheway · 2019-12-20T02:32:42Z

@DaviBones I think I found a reasonable solution to the COOKED / RAW issue. It seems to be working correctly now.

I added a set of extra_flags to be passed down the recursive function calls; currently only used for COOKED, to allow the components to know that in this hypothetical scenario they would have been cooked.

Davi-DeGanne · 2019-12-20T03:02:03Z

Looks great!

As an aside, I struggle to think of an instance where we'll want to use extra_flags for anything else, though I also can't think of any meaningful downside to using it instead of a boolean parameter, so I suppose it's better to have it and not need it, than to need it and not have it.

src/cata_algo.h

BevapDin · 2019-12-20T07:29:18Z

src/stomach.cpp

+
+bool nutrients::operator==( const nutrients &r ) const
+{
+    if( kcal != r.kcal ) {


I do wonder why return kcal == r.kcal && vitamins == r.vitamins; does not work?

Because non-entries in the vitamins map have to be considered equivalent to zero entries.

jbytheway · 2019-12-20T12:22:11Z

As an aside, I struggle to think of an instance where we'll want to use extra_flags for anything else, though I also can't think of any meaningful downside to using it instead of a boolean parameter, so I suppose it's better to have it and not need it, than to need it and not have it.

Mostly I did it that way for maintenance reasons. If in the future anyone alters the way these flags work they are likely to grep the code for "COOKED", and I wanted them to find this code when they did that.

Recipe loops are problematic (see last commit). There are none now in the core game, but mods might add them. This code detects recipe loops and marks the items contained within them. Such loops are considered an error reported via debugmsg. This implementation is a little slower than I would prefer. A slightly more advanced cycle-finding algorithm could be used to make it faster.

These functions calculate the range of possible nutrients a particular item or recipe might produce, depending on which ingredients are used within it.

Currently item info for items with variable nutrient content simply list the default values with a disclaimer that they might be inaccurate. In fact the default values are more or less the only values that *won't* occur when you craft the recipe. Instead, calculate and display the range of possible values for the nutrient content.

If we use any namespace, it should be cata (or a subnamespace thereof). It doesn't make sense to be adding other top-level namespaces.

Use a O(V+E) cycle-finding algorithm, and factor it out appropriately to clarify the code.

To predict nutrient content we need to also predict changes to the item flags that occur when cooking. Implement that, and add a test to verify that it works correctly for cooked wild veggies.

The nutrient predictions were based on a list of recipes cached in each item. It turns out that list could end up with invalid recipe_ids if those recipes became blacklisted. Detect that case and avoid adding such recipes to the list.

jbytheway · 2019-12-22T18:48:13Z

I think this is good to go now.

KorGgenT added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA labels Dec 17, 2019

BevapDin reviewed Dec 17, 2019

View reviewed changes

src/consumption.cpp Outdated Show resolved Hide resolved

jbytheway force-pushed the nutrient_predictions branch from 397be7e to e384f42 Compare December 20, 2019 02:31

BevapDin reviewed Dec 20, 2019

View reviewed changes

src/cata_algo.h Outdated Show resolved Hide resolved

BevapDin reviewed Dec 20, 2019

View reviewed changes

jbytheway force-pushed the nutrient_predictions branch from e384f42 to 857f9b4 Compare December 20, 2019 12:30

weekly-digest bot mentioned this pull request Dec 22, 2019

Weekly Digest (15 December, 2019 - 22 December, 2019) #36345

Closed

jbytheway added 2 commits December 22, 2019 11:05

Support the computation of nutrient ranges

4afe91b

These functions calculate the range of possible nutrients a particular item or recipe might produce, depending on which ingredients are used within it.

jbytheway force-pushed the nutrient_predictions branch from 08e0832 to 09c237d Compare December 22, 2019 16:11

jbytheway added 9 commits December 22, 2019 11:50

Change namespace of cata_algo.h

e7f8190

If we use any namespace, it should be cata (or a subnamespace thereof). It doesn't make sense to be adding other top-level namespaces.

Improved cycle finding algorithm

f98983e

Use a O(V+E) cycle-finding algorithm, and factor it out appropriately to clarify the code.

Tests for cycle-finding algorithm

3081118

Add iteminfo tests for nutrient display

1476913

Remove redundant 'else' branch

f5c1962

Use COOKED and RAW flags in nutrient predictions

b196175

To predict nutrient content we need to also predict changes to the item flags that occur when cooking. Implement that, and add a test to verify that it works correctly for cooked wild veggies.

Explain curious operator==

853b4b4

Don't consider blacklisted recipes for nutrients

065d355

The nutrient predictions were based on a list of recipes cached in each item. It turns out that list could end up with invalid recipe_ids if those recipes became blacklisted. Detect that case and avoid adding such recipes to the list.

jbytheway force-pushed the nutrient_predictions branch from 09c237d to 065d355 Compare December 22, 2019 16:51

kevingranade merged commit 8d59a49 into CleverRaven:master Dec 23, 2019

jbytheway deleted the nutrient_predictions branch December 24, 2019 15:44

weekly-digest bot mentioned this pull request Dec 29, 2019

Weekly Digest (22 December, 2019 - 29 December, 2019) #36542

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crafting menu shows range of possible nutrients for food crafts #36172

Crafting menu shows range of possible nutrients for food crafts #36172

jbytheway commented Dec 17, 2019 •

edited

Loading

KorGgenT commented Dec 17, 2019

Davi-DeGanne commented Dec 17, 2019

jbytheway commented Dec 18, 2019

jbytheway commented Dec 20, 2019 •

edited

Loading

Davi-DeGanne commented Dec 20, 2019

BevapDin Dec 20, 2019

jbytheway Dec 20, 2019

jbytheway commented Dec 20, 2019

jbytheway commented Dec 22, 2019

Crafting menu shows range of possible nutrients for food crafts #36172

Crafting menu shows range of possible nutrients for food crafts #36172

Conversation

jbytheway commented Dec 17, 2019 • edited Loading

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

KorGgenT commented Dec 17, 2019

Davi-DeGanne commented Dec 17, 2019

jbytheway commented Dec 18, 2019

jbytheway commented Dec 20, 2019 • edited Loading

Davi-DeGanne commented Dec 20, 2019

BevapDin Dec 20, 2019

Choose a reason for hiding this comment

jbytheway Dec 20, 2019

Choose a reason for hiding this comment

jbytheway commented Dec 20, 2019

jbytheway commented Dec 22, 2019

jbytheway commented Dec 17, 2019 •

edited

Loading

jbytheway commented Dec 20, 2019 •

edited

Loading