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

Jsonify water boiling and other temperature effects on the item #78562

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

moxian
Copy link
Contributor

@moxian moxian commented Dec 14, 2024

Summary

Infrastructure "Jsonify water boiling and other temperature effects on the item"

Purpose of change

I have long known that I can turn water into clean water by crafting (&) it - using some heat and a container suited for boiling.
But just today I have found out that I don't actually need to spend time watching the kettle boil! I can just put it into the fire, go away, come back some time later to nice clean water waiting for me. This is great!

However, to my bigger surprise this auto-boiling handling is hardcoded in C++ and only works for water->clean water specifically.

I thought that hardcode is bad, and JSON is good and set to move the logic into JSON (and perhaps apply it to more different use cases!).

Describe the solution

Add a new field to item json (and the deserialization thereof). I'll copy-paste the new docs:

"temperature_effects": [{           // Optional. Specifies certain effects happening to the item when it reaches certain internal temperature thresholds.
                                    // Holds an array of objects
    "temperature_above": "373 K",   // Temperature needs to be at least this high for the effect to happen.
    "temperature_below": "0 K",     // Temperature needs to be at least this low for the effect to happen.
                                    // Exactly one of temperature_above and temperature_below must be set.
    "transform": "water_clean",     // The item id that we transform into upon reaching the temperature. Boiling water gives clean water, as an example.
    "remove_poison": true           // Whether we should remove poison from comestible item. Defaults to false.
}],

Utilize this freshly added field for water to transform it into water_clean at 373 K and remove_poison.

Change the hardcoded water transformation to instead iterate these temperature_effects and execute any of those that get triggered.

Remove the mentions of "water", "water_clean" from item.cpp and units::temperature boiling from codebase.

Describe alternatives you've considered

  • leave things be and be sad about hardcode
  • reject and remove auto-water-boiling altogether, embrace tradition
  • do something even more extensive and handle freezing/freezeburn via the same system, somehow.
  • do something even more extensive and handle full phase transitions properly (ice<->water<->steam or perhaps butter<->buter(liquid) )

Testing

Made a firepit, set it on fire, put in a pot, filled it with water, waited for an hour, checked back, saw clean water (hot)
Temporarily changed the water json to transform water into cooking_oil instead, repeated the above steps, saw vegetable cooking oil (hot)

Additional context

Honestly, there's not too much use of the system beyond specifically water->clean water transformation. The only others I could think of were:

  • Cooking meat and eggs. Great idea! Does not work in practice because for some reason dropping a chunk of meat into a firepit immediately sets its temperature to 1.5K 1.5KK, i.e. 1500 kelvin, making it instantly cooked. I have not spent time debugging this further.
  • transforming plastic items into "plastic chunks" at high temperatures but that's already handled by the burn code
  • bricking electronics at high temps, but idk how i feel about that one
  • bricking various mutagens or other medical thingies (flu shot?) at high temps - I kinda like the idea, but I don't have enough experience with those to make a good call.
  • Turning eggs (fert.) into eggs (unfert.) when they freeze. Little to no practical value, apart from slightly thinning out bird population (why are they laying eggs in freezing temps i have no idea). Also, looking closer, I don't see a good way to control fertilization status of an individual egg item - it seems to be bound to the overall item type instead.
  • Something about lye making? Milk curdling?
  • More general phase transition of items (aforementioned ice<->water<->steam, or butter<->melted butter), buuut... 1) i dread having to deal with code handling liquid items in non-waterproof pockets and 2) perhaps that's better defined on the material rather than item level?
  • Perhaps Magiclysm can do some funny alchemy with this...

So this whole PR is just for clean-water un-hardcoding. For now.

