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

Issue 6303: RFE - Hazardous Liquid Pool #6476

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

Conversation

psikomonkie
Copy link
Collaborator

@psikomonkie psikomonkie commented Jan 31, 2025

Another step towards #6303's completion!

Hazardous Liquid Pools

For map makers: Place this in tiles with water. The level of the Hazardous Liquid Pool will control if it should kick up damaging spray in high winds (not implemented yet) and/or move with water flow (not implemented yet either). You probably shouldn't place this on its own - in order to easily implement the "act as water for movement" the Hazardous Liquid Pool relies on being placed in water.

TODO:

  • Mark these tiles as dangerous for players and princess in the movement phase

Copy link

codecov bot commented Jan 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.57%. Comparing base (bbea8ac) to head (d8d56a2).
Report is 13 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6476      +/-   ##
============================================
- Coverage     28.57%   28.57%   -0.01%     
- Complexity    14407    14421      +14     
============================================
  Files          2798     2800       +2     
  Lines        275146   275291     +145     
  Branches      48678    48703      +25     
============================================
+ Hits          78636    78671      +35     
- Misses       191838   191942     +104     
- Partials       4672     4678       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HammerGS HammerGS added (RFE) Enhancement Requests for Enhancement, new features or implementations For New Dev Cycle This PR should be merged at the beginning of a dev cycle labels Jan 31, 2025
Copy link
Collaborator

@rjhancock rjhancock left a comment

Choose a reason for hiding this comment

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

Overall looks good.

@rjhancock
Copy link
Collaborator

Just saw this was still draft. Sorry.

@psikomonkie
Copy link
Collaborator Author

Just saw this was still draft. Sorry.

No worries! At least one of those things (the formatting on the big If) wasn't going to change - I'd copied it from another (old) method so I think I'll clean it up, extract it into its own method, and use it in both places. I'll request a review from you when I'm done with it all :)

Copy link
Collaborator

@rjhancock rjhancock left a comment

Choose a reason for hiding this comment

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

Overall looks sane.

@psikomonkie psikomonkie marked this pull request as ready for review February 1, 2025 17:54
@psikomonkie
Copy link
Collaborator Author

Hey @rjhancock this is ready for review.

@Sleet01
Copy link
Collaborator

Sleet01 commented Feb 1, 2025

Hey @rjhancock this is ready for review.

I can take a look now, but if this is for next dev cycle maybe we should wait until after 0.50.03 is built tomorrow?

@psikomonkie
Copy link
Collaborator Author

Hey @rjhancock this is ready for review.

I can take a look now, but if this is for next dev cycle maybe we should wait until after 0.50.03 is built tomorrow?

This should wait for the next cycle, yeah, so no rush. Thank you!

|| ((en.getMovementMode() == EntityMovementMode.WIGE)
&& (en.getOriginalWalkMP() > 0) && !eruption))
&& !en.isImmobile()) {
if (!isUnitEffectedByHazardousGround(en, eruption)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For New Dev Cycle This PR should be merged at the beginning of a dev cycle (RFE) Enhancement Requests for Enhancement, new features or implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants