Fix potion effect overriding #11188
Closed
Fix potion effect overriding #11188
+131
−11
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TonytheMacaroni
force-pushed
the
effect-overriding
branch
from
df59454 to
ff14721
Compare
Lulu13022002
reviewed
Aug 8, 2024
patches/server/1047-Fix-EntityPotionEffectEvent-effect-overriding.patch
Outdated
Show resolved
Hide resolved
TonytheMacaroni
force-pushed
the
effect-overriding
branch
2 times, most recently
from
c849a3f to
e68c8b1
Compare
TonytheMacaroni
force-pushed
the
effect-overriding
branch
from
e68c8b1 to
8ebf238
Compare
Member
|
This changes some of the "add effect" methods that have a boolean return value to never return false. It feels like too much of an API break to make this change in methods where the expectation is something else. Perhaps new methods to take advantage of this functionality is better? smth like addCustomEffectWithMerge to clarify that it will "combine" effects utilizing the hidden effect functionality. |
kennytv
added
the
pre-softspoon: never rebased
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed arbitrary restrictions in
CraftAreaEffectCloud,CraftArrowandCraftMetaPotionthat prevented adding multiple potion effects of the same type. There seems to be no such restriction in vanilla, and multiple effects of the same type do apply properly.During testing, I noticed an issue introduced by the handling of
EntityPotionEffectEventthat prevented some potion effects from properly applying, when compared to vanilla. I wasn't entirely sure of the best approach to fix this - as such, I tried sticking to the Javadoc description ofEntityPotionEffectEvent#isOverride, where ifoverrideis true, the previous effect is entirely replaced. As such, a new effect hiding a previous effect is currently not considered "overriding", andEntityPotionEffectEvent#setOverride(true)force applies the new effect, removing the old effect.Also includes a fix for MC-259832. There's a remaining issue with the
/effectcommand improperly considering applying an effect with a lower amplifier, but longer duration as compared to the current effect, as failing to apply, but I'm unsure of how best to fix that. It only impacts messaging, so I did not attempt to fix it.