AttributePotionDuration#modifyPotionDuration does not distinguish between different potion levels
sam-kirby opened this issue ยท 1 comments
Test Environment
Minecraft 1.12.2
Forge 14.23.5.2838
Astral Sorcery 1.0.20
Baubles 1.5.2
Description of issue
If a player has an effect, receiving an effect of the same type with a longer duration will cause the original effect to be extended regardless of level.
Example
When a player drinks a potion of strength level 2 (1:30), followed by a potion of strength level 1 (8:00), they receive strength level 2 for 8 minutes with no astral perks/progression.
What should happen
The strength level 1 potion should have no effect.
Why this happens
PotionApplyEvent.Changed is fired on drinking the second potion. Strength 1 and Strength 2 effects are treated as the same and astral sorcery modifies the duration of the new effect despite this being of a different level to the added effect.
Consequences
In vanilla this can be exploited to raise the effect level of beacons and lower level potions.
In modded environments, the effect can be even worse, with players becoming invulnerable in SevTech. DarkPacks/SevTech-Ages#3874
Possible Fix
diff --git a/src/main/java/hellfirepvp/astralsorcery/common/constellation/perk/attribute/type/AttributePotionDuration.java b/src/main/java/hellfirepvp/astralsorcery/common/constellation/perk/attribute/type/AttributePotionDuration.java
index 342c2505..c9ffcfab 100644
--- a/src/main/java/hellfirepvp/astralsorcery/common/constellation/perk/attribute/type/AttributePotionDuration.java
+++ b/src/main/java/hellfirepvp/astralsorcery/common/constellation/perk/attribute/type/AttributePotionDuration.java
@@ -48,7 +48,7 @@ public class AttributePotionDuration extends PerkAttributeType {
}
private void modifyPotionDuration(EntityPlayer player, PotionEffect newSetEffect, PotionEffect addedEffect) {
- if (player.world.isRemote) {
+ if (player.world.isRemote || addedEffect.getAmplifier() < newSetEffect.getAmplifier()) {
return;
}
Edit: made modifyPotionDuration apply if added effect amplifier is greater or equal to the result of combination instead of just exactly equal, as it is also true that in the case it is greater the resulting new effect will apply.
Huh... I was wondering why higher level potion effects weren't going away despite having permanent lower level potion effects, such as Strength Lv 1-2 via Metallurgy or Genetics Reborn and using Crimson Gems from Betweenlands which can give up to Strength Lv3 for a few seconds when attacking mobs.
I think I recall Botania's Mana in a Bottle... when drunk will fling you into the air but you get a high Resistance Potion Effect. That's quite an exploit since you'll be able to keep that effect if you have a lower resist potion effect in play.