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.

Sign up for GitHub

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

Jump to bottom

Un-Hardcode water evaporation & mopping behavior #33399

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

Un-Hardcode water evaporation & mopping behavior #33399

wants to merge 5 commits into from
+40 −19

Conversation

Zap527
Copy link
Contributor

@Zap527 Zap527 commented Nov 18, 2024 •
edited
Loading

About the PR

Made evaporation and mopping logic more generic

Why / Balance

Water = "water"
On a more serious note, this is needed for downstreams, and hardcoding behavior with magic strings is not good.

Technical details

Added two new parameters to reagents, which determines if they evaporate and if they can be used to mop or not.
Removed hardcoded water evaporation and mopping behavior and made it depend on the reagent's evaporation/mopping parameter.

Requirements

Breaking changes

EvaporationReagents was removed in favor of GetEvaporatingReagents(solution) and GetMoppableReagents(solution)

Changelog
Not player facing

@github-actions github-actions bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/M Denotes a PR that changes 100-999 lines. labels Nov 18, 2024
@beck-thompson beck-thompson added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. T: New Feature Type: New feature or content, or extending existing content T: Cleanup Type: Code clean-up, without being a full refactor or feature D2: Medium Difficulty: A good amount of codebase knowledge required. S: Needs Review Status: Requires additional reviews before being fully accepted A: General Interactions Area: General in-game interactions that don't relate to another area. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Nov 18, 2024
@ScarKy0 ScarKy0 added P1: High Priority: Higher priority than other items, but isn't an emergency. A: Core Tech Area: Underlying core tech for the game and the Github repository. and removed P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. A: General Interactions Area: General in-game interactions that don't relate to another area. labels Nov 18, 2024
@Zap527 Zap527 changed the title Evaporation refactor Un-Hardcode water evaporation behavior Nov 18, 2024
@dvir001
Copy link
Contributor

dvir001 commented Nov 18, 2024

Please make sure AbsorbentSystem only works with water in the case you add more regents with evaporation
new-frontiers-14/frontier-station-14#1810

Unless you want to mop blood with puke.

@Zap527
Copy link
Contributor Author

Zap527 commented Nov 18, 2024

Tests were failing for unrelated reasons but I have now fixed it, yes.
My tests are broken locally and I can't run them, so I get to wait in anticipation to see if my code works or not!

@Zap527 Zap527 changed the title Un-Hardcode water evaporation behavior Un-Hardcode water evaporation & mopping behavior Nov 18, 2024
@Zap527 Zap527 mentioned this pull request Nov 18, 2024
Draft
2 tasks
@slarticodefast slarticodefast added P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. and removed P1: High Priority: Higher priority than other items, but isn't an emergency. labels Nov 26, 2024
@slarticodefast slarticodefast mentioned this pull request Jan 8, 2025
Open
2 tasks
@dvir001
Copy link
Contributor

dvir001 commented Jan 9, 2025 •
edited
Loading

@Zap527 Can you please make holy water and ice also work the same as water in this PR?

@Zap527
Copy link
Contributor Author

Zap527 commented Jan 9, 2025

I could, but I feel like this might be something which would be better suited for a follow-up PR. I don't think currently ice and holy water evaporate? Ideally I don't change game behavior with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A: Core Tech Area: Underlying core tech for the game and the Github repository. D2: Medium Difficulty: A good amount of codebase knowledge required. P2: Raised Priority: Item has a raised priority, indicating it might get increased maintainer attention. S: Needs Review Status: Requires additional reviews before being fully accepted size/M Denotes a PR that changes 100-999 lines. T: Cleanup Type: Code clean-up, without being a full refactor or feature T: New Feature Type: New feature or content, or extending existing content
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Zap527 @dvir001 @ScarKy0 @beck-thompson @slarticodefast