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

Check for rotten components before starting craft #36263

Closed
wants to merge 3 commits into from
Closed

Check for rotten components before starting craft #36263

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 19, 2019

Summary

SUMMARY: Bugfixes "Check for rotten components before starting craft"

Purpose of change

Fixes #33385

Describe the solution

This was an annoying one.
The crafting code gathers up all the used components by consuming them, and the consuming functions return what was used up.

So to gather up components that are going to be used and checking them beforehand, I had to propagate down the chain a "check_only" parameter that allowed the components to be returned for checking, but not used up.

So this adds a lot of checks in a lot of places.

Also there is one drawback - the crafting code donst prefer non-rotten components by default anyway, so if it so happens the first components it was gonna choose anyway are rotten, this will still warn the player about them, but give no option to skip them in favour of the non-rotten components that may be there, actually skipping rotten components when forming the inventory for the craft can be done in a seperate PR, until then , the player can get this warning, so they dont waste stuff, then if they know they have non-rotten components, they can move the rotten ones out of the way, so the non-rotten ones are picked up.

But as it stands, this at least fixes the issue and prompts a warning, I have tested a few different permutations, but I had to put so many little checks everywhere, that my confidence level is not optimal that theres some sneaky broken thing ive introduced, Ive gone over it a few times, but would appreciate scrutiny.

Describe alternatives you've considered

N/A

Testing

Crafted a few different food items with rotten and non-rotten components in a variety of places - in vehicle cargo, as stacks , as charges etc. Always got the prompt if it was a rotten component

Additional context

N/A

@ghost ghost changed the title Check rotten components before starting craft Check for rotten components before starting craft Dec 19, 2019
@I-am-Erk I-am-Erk added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Items / Item Actions / Item Qualities Items and how they work and interact Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA labels Dec 19, 2019
Copy link
Contributor

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

Maybe I'm missing something but I think this would be doable using the existing filter infrastructure and would require a good deal less special casing. It could look something like this:

check if recipe can be made using non-rotten components
yes -> consume the components using a non-rotten filter in addition to the recipe-specific filter.
no -> warn since rotten components are needed to make the craft

It seems that what you have done here is to essentially hack the functionality of has_charges/has_amount into use_charges/use_amount which is something we should probably avoid as evidenced by the plethora of checks needed to make it work

@ghost
Copy link
Author

ghost commented Dec 23, 2019

Maybe I'm missing something but I think this would be doable using the existing filter infrastructure and would require a good deal less special casing. It could look something like this:

check if recipe can be made using non-rotten components
yes -> consume the components using a non-rotten filter in addition to the recipe-specific filter.
no -> warn since rotten components are needed to make the craft

It seems that what you have done here is to essentially hack the functionality of has_charges/has_amount into use_charges/use_amount which is something we should probably avoid as evidenced by the plethora of checks needed to make it work

Yep seems better.

@Zourin2
Copy link

Zourin2 commented Dec 26, 2019

If you could sort the list of applicable ingredients by expiration time then move the rotten ones to the bottom, you'd only need a warning if the recipe selects a rotten component. This way the game naturally uses an 'oldest first', much like how you'd finish up last week's milk before cracking the seal on a new one.

@ghost
Copy link
Author

ghost commented Dec 28, 2019

Closing in favour of anyone who can implement ifreunds proper solution.

@ghost ghost closed this Dec 28, 2019
@ghost ghost deleted the check_rotten_components branch December 31, 2019 21:00
@jbytheway
Copy link
Contributor

Alternate solution at #36610.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Items / Item Actions / Item Qualities Items and how they work and interact Quality of Life QoL: degree to which players are comfortable, and able to enjoy CDDA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn player of rotten ingredients when crafting food items
4 participants