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

fix: cleanse behavior #1813

Closed
wants to merge 1 commit into from
Closed

Conversation

Arufonsu
Copy link
Contributor

@Arufonsu Arufonsu commented May 11, 2023

Refactors and fix cleanse status effect behavior so it will now:

  • Properly dispel currently active "unfriendly" Statuses and DoTs - also skips itself from being dispelled.
  • Prevent/block "unfriendly" Statuses and DoTs while active over time.
  • Should resolve bug: cleanse behavior #1812

Preview:

2023-05-11.15-52-08.mp4

@Arufonsu Arufonsu requested review from lodicolo, jcsnider, WeylonSantana and a team May 11, 2023 20:38
Refactors and fix cleanse status effect behavior so it will now:
- Properly dispel currently active "unfriendly" Statuses and DoTs - also skips itself from being dispelled.
- Prevent/block "unfriendly" Statuses and DoTs while active over time.
@lodicolo
Copy link
Member

lodicolo commented Jul 6, 2023

This PR just doesn't make any sense. Why are other statuses being dispelled in the constructor for a DoT effect...? The cleanse handling in the constructor was to prevent adding a new DoT.

@Arufonsu
Copy link
Contributor Author

The cleanse handling in the constructor was to prevent adding a new DoT

Because DoTs cast statuses and well, now cleanse not only prevents unfriendly spells/debuffs but also cleans up the active ones

@lodicolo
Copy link
Member

The cleanse handling in the constructor was to prevent adding a new DoT

Because DoTs cast statuses and well, now cleanse not only prevents unfriendly spells/debuffs but also cleans up the active ones

tl;dr: I do not disagree with cleaning up existing statuses that are incompatible with Cleanse, but the existing statuses should be cleaned up when the Cleanse is added, not in the constructor for a DoT later on.

Yeah but my point was that in the effect handler that added the cleanse effect in the first place everything should have been cleaned up. The DoT constructor should just "abort" if it can't be added, but it shouldn't clean up a mess that should have already been cleaned up. Otherwise none of those statuses would get cleaned up unless the DoT is added, what if a DoT is never added on the player until after the cleanse expires? Then the other statuses would just be on the player when they shouldn't be.

@Arufonsu Arufonsu marked this pull request as draft August 28, 2023 02:52
@Arufonsu Arufonsu added the bug Something isn't working label Sep 10, 2023
@Arufonsu Arufonsu closed this Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: cleanse behavior
2 participants