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

Overhaul sterilizing of CBMs, grid autoclaves #1550

Merged
merged 11 commits into from
Jun 28, 2022

Conversation

chaosvolt
Copy link
Member

@chaosvolt chaosvolt commented May 16, 2022

Summary

SUMMARY: Features "Sterilizing autoclaves now works via mending fault, allows use of autoclaves as tools and thus grid-powered autoclaves, alternative sterilization method"

Purpose of change

A while back, I realized that the bionic fault system could be used to give us ways to define how to sterilize bionics in JSON via mending. The key thing this enables is being able to implement grid autoclaves literally right this instant, instead of having to wait for however we'd end up implementing #937.

Describe the solution

C++ changes:

  1. Replaced use of non-sterile and non-packed flags of uninstalled bionics in bionics.cpp with newly-defined bionic fault.
  2. Likewise in character.cpp set other hardcoded cases of non-sterile bionics to use the new fault.
  3. Updated game_inventory.cpp to change the install messages to check for the fault, and not check for the old obsolete flags. I left the autoclave menu mostly untouched for now since not sure how easy it'd be to strip it out.
  4. Updated item text color and tagtext behavior to check for the new flag instead.
  5. Updated self-install behavior in iuse_actor.cpp to check for the non-sterile fault instead.
  6. Removed vehicle autoclave processing from map.cpp, instead updating the "tools added by vehicleparts" section to count autoclave vehicleparts as having an autoclave in tool form.
  7. Added vehicle-autoclave-as-tool behavior to inventory.cpp as well.
  8. Further cleanup of vehicle autoclave use in vehicle.h and vehicle_use.cpp

JSON changes:

  1. Autoclave examine action removed from furniture autoclaves, renamed to "laboratory autoclave" with a change to the description to clarify what it's used for. It now gains a furniture tool use instead. Also set it so you can't drag lab autoclaves around, but fixed not actually getting an autoclave from deconstructing them.
  2. Moved full autoclave to obsolete section.
  3. Added furniture and construction entries for grid autoclaves as well.
  4. Defined pseudo tool entries for grid autoclaves and laboratory autoclaves.
  5. Removed PACK_CBM use action from autoclave pouches.
  6. Removed the autoclave use action from the autoclave item, instead clarifying how it's used in the description a bit more.
  7. Updated the basic bionic class to have access to the non-sterile fault, removed support for the deployed fault.
  8. Implement the non-sterile fault. Requires basic electronics and first aid skill to clear up using an autoclave (or laboratory autoclave) an autoclave pouch, water etc. Skill floor also serves the purpose that mending bionics used to have, of nudging players towards having the skills needed for a more successful installation. A second method, per discussion in the BN discord, was also added. It has more advanced skill requirements, in exchange for allowing you to sterilize using a selection of disinfectant items, main requirement is much higher first aid skill. IRL hydrogen peroxide is the best bet for this out of the chemicals that also exist in-game, though chlorine and formaldehyde have been brought up by various sources.
  9. Updated harvest entries.

Describe alternatives you've considered

  1. Keeping the autoclave use action as a separate thing that also automatically removes the fault on completion?
  2. Stripping out the autoclave menus now instead of later.
  3. Adding cooking skill to alternative mending method. Logical since it covers chemistry but it's not a skill used in installing bionics, so it doesn't have value as a player nudge.
  4. Converting vehicle autoclaves to use regular autoclaves as the install part and obsoleting the mountable autoclave item to simplify things further.

Testing

  1. Compiled and load tested, in a world with Manual Bionic Installation enabled.
  2. Spawned in and killed a bionic zombie.
  3. Butchered it, confirmed bionics don't show as sterile and have a fault.
  4. Spawn in an autodoc, confirmed it won't allow installing it manually or from autodoc.
  5. Spawned in a grid autoclave, a lab autoclave, an item autoclave, and added a vehicle autoclave to a charged up vehicle.
  6. Confirmed each one is a valid tool for mending the fault the primary way.
  7. Tested examining vehicle autoclaves and that no weird behavior was evident.
  8. Mend the bionic, confirm it shows as sterile and can be installed.
  9. Successfully installed the CBM.
  10. Ran affected JSON files through the online linter.
  11. Checked affected C++ files with astyle.

Additional context

image

image

image

Some concerns:

  1. Mending action takes time and can be interrupted, with no way to resume it. Good news is that it doesn't take charges or other resources until actually completed. Plus, it was already perfectly possible for lack of batteries to interrupt autoclave actions with both time and power lost.
  2. I'm still not 100% sure the mend menu is that easy for players to find.
  3. Mending menu doesn't give you your choice of what tools to use if you have multiple available, e.g. a vehicle autoclave and an item autoclave, etc.

https://www.cdc.gov/infectioncontrol/guidelines/disinfection/disinfection-methods/chemical.html

@chaosvolt
Copy link
Member Author

Note to self: will conflict with #1532 depending on which is merged first.

@Coolthulhu
Copy link
Member

I don't see anything regarding save compatibility. You should detect the case when an item is a bionic and has those 2 flags, remove them, then add the new fault.

Usage of the 'm'end menu is arcane, since it's a rarely used menu. I don't have a good solution here, but giving autoclave an examine action that opens an inventory menu to select a bionic to mend, would make the whole thing much more intuitive. Doesn't even have to limit this mending to autoclave mending, it's enough if it still asks for "sterilize using autoclave/chemicals" after that, as long as it can be done from examine action.

