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

Can't activate item - character instead tries to eat it. #76414

Closed
AzyWng opened this issue Sep 13, 2024 · 14 comments · Fixed by #76823
Closed

Can't activate item - character instead tries to eat it. #76414

AzyWng opened this issue Sep 13, 2024 · 14 comments · Fixed by #76823
Labels
(S2 - Confirmed) Bug that's been confirmed to exist

Comments

@AzyWng
Copy link
Contributor

AzyWng commented Sep 13, 2024

Describe the bug

When activating some in progress mutant lard, instead of continuing to work on it, my character tries to eat it.

Attach save file

Summers-trimmed.tar.gz

Steps to reproduce

  1. Try to make some lard in a batch.
  2. Stop the action.
  3. Attempt to resume it
  4. Fail bc your character tries to eat it.

Expected behavior

I expected to be able to resume crafting like other crafting recipes.

Screenshots

No response

Versions and configuration

  • OS: Linux
    • OS Version: LSB Version: n/a; Distributor ID: EndeavourOS; Description: EndeavourOS Linux; Release: rolling; Codename: rolling;
  • Game Version: cdda-experimental-2024-09-13-1401 0221f6e [64-bit]
  • Graphics Version: Tiles
  • Game Language: English [en]
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]

Additional context

No response

@AzyWng AzyWng added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Sep 13, 2024
@PatrikLundell
Copy link
Contributor

/Confirmed

Tried the same thing with 10 spawned chunks of meat and then frying the batch. When interrupted and activated a failing attempt to eat the in-progress item followed.

It seems the craft resumption action is somehow suppressed and the consumption action of the underlying item (target or source?) is attempted and then failed. In-progress items shouldn't exhibit any actions of either input or result.

@mqrause
Copy link
Contributor

mqrause commented Sep 15, 2024

Like #76425, this is also caused by the water purification PR. In this case by e0650e5, which moved the eating check to before actual use actions are invoked. So comestibles, and that includes in progress crafts of comestibles, will always attempt to be eaten instead of having their use actions called.

@Aism4n
Copy link

Aism4n commented Sep 25, 2024

This bug also completely break taming (character try to eat cattle food).

@Cddasurvivor

This comment was marked as off-topic.

@CavanStone
Copy link
Contributor

Like #76425, this is also caused by the water purification PR. In this case by e0650e5, which moved the eating check to before actual use actions are invoked. So comestibles, and that includes in progress crafts of comestibles, will always attempt to be eaten instead of having their use actions called.

Does anyone have any insight as to why this was done in [e0650e5]? Possibly dumb question but why can't we just revert this particular commit? I did an admittedly cursory review and it does not appear to me that this code change actually furthered the stated purposes of the PR which was to rework purification tablets.

@PatrikLundell or @ZeroInternalReflection could you help us out here?

@mqrause
Copy link
Contributor

mqrause commented Oct 1, 2024

If it doesn't do anything for purification functionality, I'd say just revert it.

@PatrikLundell
Copy link
Contributor

I can't help. My effort was purely a technical one of sorting out merge conflicts, not any involvement in the underlying logic itself.

@ZeroInternalReflection
Copy link
Contributor

Like #76425, this is also caused by the water purification PR. In this case by e0650e5, which moved the eating check to before actual use actions are invoked. So comestibles, and that includes in progress crafts of comestibles, will always attempt to be eaten instead of having their use actions called.

Does anyone have any insight as to why this was done in [e0650e5]? Possibly dumb question but why can't we just revert this particular commit? I did an admittedly cursory review and it does not appear to me that this code change actually furthered the stated purposes of the PR which was to rework purification tablets.

@PatrikLundell or @ZeroInternalReflection could you help us out here?

It's been a long time since I've looked at that code and I'm not set up to test it right now, but if I remember correctly, that change was to allow the water (purifying) -> clean water transition. If it's what I remember, prior to that commit, attempting to activate water (purifying) just caused it to cease to exist (or maybe try to eat it? I think I ran into both issues) rather than giving you clean water.

If that particular commit is causing problems this far-reaching (in addition to the liquid-handling issues) I think much of the concept behind #71971 breaks and I'd support reverting the entire PR. It sucks, but I don't think you can pull out just this commit and have a functioning water->water (purifying)->clean water path. it'll be a while before I have a chance to dig into a better fix.

@CavanStone
Copy link
Contributor

Like #76425, this is also caused by the water purification PR. In this case by e0650e5, which moved the eating check to before actual use actions are invoked. So comestibles, and that includes in progress crafts of comestibles, will always attempt to be eaten instead of having their use actions called.

Does anyone have any insight as to why this was done in [e0650e5]? Possibly dumb question but why can't we just revert this particular commit? I did an admittedly cursory review and it does not appear to me that this code change actually furthered the stated purposes of the PR which was to rework purification tablets.
@PatrikLundell or @ZeroInternalReflection could you help us out here?

It's been a long time since I've looked at that code and I'm not set up to test it right now, but if I remember correctly, that change was to allow the water (purifying) -> clean water transition. If it's what I remember, prior to that commit, attempting to activate water (purifying) just caused it to cease to exist (or maybe try to eat it? I think I ran into both issues) rather than giving you clean water.

If that particular commit is causing problems this far-reaching (in addition to the liquid-handling issues) I think much of the concept behind #71971 breaks and I'd support reverting the entire PR. It sucks, but I don't think you can pull out just this commit and have a functioning water->water (purifying)->clean water path. it'll be a while before I have a chance to dig into a better fix.

Thank you so much for your insight on this as it will save us a whole lot of time otherwise spent second guessing you. I would suggest the following sequence of PRs to resolve this issue.

  1. Revert Water purification tablets only purify up to 4 water each #71971
  2. Add unit tests ensuring that the currently reported bugs stay resolved
  3. Take another shot at water purifying tablets as a crafting ingredient instead of an activated item. (The fix might be to just go straight to clean water instead of an intermediate state.)

Normally I'd say 2 would be part of the other PRs but I think there needs to be a discussion in the community about the standard order of operations for using an item. By making these a Unit Test, that standard would be enforced by the test code. I think a lot of developer frustration can possibly be avoided if we bulk out our test suite a little more. Yeah it slows down the velocity in experimental but we'll have less need to revert for bugs as in this example. The code base has gotten complex enough that its very difficult for a single developer to make changes to the code and not break a key assumption other developers have made, especially in a community-driven project such as this one. Adding tests for key assumptions and standards can raise awareness to the developer of what those assumptions are.

@RenechCDDA
Copy link
Member

Any progress made on this issue?

Everything is public, as you can see. There is no need to comment on the issue asking. Feel free to fix it yourself if you are impatient.

@CavanStone
Copy link
Contributor

So doing some code archeology, I think the solution is to just revert [e0650e5] and change the type of the water_purifying object to something besides Comestible, Chemical maybe. Yeah, you will not be able to drink the purifying water, but why would you? Per the description of the tablets it is a strong chemical and other descriptions imply that it is not drinkable.
This change may reflect full realism, but it will be a functional realism. https://github.com/CleverRaven/Cataclysm-DDA/pull/71971/files#diff-4afbc710a5b45b39a77f637e1f93f3bf0e7efd016039bb7f8f03308b191bf1adR1875-R1904
{
"id": "water_purifying",
"type": "COMESTIBLE",
"comestible_type": "DRINK",
"category": "food",
"name": { "str_sp": "water (purifying)" },
"description": "You've dissolved tablets of disinfecting chemicals in this water to purify it. Soon it'll be ready to drink.",
"material": [ "water" ],
"weight": "250 g",
"volume": "250 ml",
"charges": 1,
"price": 50,
"price_postapoc": 1,
"symbol": "~",
"color": "light_blue",
"phase": "liquid",
"container": "bottle_plastic",
"sealed": false,
"quench": 50,
"ammo_data": { "ammo_type": "water" },
"flags": [ "EATEN_COLD" ],
"use_action": {
"target": "water_clean",
"msg": "After using the tablets and carefully decanting out the resulting clean water, it's ready to drink",
"moves": 100,
"type": "delayed_transform",
"transform_age": 2400,
"not_ready_msg": "The chlorine released by the tablets is still working. You still need to wait a bit before you can drink it."
}

@CavanStone
Copy link
Contributor

Further supporting the purifying_water is a chemical theory, aquatabs are (https://en.wikipedia.org/wiki/Sodium_dichloroisocyanurate) which releases Chlorine gas into the water and seeing how this automatically dirty water, you are going to get that sweet organochloride aka pool water smell that used to be around pools back in the 90s but most places have cleaned up since then. So unless you really care about being able to drink dirty 90s pool water, I would say let's classify purifying water as a chemical.

@chaoticidealism
Copy link

Happens with cat food and dog food too.

@cyberpunk2021
Copy link

The problem with food is solved but sadly I have the exact same issue with in progress purifier primer --> You need a syringue to consume that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants