From 592b141d13fcd4114ace022725eb1fd5d4a2f934 Mon Sep 17 00:00:00 2001 From: Yanang Pearce Date: Fri, 5 Aug 2022 16:53:12 +0800 Subject: [PATCH] Fix ArclightCaptures reset before event being handled (#674) (#675) * Fix ArclightCaptures reset before event being handled (IzzelAliz#674) * Fix ArclightCaptures reset before being handled by event stack (IzzelAliz#674) * Fix ArclightCaptures reset before being handled by event stack (IzzelAliz#674) - resetBlockBreakPlayer -> popPrimaryBlockBreakEvent - handleBlockBreak -> handleBlockDrop - isPrimaryEvent: int -> boolean, as it does not work as intended - Add warnings - Add cleaning to avoid memory leak --- .../management/ServerPlayerGameModeMixin.java | 39 +++++- .../core/world/level/block/BlockMixin.java | 19 +-- .../common/mod/util/ArclightCaptures.java | 118 ++++++++++++++---- 3 files changed, 144 insertions(+), 32 deletions(-) diff --git a/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/server/management/ServerPlayerGameModeMixin.java b/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/server/management/ServerPlayerGameModeMixin.java index 71ccf58e..dc6cfce4 100644 --- a/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/server/management/ServerPlayerGameModeMixin.java +++ b/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/server/management/ServerPlayerGameModeMixin.java @@ -31,6 +31,7 @@ import net.minecraft.world.phys.BlockHitResult; import net.minecraftforge.common.ForgeHooks; import org.bukkit.Bukkit; import org.bukkit.GameMode; +import org.bukkit.block.Block; import org.bukkit.craftbukkit.v.block.CraftBlock; import org.bukkit.craftbukkit.v.event.CraftEventFactory; import org.bukkit.event.Event; @@ -45,6 +46,7 @@ import org.spongepowered.asm.mixin.Overwrite; import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import java.util.List; @@ -210,11 +212,42 @@ public abstract class ServerPlayerGameModeMixin implements PlayerInteractionMana } } + @Inject(method = "destroyBlock", at = @At(value = "INVOKE", target = "Lnet/minecraftforge/common/ForgeHooks;onBlockBreakEvent(Lnet/minecraft/world/level/Level;Lnet/minecraft/world/level/GameType;Lnet/minecraft/server/level/ServerPlayer;Lnet/minecraft/core/BlockPos;)I")) + public void arclight$beforePrimaryEventFired(BlockPos pos, CallbackInfoReturnable cir) { + ArclightCaptures.captureNextBlockBreakEventAsPrimaryEvent(); + } + + @Inject(method = "destroyBlock", at = @At(value = "INVOKE_ASSIGN", target = "Lnet/minecraftforge/common/ForgeHooks;onBlockBreakEvent(Lnet/minecraft/world/level/Level;Lnet/minecraft/world/level/GameType;Lnet/minecraft/server/level/ServerPlayer;Lnet/minecraft/core/BlockPos;)I")) + public void arclight$handleSecondaryBlockBreakEvents(BlockPos pos, CallbackInfoReturnable cir) { + ArclightCaptures.BlockBreakEventContext breakEventContext = ArclightCaptures.popSecondaryBlockBreakEvent(); + while (breakEventContext != null) { + Block block = breakEventContext.getEvent().getBlock(); + handleBlockDrop(breakEventContext, new BlockPos(block.getX(), block.getY(), block.getZ())); + breakEventContext = ArclightCaptures.popSecondaryBlockBreakEvent(); + } + } + @Inject(method = "destroyBlock", at = @At("RETURN")) public void arclight$resetBlockBreak(BlockPos pos, CallbackInfoReturnable cir) { - List blockDrops = ArclightCaptures.getBlockDrops(); - org.bukkit.block.BlockState state = ArclightCaptures.getBlockBreakPlayerState(); - BlockBreakEvent breakEvent = ArclightCaptures.resetBlockBreakPlayer(); + ArclightCaptures.BlockBreakEventContext breakEventContext = ArclightCaptures.popPrimaryBlockBreakEvent(); + + if (breakEventContext != null) { + handleBlockDrop(breakEventContext, pos); + } + } + + @Inject(method = {"tick", "destroyAndAck"}, at = @At(value = "INVOKE", target = "Lnet/minecraft/server/level/ServerPlayerGameMode;destroyBlock(Lnet/minecraft/core/BlockPos;)Z")) + public void arclight$clearCaptures(CallbackInfo ci) { + // clear the event stack in case that interrupted events are left here unhandled + // it should be a new event capture session each time destroyBlock is called from these two contexts + ArclightCaptures.clearBlockBreakEventContexts(); + } + + private void handleBlockDrop(ArclightCaptures.BlockBreakEventContext breakEventContext, BlockPos pos) { + BlockBreakEvent breakEvent = breakEventContext.getEvent(); + List blockDrops = breakEventContext.getBlockDrops(); + org.bukkit.block.BlockState state = breakEventContext.getBlockBreakPlayerState(); + if (blockDrops != null && (breakEvent == null || breakEvent.isDropItems())) { CraftBlock craftBlock = CraftBlock.at(this.level, pos); CraftEventFactory.handleBlockDropItemEvent(craftBlock, state, this.player, blockDrops); diff --git a/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/world/level/block/BlockMixin.java b/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/world/level/block/BlockMixin.java index b284c5b1..5bdef8d1 100644 --- a/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/world/level/block/BlockMixin.java +++ b/arclight-common/src/main/java/io/izzel/arclight/common/mixin/core/world/level/block/BlockMixin.java @@ -94,13 +94,18 @@ public abstract class BlockMixin extends BlockBehaviourMixin implements BlockBri @Inject(method = "playerDestroy", at = @At("RETURN")) private void arclight$handleBlockDrops(Level worldIn, Player player, BlockPos pos, BlockState blockState, BlockEntity te, ItemStack stack, CallbackInfo ci) { - List blockDrops = ArclightCaptures.getBlockDrops(); - org.bukkit.block.BlockState state = ArclightCaptures.getBlockBreakPlayerState(); - BlockBreakEvent breakEvent = ArclightCaptures.resetBlockBreakPlayer(); - if (player instanceof ServerPlayer && blockDrops != null && (breakEvent == null || breakEvent.isDropItems()) - && DistValidate.isValid(worldIn)) { - CraftBlock craftBlock = CraftBlock.at(((CraftWorld) state.getWorld()).getHandle(), pos); - CraftEventFactory.handleBlockDropItemEvent(craftBlock, state, ((ServerPlayer) player), blockDrops); + ArclightCaptures.BlockBreakEventContext breakEventContext = ArclightCaptures.popPrimaryBlockBreakEvent(); + + if (breakEventContext != null) { + BlockBreakEvent breakEvent = breakEventContext.getEvent(); + List blockDrops = breakEventContext.getBlockDrops(); + org.bukkit.block.BlockState state = breakEventContext.getBlockBreakPlayerState(); + + if (player instanceof ServerPlayer && blockDrops != null && (breakEvent == null || breakEvent.isDropItems()) + && DistValidate.isValid(worldIn)) { + CraftBlock craftBlock = CraftBlock.at(((CraftWorld) state.getWorld()).getHandle(), pos); + CraftEventFactory.handleBlockDropItemEvent(craftBlock, state, ((ServerPlayer) player), blockDrops); + } } } } diff --git a/arclight-common/src/main/java/io/izzel/arclight/common/mod/util/ArclightCaptures.java b/arclight-common/src/main/java/io/izzel/arclight/common/mod/util/ArclightCaptures.java index 330cc670..93e81547 100644 --- a/arclight-common/src/main/java/io/izzel/arclight/common/mod/util/ArclightCaptures.java +++ b/arclight-common/src/main/java/io/izzel/arclight/common/mod/util/ArclightCaptures.java @@ -1,6 +1,7 @@ package io.izzel.arclight.common.mod.util; import io.izzel.arclight.common.mod.ArclightConstants; +import io.izzel.arclight.common.mod.ArclightMod; import net.minecraft.core.BlockPos; import net.minecraft.core.Direction; import net.minecraft.world.InteractionHand; @@ -18,6 +19,7 @@ import org.bukkit.event.entity.EntityPotionEffectEvent; import java.util.ArrayList; import java.util.List; +import java.util.Stack; public class ArclightCaptures { @@ -35,35 +37,72 @@ public class ArclightCaptures { } } - private static BlockBreakEvent blockBreakEvent; - private static List blockDrops; - private static BlockState blockBreakPlayerState; + /** + * Indicates that next BlockBreakEvent is fired directly by ServerPlayerGameMode#destroyBlock + * and need to be captured as primary event. + * + * @see net.minecraft.server.level.ServerPlayerGameMode#destroyBlock(BlockPos) + */ + public static boolean isPrimaryEvent = false; + public static Stack blockBreakEventStack = new Stack<>(); + + public static void captureNextBlockBreakEventAsPrimaryEvent() { + // fix #674, some mod will implement their own "destroyBlock(...)" + // and its context cannot be tracked by Arclight directly. + // This is used to tell whether the event is fired by vanilla destroyBlock. + isPrimaryEvent = true; + } public static void captureBlockBreakPlayer(BlockBreakEvent event) { - blockBreakEvent = event; - blockDrops = new ArrayList<>(); - blockBreakPlayerState = event.getBlock().getState(); - } - - public static BlockBreakEvent getBlockBreakPlayer() { - return blockBreakEvent; - } - - public static BlockState getBlockBreakPlayerState() { - return blockBreakPlayerState; + blockBreakEventStack.push(new BlockBreakEventContext(event, isPrimaryEvent)); + isPrimaryEvent = false; } public static List getBlockDrops() { - return blockDrops; + if (!blockBreakEventStack.empty()) { + return blockBreakEventStack.peek().getBlockDrops(); + } + return null; } - public static BlockBreakEvent resetBlockBreakPlayer() { - try { - return blockBreakEvent; - } finally { - blockBreakEvent = null; - blockDrops = null; - blockBreakPlayerState = null; + public static BlockBreakEventContext popPrimaryBlockBreakEvent() { + if (blockBreakEventStack.size() > 0) { + BlockBreakEventContext eventContext = blockBreakEventStack.pop(); + + // deal with unhandled secondary events + // should never happen, but just in case + ArrayList unhandledEvents = new ArrayList<>(); + while (!blockBreakEventStack.empty() && !eventContext.isPrimary()) { + unhandledEvents.add(eventContext); + eventContext = blockBreakEventStack.pop(); + } + + if (unhandledEvents.size() > 0) { + ArclightMod.LOGGER.warn("Unhandled secondary block break event"); + eventContext.mergeAllDrops(unhandledEvents); + } + + return eventContext; + } + else { + return null; + } + } + + public static BlockBreakEventContext popSecondaryBlockBreakEvent() { + if (blockBreakEventStack.size() > 0) { + BlockBreakEventContext eventContext = blockBreakEventStack.peek(); + if (!eventContext.isPrimary()) { + return blockBreakEventStack.pop(); + } + } + return null; + } + + public static void clearBlockBreakEventContexts() { + if (!blockBreakEventStack.empty()) { + ArclightMod.LOGGER.warn("Unhandled block break event"); + blockBreakEventStack.clear(); } } @@ -275,4 +314,39 @@ public class ArclightCaptures { throw new IllegalStateException("Recapturing " + type); } + public static class BlockBreakEventContext { + final private BlockBreakEvent blockBreakEvent; + final private ArrayList blockDrops; + final private BlockState blockBreakPlayerState; + final private boolean primary; + + public BlockBreakEventContext(BlockBreakEvent event, boolean primary) { + this.blockBreakEvent = event; + this.blockDrops = new ArrayList<>(); + this.blockBreakPlayerState = event.getBlock().getState(); + this.primary = primary; + } + + public BlockBreakEvent getEvent() { + return blockBreakEvent; + } + + public ArrayList getBlockDrops() { + return blockDrops; + } + + public BlockState getBlockBreakPlayerState() { + return blockBreakPlayerState; + } + + public void mergeAllDrops(List others) { + for (BlockBreakEventContext other : others) { + this.getBlockDrops().addAll(other.getBlockDrops()); + } + } + + public boolean isPrimary() { + return primary; + } + } }