"name": "autoclave",
"description": "This thing is basically an extremely high tech laundry machine or dishwasher. It steams things at temperatures that will kill almost anything.",
"name": "laboratory autoclave",
"description": "This thing is basically an extremely high tech laundry machine or dishwasher. It steams things at temperatures that will kill almost anything. It could be used help to 'm'end bionics that need sterilization.",
Copy link
Member

Choose a reason for hiding this comment

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

Descriptions generally don't name default keybinds.

"time": "80 m",
"skills": [ { "id": "electronics", "level": 1 }, { "id": "firstaid", "level": 3 } ],
"requirements": {
"tools": [ [ [ "autoclave", 5000 ], [ "fake_autoclave", -1 ] ] ],
Copy link
Member

Choose a reason for hiding this comment

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

That second bit shouldn't be needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The fake autoclave is what the laboratory ones use, which don't have grid connections and therefore can't be set to use charges.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, as long as they aren't movable and you have to get down to the lab to use them, it's tolerable.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's basically just replicating their previous infinite usage under the current format, yeah. Fun fact, you COULD drag the autoclaves around, this PR makes them not draggable among other changes.

src/map.cpp Outdated
@@ -5069,6 +5046,24 @@ std::list<item> map::use_charges( const tripoint &origin, const int range,
}
}

if( autoclavepart ) { // we have a veh_forge, now to see what to drain
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to edit the comment when CTRL+C+V-ing.

@Coolthulhu Coolthulhu removed their assignment May 24, 2022
@chaosvolt
Copy link
Member Author

Usage of the 'm'end menu is arcane, since it's a rarely used menu. I don't have a good solution here, but giving autoclave an examine action that opens an inventory menu to select a bionic to mend, would make the whole thing much more intuitive. Doesn't even have to limit this mending to autoclave mending, it's enough if it still asks for "sterilize using autoclave/chemicals" after that, as long as it can be done from examine action.

I'd have to look into how to add that I guess, though it would be odd bringing up the menu from examine action if you then selected

An alternative might be to instead set it so trying to activate a faulty bionic instead brings up the mend menu? Maybe with an added side note saying "you can't self-install this, but want to at least mend it?" if Manual Bionic Installation isn't enabled.

@chaosvolt
Copy link
Member Author

Split off the fix for deconstructing furniture autoclaves as its own PR so that can hopefully get fixed sooner. Now to tinker with the proposed menu/UI ideas sometime today.

[ "chem_hydrogen_peroxide", 50 ],
[ "chem_ethanol", 500 ],
[ "denat_alcohol", 500 ],
[ "bleach", 10 ]
Copy link
Member

Choose a reason for hiding this comment

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

Bleach is very common. Denaturated alcohol isn't allowed for bandages and so is rather low value.
I'd drop those two from the list or bump their numbers to be more comparable to how valuable they are.

Copy link
Member Author

Choose a reason for hiding this comment

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

Given the large amount of dilute hydrogen peroxide required, the latter seems acceptable to me, sure.

Copy link
Member

Choose a reason for hiding this comment

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

Bleach comes in gallon jugs of 30 and is not useful for most stuff. Hydrogen peroxide is a wound disinfectant (maybe shouldn't be, because muh realism) and a small bottle of it could easily be more valuable than a gallon jug of bleach. Especially at the point when you don't have an autoclave yet.

Looks like there is no good way of balancing bleach while keeping its usage reasonable here.

Ethanol is not cheap, it can be used to craft meds. But looks like I was wrong about denaturated alcohol, it only differs from ethanol in that it is not usable for food, but still is for meds.
With 500 alcohol, you can craft 6 bandages or 20 antiseptic.

Copy link
Member Author

@chaosvolt chaosvolt May 26, 2022

Choose a reason for hiding this comment

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

Ah, I can go ahead and remove bleach from the options then. I'd since bumped ethanol and denatured alcohol usage to 1000, should I keep it like that, or bump it up to 1250 (enough to make 50 antiseptic, thus matching amount of hydrogen peroxide used)?

EDIT: Had since then remembered to remove the bleach, doh.

@Coolthulhu Coolthulhu self-assigned this Jun 5, 2022
@Coolthulhu
Copy link
Member

Oh nice, the UI for selecting sterilizable bionics already exists, it only needs to be modified to use the new faults.
game_menus::inv::sterilize_cbm in game_inventory.h/.cpp

The simplest hack would be to call the above function to pick an item, then call avatar_action::mend on it, like

avatar &u = get_avatar();
item_location bionic = game_menus::inv::sterilize_cbm( u );
avatar_action::mend( u, bionic );

This will be imperfect because it will just open the mend menu and not preselect "sterilize with autoclave", but it would suffice for now.

@chaosvolt
Copy link
Member Author

I'll need to work on it either later today or tomorrow, yes. Along with fixing the merge conflict.

@Coolthulhu Coolthulhu merged commit 19693cf into cataclysmbnteam:upload Jun 28, 2022
@Coolthulhu
Copy link
Member

Crap, merged too fast because I mistook it for another PR. I'll revert for now, because UI is important.
Sorry.

Coolthulhu added a commit that referenced this pull request Jun 28, 2022
@chaosvolt
Copy link
Member Author

Just mentioned that on the discord, I saw the merge and was a bit caught off guard by it. I still hadn't made any progress in fixing up the menu.

joveeater pushed a commit to joveeater/Cataclysm-BN that referenced this pull request Jul 10, 2022
* Overhaul sterilizing of CBMs, grid autoclaves

* Update mods

* Update game_inventory.cpp

* Update game_inventory.cpp

* Misc feedback updates

* Couple more bits of feedback

* Remembered to remove bleach

* Update item.cpp
joveeater pushed a commit to joveeater/Cataclysm-BN that referenced this pull request Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants