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

Fix item consumption upon crafting failure #28501

Closed

Conversation

Projects
None yet
5 participants
@dissociativity
Copy link
Contributor

commented Mar 5, 2019

SUMMARY: Bugfixes "Fix failure to consume any materials upon crafting failure message

Purpose of change

Fixing the failure to consume any items on crafting failure

Describe the solution

I'll just quote a post from here to explain things:
thread link

tl;dr: Change Line 446 of crafting.cpp from

if( cou > 0 && one_in( 2 ) ) {
to
if( cou == 0 || one_in( 2 ) ) {

I did a bit more research just now, and it is definitely broken. After cooking 20 lards at skill level 0 and 1, I never consumed any fat. After crafting 5 upgraded solar panels at skill level 0, I consumed the following: 1 solar panel, 30 copper wires, 8 amplifier circuits, 8 solar cells, 2 power converters, and no solder.

According to the comments though (line 444/445 of crafting.cpp)

// Each component currently has 50% chance of not being consumed
// Skip first item so failed craft with one item recipe always loses component

I don’t know how to compile and test exactly what is wrong, but it is certainly not working as intended. My hunch? Line 446 reads:

if( cou > 0 && one_in( 2 ) ) {

But if the comments are correct, it should read:

if( cou == 0 || one_in( 2 ) ) {

Currently, I think that it is never consuming the first thing in the inventory list, instead of always consuming the first thing in the inventory list.

Describe alternatives you've considered

Rewrite that portion of the code, which I'm unqualified to do.

Additional context

Tested it in-game and seems to work as intended in the quotes post above.

craftingfailconsumefix
fixes crafted item ingredient loss on failure
@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

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

https://discourse.cataclysmdda.org/t/8561-catastrophically-failing-at-crafting-cooked-liver-and-cracklins-does-not-consume-components/19048/9

@ifreund

ifreund approved these changes Mar 5, 2019

@digitCruncher

This comment has been minimized.

Copy link

commented Mar 5, 2019

Argh! I feel super bad now, because I was the one who suggested those changes. Now it is early morning and I am fresh and looked at the code change (and the testing reports that dissociativity mentioned) and I see I made a mistake... This change will mean that on a catastrophic failure the first ingredient will be consumed half the time, and all subsequent ingredients will be consumed ALL the time >.<

The line should be
if( cou == 0 || one_in( 2 ) ) {

I think. I've been wrong before...

Sorry I can't test this. I still don't have time to mess around and try to compile the game from my computer. I got a github account finally, so you guys are slowly dragging me into actively contributing >.>

@ifreund
Copy link
Contributor

left a comment

Yeah @digitCruncher is totally right about what needs to happen here, I read too fast.

@dissociativity

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

alright, should be ready to go now

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Bug report via email that is almost certainly this issue.

 I BELIEVE (but have yet to definitively confirm) that, when crafting more of something using the “-“ key, it doesn’t consume the items if you “fail and waste some items” like it used to

@dissociativity dissociativity changed the title craftingfailconsumefix Fix item consumption upon crafting failure Mar 6, 2019

@dissociativity

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2019

Jenkins Rebuild

@ifreund

ifreund approved these changes Mar 6, 2019

@AMurkin

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2019

Add closing quotation mark to your summary line.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 6, 2019

Meged to master and development as c24d77f

@dissociativity dissociativity deleted the dissociativity:craftfailfix branch Mar 17, 2019

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.