Skip to content

/Cataclysm-DDA

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

Animal cooking oil and baking fixes #29086

Merged
merged 13 commits into from Apr 2, 2019

Conversation

Reviewers

@ZhilkinSerg ZhilkinSerg

Assignees
No one assigned
Labels
Food / Vitamins [C++] [JSON]
Projects
None yet
Milestone
No milestone
4 participants
@anonym @KorGgenT @ZhilkinSerg @kevingranade
@anonym
Copy link
Contributor

commented Mar 28, 2019 •
edited

Summary

SUMMARY: Content "Add animal cooking oil, add recipe for canning beans, and rebalance various foods and recipes"

Purpose of change

I actually got pulled into this rabbit hole of fixing baking-related inconsistencies when I experienced #29076 (but @KorGgenT was faster than me!). And that is what the purpose is, fixing a few bugs, (and a typo that made chocolate pancakes 10x more nutritious than they should be) and filling a few wholes (I can't make edible cannabis without chocolate?).

Describe alternatives you've considered

Instead of adding animal cooking oil I suppose I could have made lard/tallow more granular by making each charge an 1/8th of was they are now.

Additional context

Perhaps you, @KorGgenT, want to have a look since this is an area you've been working on lately!
@KorGgenT

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 28, 2019

note: if the travis test fails, it could possibly be because of the recipes. i'll check back in a few hours when it finishes

@KorGgenT KorGgenT added [JSON] Food / Vitamins labels Mar 28, 2019

@KorGgenT

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 28, 2019

also: you only want one summary line. you've got four, so you want to amalgamate those summaries

@KorGgenT KorGgenT added the [C++] label Mar 28, 2019

@KorGgenT

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 28, 2019

image
(you can ignore the fish soup because i'm fixing it in another PR)
if the "average" is higher than the upper bound, then that means the sum of the calories of the ingredients is too high. you can either fix that by increasing the number of charges the recipe makes, or reducing the overall calorie count. semi-recently i put a test into place that checks the ingredients of a recipe against its output to make sure that they're pretty close to the same calories input and output.
@anonym

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Mar 29, 2019 •
edited

Cheers, @KorGgenT! Gonna start running cata_test before each PR from now on. It passes for me now, except soup_fish_jarred_3l like you mentioned, but also soup_meat_jarred_3l but I'm guessing it too is unrelated (?).

I ended up also fixing stuff related to beans/rice, and added recipes for canning beans.
@KorGgenT

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 29, 2019 •
edited

try rebasing to master. it might be related. the fish soup thing was fixed and merged

@anonym anonym force-pushed the anonym:baking-fixes branch from 5033773 to 5db70fd Mar 29, 2019

@ZhilkinSerg ZhilkinSerg self-assigned this Mar 31, 2019

@ZhilkinSerg

ZhilkinSerg reviewed Mar 31, 2019
View changes

Show resolved Hide resolved data/json/recipes/recipe_food.json Outdated
@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Mar 31, 2019

image

anonym and others added some commits Mar 28, 2019

@anonym
Harmonize brownie vs brownie_weed. 
There's no reason the charges and nutrition should be different.
47857b3
@anonym
Rebalance brownies: they need fat and sugar, and more chocolate. 
These are some sad brownies! Given the current recipe I'd expect a
much lower enjoyment... maybe even negative because it will crush your
expectations! ;P

I've never encountered a brownie recipe IRL that doesn't use some pure
fat in addition to the egg yolk. And for brownie_weed the fat is
needed for decarboxylation (well, the egg yolk could be enough for
that unless the coagulation interferes, but I'm no expert in cannabis
cooking, and the experts all recommend pure fat).

And the same goes for sugar; with just 34g chocolate worth of
sugar (so probably less than 20g) it will barely taste sweet. And the
cacao content is really low too. We need to up both of them!
bdd1f20
@anonym
Vegetable oil is made of "veggy". 
But it doesn't contain any vitamins.
1e9d03c
@anonym
Add animal cooking oil, rebalance baking goods a bit. 
The main objective of this commit is to introduce a more granular
animal based fat source compared to lard and tallow. Without this, if
I want to fry four pancake but lack (vegetable) cooking oil, I'd need
to use 1 lard = 110g of fat = 992 kcals, which itself is way more
than the kcal content of the result.

For balancing I'm using 1 lard/tallow = 8 cooking oil, which I based
on the kcal count. I've removed lard/tallow as possible ingredients
in all recipes where <8 cooking oil is required -- since the recipe
for animal cooking oil is pretty easy this shouldn't be much of a
hindrance. In most cases this resulted in lard/tallow being dropped,
but in some instances (brioche, glazed_tenderloin, ...) it turned out
the lard/tallow amount was correct and the cooking oil had to be
bumped a lot.

As I iterated on this work I noticed some (AFAICT) wrong recipes and
other various inconsistencies, so I non-atomically also fixed:

* fruit pancake: fun +1 so it is funnier than plain pancake.
* chocolate pancake: divided calories by 10. Seems an extra digit was
  added by mistake in KorGgenT's nutrition rebalance.
* johnnycake:
  - Added seasoning to justify being funnier than bread.
  - Fat + flour isn't healthy. It's even flagged "junk".
* biscuit:
  - These aren't healthy either.
  - Made it funnier than johnnycake since it contains milk and more
    fat.
  - Cut effective nutrition in half: it used to be that it uses
    4 flour + cooking oil which is 4*48 + 127 kcals = 319 kcals with
    the resulting 3 biscuits being worth 3*212 kcals = 636 kcals, i.e.
    twice as much.
  - Dropped condensed milk to force *some* liquid in the recipe
    (ideally we could require "milk OR (condensed milk AND
    water)"). It's easy to make milk from condensed milk.
  - Dropped coconut milk since it is too nutritious => waste. (It
    could be re-added if we make coconut milk more granular i.e.
    decrease weight and/or increase charges.)
* normal and 'special' brownie:
  - Added batch_time_factors since you easily can cook huge batches in
    parallel.
  - Halved recipe to make it more convenient (although that made me
    drop lard/tallow).
* Added sponge cake (and space cake as a cannabis edible) for those
  times when you want to make brownies but lack chocolate.
1f3b882
@anonym
Fix choc_pancakes. 
It lacked "charges: 4", making the recipe producing 1/4 of what it
should and failing the recipe_permutations test. It now inherited the
correct charges value from pancackes.
1054164
@anonym
Nerf cooked/canned beans, fix rice/beans recipes.
c92e6d3
@anonym
Add recipes for canning beans.
ac3974d
@anonym
Let's call canned beans for what they are, not just "beans".
8fba6f7
@anonym
Make canned beans usable in more recipes.
b5c060a
@ZhilkinSerg @anonym
Formatting. 
Co-Authored-By: anonym <anonym@riseup.net>
1bccf05
@anonym
Formatting.
Loading status checks…
aeb1dbc

@anonym anonym force-pushed the anonym:baking-fixes branch from e43c789 to aeb1dbc Mar 31, 2019

anonym added some commits Mar 31, 2019

@anonym
Fix soup_meat_jarred_3l. 
It's components were based on 12 times the basic soup_meat recipe, but
that one has 2 charges, so the multiplier for components should just
be 6.
6c71b1f
@anonym
Boost soup_fish and fix its recipes. 
The old recipes used to give soup that has less calories than just the
fish (so when we take the "filler" component into account it gets
really bad). Now soup_fish and its recipes are more similar to
soup_meat (and 2 charges), but I went for slightly lower calories
since 2 fish fillet < 1 chunk of meat.
Loading status checks…
9d0e806

@anonym anonym force-pushed the anonym:baking-fixes branch from af092c8 to 9d0e806 Mar 31, 2019

@anonym

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Mar 31, 2019

So git bisect lead me to the reason those soups failed in the test suite: the nerf of cooked beans. Looking deeper I could see that the real problems was that soup_meat_jarred_3l required twice the amounts of components it should, and soup_fish was basically worth half the calories it should, both which I fixed. Now the tests pass for me locally!

However, in the soup recipes the "filler" ingredient is extremely unbalanced, for example a unit of 15 kcal celery is equivalent to a unit of 222 kcal rice. But I must stop delving deeper into this rabbit hole! :)

@ZhilkinSerg ZhilkinSerg merged commit cda5b37 into CleverRaven:master Apr 2, 2019
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Apr 2, 2019

@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented Apr 3, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/cooked-beans-recipe-beans-and-rice-recipe/19512/3
@anonym

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented Apr 3, 2019

@ZhilkinSerg, see the mention on discord above. It seems the commit we co-authored (1bccf05) is wrong, and adds rice as a requirement for cooked beans. Can you please revert it?
@ZhilkinSerg

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Apr 3, 2019

Just create a PR to fix any issues found.

anonym added a commit to anonym/Cataclysm-DDA that referenced this pull request Apr 4, 2019

@anonym
Drop dry_rice from beans_cooked recipe. 
That component was erroneously added in this PR:

    CleverRaven#29086

Cheers to Cynicus26 and MamaLlama on discourse for finding the bug and
connecting it to my PR to it was brought to my attention!
8e2cdb9

@anonym anonym referenced this pull request Apr 4, 2019

Merged

Drop dry_rice from beans_cooked recipe. #29270

ZhilkinSerg added a commit that referenced this pull request Apr 4, 2019

@anonym @ZhilkinSerg
Drop dry_rice from beans_cooked recipe. (#29270) 
* Drop dry_rice from beans_cooked recipe.

That component was erroneously added in this PR:

    #29086

Cheers to Cynicus26 and MamaLlama on discourse for finding the bug and
connecting it to my PR to it was brought to my attention!
Loading status checks…
8c72559
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.