From 6cb320661741d073970093de785e21106abfed53 Mon Sep 17 00:00:00 2001 From: OroArmor Date: Thu, 22 Aug 2024 19:24:12 -0700 Subject: [PATCH 1/3] Improve mixin quality for Status Effect API --- library/entity/status_effect/build.gradle | 1 + ...iltLivingEntityStatusEffectExtensions.java | 3 +- .../effect/api/StatusEffectRemovalReason.java | 2 +- .../effect/mixin/EffectCommandMixin.java | 16 +-- .../effect/mixin/LivingEntityMixin.java | 103 ++++++++++++------ .../effect/mixin/MilkBucketItemMixin.java | 10 +- 6 files changed, 86 insertions(+), 49 deletions(-) diff --git a/library/entity/status_effect/build.gradle b/library/entity/status_effect/build.gradle index d909cd82ae..34c7be53a5 100644 --- a/library/entity/status_effect/build.gradle +++ b/library/entity/status_effect/build.gradle @@ -10,6 +10,7 @@ qslModule { moduleDependencies { core { api("qsl_base") + testmodOnly("resourceLoader") } } injectedInterface("net/minecraft/class_1309") { diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/QuiltLivingEntityStatusEffectExtensions.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/QuiltLivingEntityStatusEffectExtensions.java index 567d2ef847..573d48561c 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/QuiltLivingEntityStatusEffectExtensions.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/QuiltLivingEntityStatusEffectExtensions.java @@ -21,6 +21,7 @@ import net.minecraft.entity.LivingEntity; import net.minecraft.entity.effect.StatusEffect; import net.minecraft.entity.effect.StatusEffectInstance; +import net.minecraft.registry.Holder; import org.quiltmc.qsl.base.api.util.InjectedInterface; @@ -36,7 +37,7 @@ public interface QuiltLivingEntityStatusEffectExtensions { * @param reason the reason to remove the status effect * @return {@code true} if the status effect was successfully removed, or {@code false} otherwise. */ - default boolean removeStatusEffect(@NotNull StatusEffect type, @NotNull StatusEffectRemovalReason reason) { + default boolean removeStatusEffect(@NotNull Holder type, @NotNull StatusEffectRemovalReason reason) { throw new UnsupportedOperationException("No implementation of removeStatusEffect could be found."); } diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/StatusEffectRemovalReason.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/StatusEffectRemovalReason.java index 4081bb6c80..30daf888f5 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/StatusEffectRemovalReason.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/api/StatusEffectRemovalReason.java @@ -49,7 +49,7 @@ public class StatusEffectRemovalReason { public static final StatusEffectRemovalReason GENERIC_ALL = new StatusEffectRemovalReason(QuiltStatusEffectInternals.id("generic.all")); /** - * Used when an effect is removed via the vanilla {@link LivingEntity#removeStatusEffect(StatusEffect)} method. + * Used when an effect is removed via the vanilla {@link LivingEntity#removeStatusEffect(net.minecraft.registry.Holder) LivingEntity#removeStatusEffect(Holder<StatusEffect>)} method. */ public static final StatusEffectRemovalReason GENERIC_ONE = new StatusEffectRemovalReason(QuiltStatusEffectInternals.id("generic.one")); diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/EffectCommandMixin.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/EffectCommandMixin.java index f18f6cf88c..4109ffe87d 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/EffectCommandMixin.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/EffectCommandMixin.java @@ -16,9 +16,10 @@ package org.quiltmc.qsl.entity.effect.mixin; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Redirect; import net.minecraft.entity.LivingEntity; import net.minecraft.entity.effect.StatusEffect; @@ -27,24 +28,25 @@ import org.quiltmc.qsl.entity.effect.api.StatusEffectRemovalReason; -@Mixin(EffectCommand.class) +// See LivingEntityMixin +@Mixin(value = EffectCommand.class, priority = 500) public abstract class EffectCommandMixin { - @Redirect( + @WrapOperation( method = "executeClear(Lnet/minecraft/server/command/ServerCommandSource;Ljava/util/Collection;)I", at = @At(value = "INVOKE", target = "Lnet/minecraft/entity/LivingEntity;clearStatusEffects()Z") ) - private static boolean quilt$addRemovalReason(LivingEntity instance) { + private static boolean quilt$addRemovalReason(LivingEntity instance, Operation original) { return instance.clearStatusEffects(StatusEffectRemovalReason.COMMAND_ALL) > 0; } - @Redirect( + @WrapOperation( method = "executeClear(Lnet/minecraft/server/command/ServerCommandSource;Ljava/util/Collection;Lnet/minecraft/registry/Holder;)I", at = @At( value = "INVOKE", target = "Lnet/minecraft/entity/LivingEntity;removeStatusEffect(Lnet/minecraft/registry/Holder;)Z" ) ) - private static boolean quilt$addRemovalReason(LivingEntity instance, Holder type) { - return instance.removeStatusEffect(type.value(), StatusEffectRemovalReason.COMMAND_ONE); + private static boolean quilt$addRemovalReason(LivingEntity instance, Holder type, Operation original) { + return instance.removeStatusEffect(type, StatusEffectRemovalReason.COMMAND_ONE); } } diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java index dae38719cc..1fb685f9bf 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java @@ -16,17 +16,20 @@ package org.quiltmc.qsl.entity.effect.mixin; +import java.util.Collection; +import java.util.Iterator; import java.util.Map; +import com.google.common.collect.Iterators; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import org.jetbrains.annotations.NotNull; import org.spongepowered.asm.mixin.Final; import org.spongepowered.asm.mixin.Mixin; -import org.spongepowered.asm.mixin.Overwrite; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.Unique; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; -import org.spongepowered.asm.mixin.injection.Redirect; import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import net.minecraft.entity.Entity; @@ -41,8 +44,11 @@ import org.quiltmc.qsl.entity.effect.api.StatusEffectRemovalReason; import org.quiltmc.qsl.entity.effect.api.StatusEffectUtils; import org.quiltmc.qsl.entity.effect.impl.QuiltStatusEffectInternals; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -@Mixin(LivingEntity.class) +// We want to make sure that our wrap operations are put before other mods, so that we wrap the vanilla call and not a mods call. +// This is because we do not call the vanilla method, so any mod adding something will not be called. +@Mixin(value = LivingEntity.class, priority = 500) public abstract class LivingEntityMixin extends Entity implements QuiltLivingEntityStatusEffectExtensions { @SuppressWarnings("ConstantConditions") public LivingEntityMixin() { @@ -51,7 +57,7 @@ public LivingEntityMixin() { @Shadow @Final - private Map activeStatusEffects; + private Map, StatusEffectInstance> activeStatusEffects; @Shadow protected abstract void onStatusEffectRemoved(StatusEffectInstance effect); @@ -61,7 +67,7 @@ public LivingEntityMixin() { @SuppressWarnings("ConstantConditions") @Override - public boolean removeStatusEffect(@NotNull StatusEffect type, @NotNull StatusEffectRemovalReason reason) { + public boolean removeStatusEffect(@NotNull Holder type, @NotNull StatusEffectRemovalReason reason) { var effect = this.activeStatusEffects.get(type); if (effect == null) { return false; @@ -117,44 +123,79 @@ public void onStatusEffectRemoved(@NotNull StatusEffectInstance effect, @NotNull StatusEffectEvents.ON_APPLIED.invoker().onApplied((LivingEntity) (Object) this, effect, false); } - @Redirect( + @WrapOperation( method = "onStatusEffectRemoved", at = @At( value = "INVOKE", target = "Lnet/minecraft/entity/effect/StatusEffect;onRemoved(Lnet/minecraft/entity/attribute/AttributeContainer;)V" ) ) - private void quilt$callOnRemovedWithReason(StatusEffect instance, AttributeContainer attributes) { - StatusEffectInstance effect = this.activeStatusEffects.get(instance); + private void quilt$callOnRemovedWithReason(StatusEffect instance, AttributeContainer attributes, Operation original, StatusEffectInstance effect) { instance.onRemoved((LivingEntity) (Object) this, attributes, effect, this.quilt$lastRemovalReason); StatusEffectEvents.ON_REMOVED.invoker().onRemoved((LivingEntity) (Object) this, effect, this.quilt$lastRemovalReason); } - /** - * @author The Quilt Project - * @reason Adding removal reason - */ - @Overwrite - public boolean removeStatusEffect(Holder type) { - return this.removeStatusEffect(type.value(), StatusEffectRemovalReason.GENERIC_ONE); + @Inject( + method = "removeStatusEffect(Lnet/minecraft/registry/Holder;)Z", + at = @At( + value = "HEAD" + ), + cancellable = true + ) + public void quilt$shouldRemoveEffect(Holder effect, CallbackInfoReturnable cir) { + StatusEffectInstance instance = this.activeStatusEffects.get(effect); + if (instance != null) { + if (!StatusEffectUtils.shouldRemove((LivingEntity) (Object) this, instance, StatusEffectRemovalReason.GENERIC_ONE)) { + cir.setReturnValue(false); + } + } + } + + @WrapOperation( + method = "removeStatusEffect(Lnet/minecraft/registry/Holder;)Z", + at = @At( + value = "INVOKE", + target = "Lnet/minecraft/entity/LivingEntity;onStatusEffectRemoved(Lnet/minecraft/entity/effect/StatusEffectInstance;)V" + ) + ) + public void quilt$addRemoveStatusEffectReason(LivingEntity instance, StatusEffectInstance effect, Operation original) { + this.quilt$lastRemovalReason = StatusEffectRemovalReason.GENERIC_ONE; + original.call(instance, effect); + this.quilt$lastRemovalReason = QuiltStatusEffectInternals.UNKNOWN_REASON; } - /** - * @author The Quilt Project - * @reason Adding removal reason - */ - @Overwrite - public boolean clearStatusEffects() { - return this.clearStatusEffects(StatusEffectRemovalReason.GENERIC_ALL) > 0; + @WrapOperation( + method = "clearStatusEffects", + at = @At( + value = "INVOKE", + target = "Ljava/util/Collection;iterator()Ljava/util/Iterator;" + ) + ) + private Iterator quilt$filterStatusEffects(Collection instance, Operation> original) { + return Iterators.filter(original.call(instance), effect -> StatusEffectUtils.shouldRemove( + (LivingEntity) (Object) this, effect, StatusEffectRemovalReason.GENERIC_ALL + )); } - @Redirect(method = "tickStatusEffects", at = @At( + @WrapOperation(method = "tickStatusEffects", at = @At( value = "INVOKE", target = "Lnet/minecraft/entity/LivingEntity;onStatusEffectRemoved(Lnet/minecraft/entity/effect/StatusEffectInstance;)V") ) - private void quilt$removeWithExpiredReason(LivingEntity instance, StatusEffectInstance effect) { - instance.onStatusEffectRemoved(effect, StatusEffectRemovalReason.EXPIRED); - StatusEffectEvents.ON_REMOVED.invoker().onRemoved(instance, effect, StatusEffectRemovalReason.EXPIRED); + private void quilt$removeWithExpiredReason(LivingEntity instance, StatusEffectInstance effect, Operation original) { + this.quilt$lastRemovalReason = StatusEffectRemovalReason.EXPIRED; + original.call(instance, effect); + } + + @WrapOperation( + method = "onStatusEffectUpgraded", + at = @At( + value = "INVOKE", + target = "Lnet/minecraft/entity/effect/StatusEffect;onRemoved(Lnet/minecraft/entity/attribute/AttributeContainer;)V" + ) + ) + private void quilt$removeWithUpgradeApplyingReason(StatusEffect instance, AttributeContainer attributes, Operation original, StatusEffectInstance statusEffectInstance) { + instance.onRemoved((LivingEntity) (Object) this, attributes, statusEffectInstance, StatusEffectRemovalReason.UPGRADE_REAPPLYING); + StatusEffectEvents.ON_REMOVED.invoker().onRemoved((LivingEntity) (Object) this, statusEffectInstance, StatusEffectRemovalReason.UPGRADE_REAPPLYING); } @SuppressWarnings("ConstantConditions") @@ -169,14 +210,4 @@ public boolean clearStatusEffects() { private void quilt$callOnAppliedEvent_upgradeReapplying(StatusEffectInstance effect, boolean reapplyEffect, Entity source, CallbackInfo ci) { StatusEffectEvents.ON_APPLIED.invoker().onApplied((LivingEntity) (Object) this, effect, true); } - - @Redirect(method = "onStatusEffectUpgraded", at = @At( - value = "INVOKE", - target = "Lnet/minecraft/entity/effect/StatusEffect;onRemoved(Lnet/minecraft/entity/attribute/AttributeContainer;)V") - ) - private void quilt$removeWithUpgradeApplyingReason(StatusEffect instance, AttributeContainer attributes) { - StatusEffectInstance effect = this.activeStatusEffects.get(instance); - instance.onRemoved((LivingEntity) (Object) this, attributes, effect, StatusEffectRemovalReason.UPGRADE_REAPPLYING); - StatusEffectEvents.ON_REMOVED.invoker().onRemoved((LivingEntity) (Object) this, effect, StatusEffectRemovalReason.UPGRADE_REAPPLYING); - } } diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/MilkBucketItemMixin.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/MilkBucketItemMixin.java index b2be001815..78797f8034 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/MilkBucketItemMixin.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/MilkBucketItemMixin.java @@ -16,19 +16,21 @@ package org.quiltmc.qsl.entity.effect.mixin; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.injection.At; -import org.spongepowered.asm.mixin.injection.Redirect; import net.minecraft.entity.LivingEntity; import net.minecraft.item.MilkBucketItem; import org.quiltmc.qsl.entity.effect.api.StatusEffectRemovalReason; -@Mixin(MilkBucketItem.class) +// See LivingEntityMixin +@Mixin(value = MilkBucketItem.class, priority = 500) public abstract class MilkBucketItemMixin { - @Redirect(method = "finishUsing", at = @At(value = "INVOKE", target = "Lnet/minecraft/entity/LivingEntity;clearStatusEffects()Z")) - private boolean quilt$addRemovalReason(LivingEntity instance) { + @WrapOperation(method = "finishUsing", at = @At(value = "INVOKE", target = "Lnet/minecraft/entity/LivingEntity;clearStatusEffects()Z")) + private boolean quilt$addRemovalReason(LivingEntity instance, Operation original) { return instance.clearStatusEffects(StatusEffectRemovalReason.DRANK_MILK) > 0; } } From 1edef9f7d00b52e3d886ce4811e9f3bf3822f346 Mon Sep 17 00:00:00 2001 From: OroArmor Date: Thu, 22 Aug 2024 20:23:47 -0700 Subject: [PATCH 2/3] oops --- library/entity/status_effect/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/entity/status_effect/build.gradle b/library/entity/status_effect/build.gradle index 34c7be53a5..f73a00e622 100644 --- a/library/entity/status_effect/build.gradle +++ b/library/entity/status_effect/build.gradle @@ -10,7 +10,7 @@ qslModule { moduleDependencies { core { api("qsl_base") - testmodOnly("resourceLoader") + testmodOnly("resource_loader") } } injectedInterface("net/minecraft/class_1309") { From cdb93289124a4ced5fb33b5f8ebec76f06f89308 Mon Sep 17 00:00:00 2001 From: Eli Orona Date: Sun, 8 Sep 2024 12:36:42 -0700 Subject: [PATCH 3/3] Update library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java Co-authored-by: ix0rai --- .../org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java index 1fb685f9bf..3e0f298112 100644 --- a/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java +++ b/library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/LivingEntityMixin.java @@ -46,7 +46,7 @@ import org.quiltmc.qsl.entity.effect.impl.QuiltStatusEffectInternals; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; -// We want to make sure that our wrap operations are put before other mods, so that we wrap the vanilla call and not a mods call. +// We want to make sure that our wrap operations are put before other mods, so that we wrap the vanilla call and not a mod's call. // This is because we do not call the vanilla method, so any mod adding something will not be called. @Mixin(value = LivingEntity.class, priority = 500) public abstract class LivingEntityMixin extends Entity implements QuiltLivingEntityStatusEffectExtensions {