Improve mixin quality for Status Effect API

build-logic/src/main/java/qsl/internal/Versions.java

@@ -24,7 +24,7 @@ public final class Versions {
 	 * The QSL version
 	 */
 	// Note: Make sure this matches QFAPI's gradle.properties entry for qsl_version
-	public static final String QSL_VERSION = "10.0.0-alpha.3";
+	public static final String QSL_VERSION = "10.0.0-alpha.4";
 
 	/**
 	 * The target Minecraft version.

library/entity/status_effect/build.gradle

@@ -10,6 +10,7 @@ qslModule {
 	moduleDependencies {
 		core {
 			api("qsl_base")
+			testmodOnly("resource_loader")
 		}
 	}
 	injectedInterface("net/minecraft/class_1309") {

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<StatusEffect> type, @NotNull StatusEffectRemovalReason reason) {
 		throw new UnsupportedOperationException("No implementation of removeStatusEffect could be found.");
 	}
 

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"));
 

library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/impl/QuiltStatusEffectInternals.java

@@ -30,6 +30,8 @@ private QuiltStatusEffectInternals() {
 
 	public static final String NAMESPACE = "quilt_status_effect";
 
+	public static final int MIXIN_PRIORITY = 500;
+
 	public static Identifier id(String path) {
 		return Identifier.of(NAMESPACE, path);
 	}

library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/EffectCommandMixin.java

@@ -16,35 +16,38 @@
 
 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;
 import net.minecraft.server.command.EffectCommand;
 import net.minecraft.registry.Holder;
 
 import org.quiltmc.qsl.entity.effect.api.StatusEffectRemovalReason;
+import org.quiltmc.qsl.entity.effect.impl.QuiltStatusEffectInternals;
 
-@Mixin(EffectCommand.class)
+// See LivingEntityMixin
+@Mixin(value = EffectCommand.class, priority = QuiltStatusEffectInternals.MIXIN_PRIORITY)
 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<Boolean> 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<StatusEffect> type) {
-		return instance.removeStatusEffect(type.value(), StatusEffectRemovalReason.COMMAND_ONE);
+	private static boolean quilt$addRemovalReason(LivingEntity instance, Holder<StatusEffect> type, Operation<Boolean> original) {
+		return instance.removeStatusEffect(type, StatusEffectRemovalReason.COMMAND_ONE);
 	}
 }

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 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 = QuiltStatusEffectInternals.MIXIN_PRIORITY)
 public abstract class LivingEntityMixin extends Entity implements QuiltLivingEntityStatusEffectExtensions {
 	@SuppressWarnings("ConstantConditions")
 	public LivingEntityMixin() {
@@ -51,7 +57,7 @@ public LivingEntityMixin() {
 
 	@Shadow
 	@Final
-	private Map<StatusEffect, StatusEffectInstance> activeStatusEffects;
+	private Map<Holder<StatusEffect>, 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<StatusEffect> 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<Void> 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<StatusEffect> 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<StatusEffect> effect, CallbackInfoReturnable<Boolean> 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<Void> 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<StatusEffectInstance> quilt$filterStatusEffects(Collection<StatusEffectInstance> instance, Operation<Iterator<StatusEffectInstance>> 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<Void> 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<Void> 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);
-	}
 }

library/entity/status_effect/src/main/java/org/quiltmc/qsl/entity/effect/mixin/MilkBucketItemMixin.java

@@ -16,19 +16,22 @@
 
 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;
+import org.quiltmc.qsl.entity.effect.impl.QuiltStatusEffectInternals;
 
-@Mixin(MilkBucketItem.class)
+// See LivingEntityMixin
+@Mixin(value = MilkBucketItem.class, priority = QuiltStatusEffectInternals.MIXIN_PRIORITY)
 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<Boolean> original) {
 		return instance.clearStatusEffects(StatusEffectRemovalReason.DRANK_MILK) > 0;
 	}
 }