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

Water purification tablets only purify up to 4 water each #71971

Merged

Conversation

ZeroInternalReflection
Copy link
Contributor

@ZeroInternalReflection ZeroInternalReflection commented Feb 25, 2024

Summary

Bugfixes "Water purification tablets purify 1-4 water each"

Purpose of change

Attempt 2:
Water purification tablets do not appear to be modeled on any specific real-world item and behave in unexpected ways:

  1. activating the tablets will purify any amount of water selected, and
  2. The "clean water" crafting recipe lists water purification tablets as an option, but can't actually use them in crafting
    Fixes Water purification tablet. Wrong work? #71818

In addition, there are some odd behaviours related to activating/examining/consuming comestibles that I needed to fix to get this working satisfactorily:

  1. If you examine a water source, select the "consume" option, then back out (e.g. because you decide the toilet tank water is too gross, or because you're full), a unit of water is still consumed from the source
  2. If you activate water or certain other comestibles (such as alcohol), it will immediately consume all charges rather than only one

Describe the solution

  1. Pick the 49mg Aquatab as a reference item for behaviour (this is a smaller version of the brand linked by Rokharn below, designed for hiking/camping, which I think is likelier to be the more common style in New England)
  2. Split the tablets out to a new iuse function that figures out how many tablets are in your crafting inventory and using them rather than trying to use charges of an uncharged item
  3. Add a new recipe for using the tablets from the crafting menu where the tablets are a component rather than a tool (tools expect charges, and there doesn't seem to be a way to require a tool or a component)
  4. Add a new water_purifying item that, after 40 minutes, can be activating/consumed to turn into clean water.
  5. Document the usage instructions for the equivalent real tablets in the pur_tablets json so we know what we're abstracting

Tasks remaining:

  • Figure out why you can, in-game, turn "purifying water" into clean water immediately when it's in a toilet tank
  • Increase spawn of the tablets? (I believe they currently spawn in groups of up to 15, but the real-world tablets are sold in packs of 50)
  • Define the water:tablet ratio in json (This could use a second set of eyes, as it doesn't look like many items use this feature)
  • Fix consumption of water via activation
  • Fix handling of consuming water_purifying via examination

Describe alternatives you've considered

  • Restoring charges to pur_tablets
    I think this is how they originally worked, but initial testing didn't seem promising
  • Removing the activation from purification tablets
    I was sorely tempted, but then you can just Eat the tablets to do nothing, which is less than ideal
  • Expanding water quality into clarification/etc
    Not today.

Testing

In-game testing:

  1. Crafting clean water using the tablets works in a 1:4 tablet:water ratio
  2. activating the tablets uses 1 for up to 4 water in the container selected
  3. activating the purifying water 40 minutes later turns it into clean water
  4. Eating the tablet leads to the activation route and works
  5. Eating the purifying water leads to the activation route and works
  6. activating six tablets will turn a toilet tank to water_purifiyng, which becomes clean water after 40 minutes
  7. examining a toilet tank or tank full of water allows you to back out after selecting "consume"
  8. activating various food/drink items follows the same route as Eating them, and adds the appropriate nutrients to the player's stomach
  9. Both existing and new pur_tablets work, though existing ones don't have the variable pulled from json

Unit testing:

  • A variety of combinations of water and tablets in your inventory and sitting around nearby. Yes, every single one of those combinations broke on a previous iteration of the iuse action, why do you ask?
  • water_purifying turns in water_clean after 40 minutes and not earlier, even if the water is old
  • Using purification tablets on water known to cause food poisoning results in water that doesn't cause food poisoning
  • Changing activation to use the consume_activity_actor broke one of the eoc unit tests, since the eoc was structured to expect the character to immediately get the effects from consuming the item. I think my modification still fits the intent of the test

Additional context

@github-actions github-actions bot added Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Feb 25, 2024
@Rokharn
Copy link

Rokharn commented Feb 25, 2024

Interesting moment, most of real world water purification tablets not works instant, but takes time (from 30 min to 4h). Mostly something like 1-2 tablet for 5-10 liters depending of water quality.
For example https://www.cdc.gov/healthywater/pdf/global/posters/11_229310-o_aquatabs_blue_print-africa.pdf
So I am not sure, what source of ingame description of this item. Cannot find prototype from our world, which worlds with just a 250 ml of water per tablet.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 25, 2024
@ZeroInternalReflection
Copy link
Contributor Author

I've been doing some digging trying to figure out the origin of the stated 1:250mL ratio, and can't find any documentation behind it, so I don't know if there was a particular 'model' of tablet in mind. The description change was way back in #5586, and the tablets themselves were added back in 2013: a6b5cd0. The clean water recipe json lists references for purifiers and charcoal filters, but nothing for tablets.

Changing the ratio of tablets:water is (probably) pretty simple if we want to point at a specific tablet like that.

Having the transformation take time makes sense from a 'realism' perspective, but a previous attempt apparently fell apart: #39973. I believe there's infrastructure to turn the water into an item that can be activated into clean water 30 minutes later, but I'd have to look into the code for that.

@Rokharn
Copy link

Rokharn commented Feb 25, 2024

I believe there's infrastructure to turn the water into an item that can be activated into clean water 30 minutes later

Something like fermentation tank mechanics? Sounds pretty similar at least on code logic. I could be wrong, but it seems that we once had large carbon water filters with similar mechanics (but not sure here, in another fork or in mods).

Anyway, I think 1 to 250ml per unit will make tablets mostly useless with current spawn numbers of this kind of items. With this changes needs some re-balance as well, imho.

@PatrikLundell
Copy link
Contributor

A delay process would be good.

In progress items are weird for liquids, though. Back before someone informed me that you could purify water passively with fire I had manual water purification being broken off a lot, resulting with in progress items.

Note that an increase in the amount of water a tablet can purify shouldn't make it mandatory: an early game character may only have access to one or two bottles, and demanding that they'd have 10 (or whatever number) before they could start to purify the water would be bad.
For a similar reason, I think it would make sense to allow a player to purify the water in their starting container, such as a toilet tank. I suspect an interaction might be capable of performing an "up to" calculation that the recipe rigidity doesn't allow for.

@ZeroInternalReflection
Copy link
Contributor Author

The notion | have at the moment (pending sifting through the code for transforming items) is:

  1. via crafting/activating, turn the tablets and water into a "water (purifying)" object
  2. The crafting action should be brief, so that the player can go do something else (and you're unlikely to end up with an 'in-progress' craft for 'drop the tablets in the water')
  3. The "water (purifying)" is either not consumable or is equivalent to drinking regular water (the first is probably easier, but I somehow see someone complaining about it)
  4. After 30 minutes (unless someone finds a better reference for water purification tablets that are faster/designed for smaller volumes), the "water (purifying)" can be activated to turn it into clean water (automatically might also be possible, but I'm not sure)

If we specifically reference the aquatabs that Rokharn found then I'd rework the tablet:water ratio. However, there's probably also a minimum recommended ratio. The link gives "8-10L" and "20L if clear", but is pretty insistent on the 'do not swallow tablets' thing, and they may not be suitable for use in, e.g. a 500mL water bottle.
It would (probably) be fairly straightforward to set up the logic so that one tablet purifies a minimum of 5L and a maximum of 20L. And if that's the logic, then I'd make sure to include tests for it, including one of "the player dropped a tablet in a toilet tank"

To think I only touched this to fix the crafting recipe so I could continue with #60417...

@Rokharn
Copy link

Rokharn commented Feb 26, 2024

to turn it into clean water (automatically might also be possible, but I'm not sure)

For example water, heat and time can automatically turn 'water' into 'clean water' already, mostly same thing, I think. Aftershock ice can be turned into water at similar way.

Another example of similar sounds mechanic (currently ingame too) is 'sauerkraut' and 'wild yeast'. Using some components and container sealing recipe, after some time we can get complete new item or items. This one might work better.

@PatrikLundell
Copy link
Contributor

I think the sealed container recipes suffer from a standard problem with recipes; namely that the container is a fixed part of the recipe result, so while you can vary a container input item through a list of alternatives, the resulting product is always going to be in the same fixed type of container.

@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Items: Food / Vitamins Comestibles and drinks Crafting / Construction / Recipes Includes: Uncrafting / Disassembling labels Feb 27, 2024
Copy link
Contributor

Spell checker encountered unrecognized words in the in-game text added in this pull request. See below for details.

Click to expand
  • Intended for the clarification and disinfection of unsafe drinking water while camping or travelling, this sodium dichloroisocyanurate-based purification tablet removes dangerous contaminants using powerful chemicals. The label says to use one tablet per litre (4 units of water).
  • The chlorine released by the tablets is still working. This isn't ready to drink yet.

This alert is automatically generated. You can simply disregard if this is inaccurate, or (optionally) you can also add the new words to tools/spell_checker/dictionary.txt so they will not trigger an alert next time.

@github-actions github-actions bot removed json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Feb 27, 2024
@ZeroInternalReflection ZeroInternalReflection marked this pull request as draft February 27, 2024 03:29
@ZeroInternalReflection
Copy link
Contributor Author

Alright, this is most of the way to "put in the tablets, walk away, and come back half an hour later to clean water". The tablets create "water (purifying)" which is not yet consumable, but can be transformed into clean water 40 minutes later. The transformation uses the existing mechanism used for sourdough starters and was surprisingly easy to set up.

It also almost works in a toilet tank, but some more testing and debugging is still required, so I've marked it as draft for now.

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Feb 29, 2024
@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. EOC: Effects On Condition Anything concerning Effects On Condition labels Mar 1, 2024
@ZeroInternalReflection ZeroInternalReflection changed the title Water purification tablets purify 1 water each Water purification tablets only purify up to 4 water each Mar 1, 2024
@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display Spawn Creatures, items, vehicles, locations appearing on map labels Mar 4, 2024
@ZeroInternalReflection
Copy link
Contributor Author

ZeroInternalReflection commented Mar 4, 2024

I'm calling this one ready for review (again). There are still a variety of water-quality-related issues, but this PR is already complicated enough.

There are also a couple of bits I'm not sure about, particularly how I go about pulling the water_per_tablet variable from json and the use of test_mode in iuse.cpp, but at this point it needs another set of eyes.

@ZeroInternalReflection ZeroInternalReflection marked this pull request as ready for review March 4, 2024 20:20
@Inglonias
Copy link
Contributor

This looks super cool. I like it!

@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Apr 25, 2024
@osuphobia osuphobia mentioned this pull request May 1, 2024
@RenechCDDA
Copy link
Member

Needs a rebase for clang and some minor touch-ups to switch over to mod_moves instead of touching moves directly.

@Maleclypse
Copy link
Member

@PatrikLundell sorry to bother but when I resolved conflicts on this stale PR it looks like some mapgen changes since then have changed how this should have been written. If you have any suggestions on how to fix it I would greatly appreciate it. If not and I was confused about the cause, my apologies.

@PatrikLundell
Copy link
Contributor

I assume you're referring to test failures (at least something in the first one seems related to my changes)?

I'd need some help, in that case. In order to examine and modify the code in a sane manner I'd need to know how to download it and then push the modified code up again onto the same branch (assuming, of course, it could be fixed). I can get master and my own branches down, but I don't know how to "name" someone else's as I assume the branch name isn't global.

@mqrause
Copy link
Contributor

mqrause commented Sep 10, 2024

In order to examine and modify the code in a sane manner I'd need to know how to download it and then push the modified code up again onto the same branch (assuming, of course, it could be fixed).

The easiest option is probably to add ZeroInternalReflections fork as a remote, then you can just pull the branch this PR is based on. Though I doubt you'll have the permissions to push directly back to it. But if you push your changes to your fork and then make a PR to their branch on their fork, Maleclypse might be able to merge that.

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 <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Crafting / Construction / Recipes Includes: Uncrafting / Disassembling EOC: Effects On Condition Anything concerning Effects On Condition Info / User Interface Game - player communication, menus, etc. Items: Food / Vitamins Comestibles and drinks [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Spawn Creatures, items, vehicles, locations appearing on map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Water purification tablet. Wrong work?
7 participants