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

Quickfix for sweetbread missing from offal_raw #1916

Merged
merged 7 commits into from
Nov 19, 2022

Conversation

chaosvolt
Copy link
Member

@chaosvolt chaosvolt commented Sep 26, 2022

Summary

SUMMARY: Bugfixes "fix sweetbread not being usable in offal recipes, make comestible_test.cpp more forgiving"

Purpose of change

Quick fix for a thing I just now noticed got missed during #1371, sweetbread was axed from the new crafting requirement by accident.

Describe the solution

Re-added sweetbread to the offal_raw crafting requirement, also added a suggested tweak to one of the tests along the way. Basically less use of standard deviation in favor of a fixed (more forgiving) distance away from default calories.

Describe alternatives you've considered

Procrastinating until Coolthulhu does it.

Testing

Checked affected file for syntax and lint errors.

Additional context

How the fuck did I not notice that when I looked over that PR, I thought I eyeballed it for the sake of grabbing offal item IDs for the follow-up PR (#1519)...

@chaosvolt
Copy link
Member Author

chaosvolt commented Sep 26, 2022

Okay, so now we have what should be a basic one-line fix instead being more complicated because the test is being weird. First it was complaining because 2 sweetbread seemed to be too much, but this latest test failure makes zero sense:

/home/runner/work/Cataclysm-BN/Cataclysm-BN/tests/comestible_test.cpp:144: FAILED:
  CHECK( lower_bound <= mystats.calories.avg() )
with expansion:
  196.77972f <= 193.1666666667



RecipeID: offal_canned, Lower Bound: 196.779724, Average: 193.166667, Upper Bound: 607.220276

/home/runner/work/Cataclysm-BN/Cataclysm-BN/tests/comestible_test.cpp:144: FAILED:
  CHECK( lower_bound <= mystats.calories.avg() )
with expansion:
  393.55945f <= 386.3333333333



RecipeID: offal_pickled_jarred, Lower Bound: 393.559448, Average: 386.333333, Upper Bound: 1214.440552

It already has two high-calorie offal items and several low-calorie ones, an in-between ingredient shouldn't screw things up that readily. Especially since PREVIOUSLY sweetbread was used in the offal crafting requirement with no weird effects.

@Coolthulhu Coolthulhu self-assigned this Sep 27, 2022
@Coolthulhu
Copy link
Member

It already has two high-calorie offal items and several low-calorie ones, an in-between ingredient shouldn't screw things up that readily.

It calculates standard deviation, meaning every point close to the mean makes it want less variance.

@chaosvolt
Copy link
Member Author

It calculates standard deviation, meaning every point close to the mean makes it want less variance.

Ah, weird. Any idea on how to enable it to pass the test while still letting sweetbread work in the recipe again?

@Coolthulhu
Copy link
Member

Ah, weird. Any idea on how to enable it to pass the test while still letting sweetbread work in the recipe again?

We probably should rework the test to have some minimum spread. stddev is not a good idea for thresholds.

@chaosvolt
Copy link
Member Author

Ah, hmm. Not sure how to go about that, nor if that'd be out of scope for this fix.

@Coolthulhu Coolthulhu removed their assignment Sep 30, 2022
* Reverted the change to how much sweetbread is used, because the test that the original commit is less CBT to try and fix.
* Per Coolthulhu's suggestion on the discord, adjusted how comestible_test.cpp works to make the range more forgiving.
@chaosvolt chaosvolt added JSON related to game datas in JSON format. tests changes related to tests labels Oct 27, 2022
@chaosvolt
Copy link
Member Author

Reverting the change to the comestible test, this seems to need to be a separate PR as it's going to be bigger than the intended one-line change to allow sweetbread.

@chaosvolt chaosvolt removed the tests changes related to tests label Oct 27, 2022
@Coolthulhu Coolthulhu self-assigned this Nov 15, 2022
@Coolthulhu Coolthulhu merged commit f0ae817 into cataclysmbnteam:upload Nov 19, 2022
@chaosvolt chaosvolt deleted the quick-offal-fix branch November 19, 2022 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JSON related to game datas in JSON format.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants