From ecdae055a4e91481b7335542ce2f00eb3778fa90 Mon Sep 17 00:00:00 2001 From: Ampflower Date: Tue, 25 Feb 2025 01:26:18 -0800 Subject: [PATCH] fix: Use MixinExtras over ThreadLocal (#130) * Fix: Always clear fall event If the event hasn't been modified, this can temporarily leak an entity and its world, causing memory usage to inflate on the client. * Port fall event to MixinExtras This removes the need to store a global variable. * Use Share local for fromDimHolder * Use Share local for balmCurrentUseEvent This could've ideally just had `@Cancellable CallbackInfo` but it appears that this has an older Mixin Extras. * Use Share local for getGrowthSpeedBlockState, renamed to farmBlock --- .../mods/balm/mixin/FabricCropBlockMixin.java | 13 +++++------ .../mods/balm/mixin/LivingEntityMixin.java | 22 +++++++++---------- .../mods/balm/mixin/MinecraftMixin.java | 13 ++++------- .../mods/balm/mixin/ServerPlayerMixin.java | 13 +++++------ 4 files changed, 26 insertions(+), 35 deletions(-) diff --git a/fabric/src/main/java/net/blay09/mods/balm/mixin/FabricCropBlockMixin.java b/fabric/src/main/java/net/blay09/mods/balm/mixin/FabricCropBlockMixin.java index b0f2faaa..f717cd18 100644 --- a/fabric/src/main/java/net/blay09/mods/balm/mixin/FabricCropBlockMixin.java +++ b/fabric/src/main/java/net/blay09/mods/balm/mixin/FabricCropBlockMixin.java @@ -1,5 +1,7 @@ package net.blay09.mods.balm.mixin; +import com.llamalad7.mixinextras.sugar.Share; +import com.llamalad7.mixinextras.sugar.ref.LocalRef; import net.blay09.mods.balm.api.Balm; import net.blay09.mods.balm.api.block.CustomFarmBlock; import net.blay09.mods.balm.api.event.CropGrowEvent; @@ -20,8 +22,6 @@ @Mixin(CropBlock.class) public class FabricCropBlockMixin { - private static final ThreadLocal getGrowthSpeedBlockState = new ThreadLocal<>(); - @Inject(method = "randomTick(Lnet/minecraft/world/level/block/state/BlockState;Lnet/minecraft/server/level/ServerLevel;Lnet/minecraft/core/BlockPos;Lnet/minecraft/util/RandomSource;)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/server/level/ServerLevel;setBlock(Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/level/block/state/BlockState;I)Z"), cancellable = true) public void randomTickPreGrow(BlockState state, ServerLevel level, BlockPos pos, RandomSource random, CallbackInfo callbackInfo) { CropGrowEvent.Pre event = new CropGrowEvent.Pre(level, pos, state); @@ -38,17 +38,16 @@ public void randomTickPostGrow(BlockState state, ServerLevel level, BlockPos pos } - @ModifyVariable(method = "getGrowthSpeed(Lnet/minecraft/world/level/block/Block;Lnet/minecraft/world/level/BlockGetter;Lnet/minecraft/core/BlockPos;)F", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/BlockState;is(Lnet/minecraft/world/level/block/Block;)Z", ordinal = 0)) - private static BlockState getGrowthSpeedCaptureLocals(BlockState state) { - getGrowthSpeedBlockState.set(state); + private static BlockState getGrowthSpeedCaptureLocals(BlockState state, @Share("farmBlock") LocalRef farmBlock) { + farmBlock.set(state); return state; } @ModifyVariable(method = "getGrowthSpeed(Lnet/minecraft/world/level/block/Block;Lnet/minecraft/world/level/BlockGetter;Lnet/minecraft/core/BlockPos;)F", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/BlockState;is(Lnet/minecraft/world/level/block/Block;)Z"), ordinal = 1) - private static float getGrowthSpeed(float g, Block block, BlockGetter blockGetter, BlockPos pos) { - BlockState state = getGrowthSpeedBlockState.get(); + private static float getGrowthSpeed(float g, Block block, BlockGetter blockGetter, BlockPos pos, @Share("farmBlock") LocalRef farmBlock) { + BlockState state = farmBlock.get(); if (state.getBlock() instanceof CustomFarmBlock customFarmBlock) { if (customFarmBlock.canSustainPlant(state, blockGetter, pos, Direction.UP, block)) { if (customFarmBlock.isFertile(state, blockGetter, pos)) { diff --git a/fabric/src/main/java/net/blay09/mods/balm/mixin/LivingEntityMixin.java b/fabric/src/main/java/net/blay09/mods/balm/mixin/LivingEntityMixin.java index c1ac66c7..e4c7ffe5 100644 --- a/fabric/src/main/java/net/blay09/mods/balm/mixin/LivingEntityMixin.java +++ b/fabric/src/main/java/net/blay09/mods/balm/mixin/LivingEntityMixin.java @@ -1,5 +1,9 @@ package net.blay09.mods.balm.mixin; +import com.llamalad7.mixinextras.injector.wrapoperation.Operation; +import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation; +import com.llamalad7.mixinextras.sugar.Share; +import com.llamalad7.mixinextras.sugar.ref.LocalRef; import net.blay09.mods.balm.api.Balm; import net.blay09.mods.balm.api.event.LivingDamageEvent; import net.blay09.mods.balm.api.event.LivingFallEvent; @@ -8,7 +12,6 @@ import net.minecraft.world.damagesource.DamageSource; import net.minecraft.world.entity.LivingEntity; import org.spongepowered.asm.mixin.Mixin; -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.ModifyVariable; @@ -17,9 +20,6 @@ @Mixin(LivingEntity.class) public abstract class LivingEntityMixin { - @Unique - private static final ThreadLocal balmCurrentFallEvent = new ThreadLocal<>(); - @ModifyVariable(method = "actuallyHurt(Lnet/minecraft/server/level/ServerLevel;Lnet/minecraft/world/damagesource/DamageSource;F)V", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/LivingEntity;setAbsorptionAmount(F)V"), argsOnly = true) private float actuallyHurt(float damageAmount, ServerLevel serverLevel, DamageSource damageSource) { LivingDamageEvent event = new LivingDamageEvent((LivingEntity) (Object) this, damageSource, damageAmount); @@ -32,22 +32,22 @@ private float actuallyHurt(float damageAmount, ServerLevel serverLevel, DamageSo } @Inject(method = "causeFallDamage(FFLnet/minecraft/world/damagesource/DamageSource;)Z", at = @At("HEAD"), cancellable = true) - private void causeFallDamage(float distance, float damageMultiplier, DamageSource damageSource, CallbackInfoReturnable callbackInfo) { + private void causeFallDamage(float distance, float damageMultiplier, DamageSource damageSource, CallbackInfoReturnable callbackInfo, @Share("eventRef") LocalRef eventRef) { LivingFallEvent event = new LivingFallEvent((LivingEntity) (Object) this); Balm.getEvents().fireEvent(event); if (event.isCanceled()) { callbackInfo.setReturnValue(false); } - balmCurrentFallEvent.set(event); + eventRef.set(event); } - @Inject(method = "calculateFallDamage(FF)I", at = @At("RETURN"), cancellable = true) - private void calculateFallDamage(float f, float g, CallbackInfoReturnable cir) { - LivingFallEvent event = balmCurrentFallEvent.get(); + @WrapOperation(method = "causeFallDamage(FFLnet/minecraft/world/damagesource/DamageSource;)Z", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/entity/LivingEntity;calculateFallDamage(FF)I")) + private int calculateFallDamage(LivingEntity self, float fallDistance, float multiplier, Operation operation, @Share("eventRef") LocalRef eventRef) { + LivingFallEvent event = eventRef.get(); if (event != null && event.getFallDamageOverride() != null) { - cir.setReturnValue(event.getFallDamageOverride().intValue()); - balmCurrentFallEvent.set(null); + return event.getFallDamageOverride().intValue(); } + return operation.call(self, fallDistance, multiplier); } @ModifyVariable(method = "heal(F)V", at = @At("HEAD"), argsOnly = true) diff --git a/fabric/src/main/java/net/blay09/mods/balm/mixin/MinecraftMixin.java b/fabric/src/main/java/net/blay09/mods/balm/mixin/MinecraftMixin.java index 2cf7cf85..bb7bf152 100644 --- a/fabric/src/main/java/net/blay09/mods/balm/mixin/MinecraftMixin.java +++ b/fabric/src/main/java/net/blay09/mods/balm/mixin/MinecraftMixin.java @@ -1,5 +1,7 @@ package net.blay09.mods.balm.mixin; +import com.llamalad7.mixinextras.sugar.Share; +import com.llamalad7.mixinextras.sugar.ref.LocalRef; import net.blay09.mods.balm.api.Balm; import net.blay09.mods.balm.api.event.LevelLoadingEvent; import net.blay09.mods.balm.api.event.client.OpenScreenEvent; @@ -13,7 +15,6 @@ import org.objectweb.asm.Opcodes; import org.spongepowered.asm.mixin.Mixin; 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.ModifyArg; @@ -28,9 +29,6 @@ public class MinecraftMixin { @Shadow public ClientLevel level; - @Unique - private static final ThreadLocal balmCurrentUseEvent = new ThreadLocal<>(); - @ModifyVariable(method = "setScreen(Lnet/minecraft/client/gui/screens/Screen;)V", at = @At(value = "FIELD", target = "Lnet/minecraft/client/Minecraft;screen:Lnet/minecraft/client/gui/screens/Screen;", opcode = Opcodes.GETFIELD, shift = At.Shift.AFTER), argsOnly = true) public Screen modifyScreen(Screen screen) { OpenScreenEvent event = new OpenScreenEvent(screen); @@ -39,22 +37,19 @@ public Screen modifyScreen(Screen screen) { } @Inject(method = "startUseItem()V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/player/LocalPlayer;getItemInHand(Lnet/minecraft/world/InteractionHand;)Lnet/minecraft/world/item/ItemStack;", shift = At.Shift.AFTER), cancellable = true) - public void startUseItem(CallbackInfo callbackInfo) { + public void startUseItem(CallbackInfo callbackInfo, @Share("currentUseEvent") LocalRef balmCurrentUseEvent) { final var event = balmCurrentUseEvent.get(); if (event != null && event.isCanceled()) { callbackInfo.cancel(); } - balmCurrentUseEvent.remove(); } @ModifyArg(method = "startUseItem()V", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/player/LocalPlayer;getItemInHand(Lnet/minecraft/world/InteractionHand;)Lnet/minecraft/world/item/ItemStack;"), index = 0) - public InteractionHand modifyHand(InteractionHand hand) { + public InteractionHand modifyHand(InteractionHand hand, @Share("currentUseEvent") LocalRef balmCurrentUseEvent) { if (this.hitResult != null && this.hitResult.getType() != HitResult.Type.MISS) { UseItemInputEvent event = new UseItemInputEvent(hand); Balm.getEvents().fireEvent(event); balmCurrentUseEvent.set(event); - } else { - balmCurrentUseEvent.remove(); } return hand; } diff --git a/fabric/src/main/java/net/blay09/mods/balm/mixin/ServerPlayerMixin.java b/fabric/src/main/java/net/blay09/mods/balm/mixin/ServerPlayerMixin.java index 9aaaecc9..a8de90e1 100644 --- a/fabric/src/main/java/net/blay09/mods/balm/mixin/ServerPlayerMixin.java +++ b/fabric/src/main/java/net/blay09/mods/balm/mixin/ServerPlayerMixin.java @@ -1,5 +1,7 @@ package net.blay09.mods.balm.mixin; +import com.llamalad7.mixinextras.sugar.Share; +import com.llamalad7.mixinextras.sugar.ref.LocalRef; import net.blay09.mods.balm.api.Balm; import net.blay09.mods.balm.api.event.PlayerChangedDimensionEvent; import net.blay09.mods.balm.api.event.PlayerOpenMenuEvent; @@ -7,14 +9,12 @@ import net.minecraft.resources.ResourceKey; import net.minecraft.server.level.ServerPlayer; import net.minecraft.world.MenuProvider; -import net.minecraft.world.entity.Entity; import net.minecraft.world.entity.player.Inventory; import net.minecraft.world.item.ItemStack; import net.minecraft.world.level.Level; import net.minecraft.world.level.portal.TeleportTransition; import org.jetbrains.annotations.Nullable; import org.spongepowered.asm.mixin.Mixin; -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.callback.CallbackInfoReturnable; @@ -24,9 +24,6 @@ @Mixin(ServerPlayer.class) public class ServerPlayerMixin { - @Unique - private static final ThreadLocal> fromDimHolder = new ThreadLocal<>(); - @Inject(method = "openMenu(Lnet/minecraft/world/MenuProvider;)Ljava/util/OptionalInt;", at = @At("RETURN")) public void openMenu(@Nullable MenuProvider menuProvider, CallbackInfoReturnable callbackInfo) { ServerPlayer player = (ServerPlayer) (Object) this; @@ -34,17 +31,17 @@ public void openMenu(@Nullable MenuProvider menuProvider, CallbackInfoReturnable } @Inject(method = "teleport(Lnet/minecraft/world/level/portal/TeleportTransition;)Lnet/minecraft/server/level/ServerPlayer;", at = @At("HEAD")) - public void teleportHead(TeleportTransition transition, CallbackInfoReturnable callbackInfo) { + public void teleportHead(TeleportTransition transition, CallbackInfoReturnable callbackInfo, @Share("fromDimHolder") LocalRef> fromDimHolder) { ServerPlayer player = (ServerPlayer) (Object) this; fromDimHolder.set(player.serverLevel().dimension()); } @Inject(method = "teleport(Lnet/minecraft/world/level/portal/TeleportTransition;)Lnet/minecraft/server/level/ServerPlayer;", at = @At("RETURN")) - public void teleportTail(TeleportTransition transition, CallbackInfoReturnable callbackInfo) { + public void teleportTail(TeleportTransition transition, CallbackInfoReturnable callbackInfo, @Share("fromDimHolder") LocalRef> fromDimHolder) { ServerPlayer player = (ServerPlayer) (Object) this; final ResourceKey fromDim = fromDimHolder.get(); final ResourceKey toDim = transition.newLevel().dimension(); - if(!fromDim.equals(toDim)) { + if (!fromDim.equals(toDim)) { Balm.getEvents().fireEvent(new PlayerChangedDimensionEvent(player, fromDim, toDim)); } }