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

Make purifying water take a reasonable amount of time #39973

Closed
wants to merge 24 commits into from

Conversation

SkuliAdams
Copy link
Contributor

@SkuliAdams SkuliAdams commented Apr 27, 2020

Summary

SUMMARY: Bugfixes "Purifying water takes a reasonable amount of time"

Purpose of change

Fixes #38814, Water purifiers take the same time regardless of amount of water

Describe the solution

Previously, purifying water outside of crafting recipes took two seconds regardless of the volume of water purified. This is obviously incorrect. I have split water purifying methods into two categories: filter based purifiers and chemical purifiers.

Chemical purifiers (currently only water purification tablets) still take a constant amount of time, regardless of water purified. This is because the ratio of chemical to water is constant. This PR sets the time taken to 30 minutes.

Filter based purifiers (being the water purifier, charcoal purifier, and lifestraw) now scale the amount of time taken with the volume of water being purified. The base amount of time is two minutes per unit of water, or eight minutes per liter of water. This is further modified by a multiplicative factor that depends on the specific item used. Currently, the electric purifier has a factor of 1 (so it takes the base time), the charcoal purifier has a factor of 2 (as it uses gravity rather than a pump), and the lifestraw has a factor of 4 (due to its small size).

This purification factor is added as a new JSON property, which any new water purifiers should use. To make the purifier take a constant 30 minutes, the factor is set to 0. Otherwise, the factor should be an integer value equal to or greater than 1. This property is compatible with tools and comestibles.

Additionally, purifying water is now an activity, and takes advantage of the activity_actor system.

Describe alternatives you've considered

Because purifying water is now an activity, the player is occupied with the purification for the entire duration. Ideally, the player should be able to set a container of water to be purified and do other things until the purification is complete. However, I am not sure how this would be accomplished, and the same problem already exists for boiling water via crafting. I believe this is a good intermediary step.

Testing

I tested purifying water with all methods of purification available, including the vehicle based purifiers.

Additional context

This is my first time contributing with C++ code, so I would appreciate any suggestions or corrections. Also, if I happened to miss a method of purification, please let me know.

@ifreund ifreund added <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` labels Apr 28, 2020
src/activity_actor.h Outdated Show resolved Hide resolved
src/iuse.cpp Outdated Show resolved Hide resolved
src/iuse.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

One more thing, you'll need to add the deserialize function of your new activity actor to the deserialize_functions map in activity_actor.cpp. (I may have forgotten to put this in the docs)

src/activity_actor.cpp Outdated Show resolved Hide resolved
@SkuliAdams
Copy link
Contributor Author

Is there a way to restart checks now that #39997 and #39999 are merged?

@anothersimulacrum
Copy link
Member

No need, mergers won't hold up your PR because of it.

@John-Candlebury
Copy link
Member

I wouldnt consider this to be any better than the current situation, at least for when it comes to using the water purification tablets. I mean, having your gallon jug of water disinfect in two seconds is clearly wrong, but so is forcing the character to loose half an hour while the tablets dissolve in water.

Should probably look into making the tablets purifying effect work similar to the crafting effect of the charcoal killns.

@SkuliAdams
Copy link
Contributor Author

Should probably look into making the tablets purifying effect work similar to the crafting effect of the charcoal kilns.

I'll look into doing this next - thanks for pointing out that charcoal kilns do something similar.

src/activity_actor.cpp Outdated Show resolved Hide resolved
@ZhilkinSerg
Copy link
Contributor

Feel free to clean branch, make it compile and reopen.

@ZhilkinSerg ZhilkinSerg added the stale Closed for lack of activity, but still valid. label Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Water purifiers take the same time regardless of amount of water
5 participants