(P.s.: driveby change - i've moved itype::name field to be the first field in the struct to make debugging in VS less agonizing. No behavior change, no save compat issues or anything of that sort)

Visual Studio debugger takes AGES to show value of any struct members,
but even longer for members that are down the list (since it seemingly
retrieves them in order, and in order to see the `name` halfway
through the struct you need to first see milling data, and weight
and its pockets and...)
src/item_factory.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added <Documentation> Design documents, internal info, guides and help. [JSON] Changes (can be) made in JSON Items: Food / Vitamins Comestibles and drinks [C++] Changes (can be) made in C++. Previously named `Code` [Markdown] Markdown issues and PRs labels Dec 14, 2024
src/itype.h Outdated Show resolved Hide resolved
@github-actions github-actions bot added the Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style label Dec 14, 2024
@GuardianDll
Copy link
Member

Two quesitons: to this particular change, shouldn't we specify for how long the temperature should last for boiling to happen?
Second, to the rest of the team: do we even want to have this mechanic at all? my understanding was that it was a one-off gimmick because we have no #62618, and expanding it sound rather counterproductive

@moxian
Copy link
Contributor Author

moxian commented Dec 14, 2024

to this particular change, shouldn't we specify for how long the temperature should last for boiling to happen?

Good idea, but out of scope of this PR. It would no longer be a jsonification, but instead a new feature, and adding it seems not straightforward at a glance. Adding the neccessary duration to the json would be trivial tho (in other words, the proposed json system is easily extensible)

FWIW the water boiling is not instant. It took me an hour of waiting for 0.5L of water in the pot to become water_clean. For some reason this does not work as well for meat. I suspect because I was setting a bit lower transition temperature for it, but as I mentioned in the OP i did not spend too much time debugging.
Regardless, this does not create any sort of "exploit" - you still need to wait for the water to boil. The only benefit is that you can be busy doing other things in the meanwhile.

do we even want to have this mechanic at all?

I have no strong opinion on this, since I have only learned of its existence today. If the whole system gets removed, I would be almost equally happy since the goal of un-hardcoding the water is still achieved.

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 14, 2024
Copy link
Member

@RenechCDDA RenechCDDA left a comment

Choose a reason for hiding this comment

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

Honestly, there's not too much use of the system beyond specifically water->clean water transformation. The only others I could think of were:

This is kind of my immediate concern, in that it lacks a clear use case. The player's ability to manipulate temperature of an item is very limited, and in cases where they can (i.e. boiling water) it only currently exists because it's vital to survival.

To put on my snobbery hat, JSONizing this system introduces a suggestion that these things should happen through the temperature system. That if you wanted to turn raw meat into cooked meat or whatever it's valid to just drop it on the fire and done. It (like the original water boiling effect) is meant to bypass the crafting system, but without a clear need for doing so.

Anyway I have only minor nitpicks with the PR's implementation

src/item.cpp Show resolved Hide resolved

static void load_temperature_effects(
const JsonObject &jo, const std::string_view member,
cata::value_ptr<islot_temperature_effects> &slotptr, const std::string &src )
Copy link
Member

Choose a reason for hiding this comment

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

Clang error here. Looks like you pass along src but forgot to check if two definitions come from the same source?

@@ -1228,6 +1243,14 @@ struct itype {

using FlagsSetType = std::set<flag_id>;

protected:
Copy link
Member

Choose a reason for hiding this comment

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

I see you made a comment for this (quoted below), could you expand on what you meant? I've never had any trouble debugging in VS regardless of where a member is placed inside a struct. Do you mean scrolling through the raw data shown when you hover a variable, while the program is breakpointed?

(P.s.: driveby change - i've moved itype::name field to be the first field in the struct to make debugging in VS less agonizing. No behavior change, no save compat issues or anything of that sort)

@Maleclypse
Copy link
Member

This is kind of my immediate concern, in that it lacks a clear use case. The player's ability to manipulate temperature of an item is very limited, and in cases where they can (i.e. boiling water) it only currently exists because it's vital to survival.

I would suggest that an immediate use case would be expanding this to teas and coffees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style <Documentation> Design documents, internal info, guides and help. Items: Food / Vitamins Comestibles and drinks [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions [Markdown] Markdown issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants