Forge moves itemstack.mine before breaking the block. Fixes #3516

Signed-off-by: Gabriel Harris-Rouquette <gabizou@me.com>

forge/gradle.properties

@@ -4,4 +4,4 @@ description=The SpongeAPI implementation for MinecraftForge
 
 forgeVersion=36.2.1
 loom.platform=forge
-mixinConfigs=mixins.spongeforge.accessors.json,mixins.spongeforge.api.json,mixins.spongeforge.core.json
+mixinConfigs=mixins.spongeforge.accessors.json,mixins.spongeforge.api.json,mixins.spongeforge.core.json,mixins.spongeforge.tracker.json

forge/src/mixins/java/org/spongepowered/forge/mixin/tracker/server/level/ServerPlayerGameModeMixin_Forge_Tracker.java

@@ -0,0 +1,79 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.forge.mixin.tracker.server.level;
+
+import net.minecraft.core.BlockPos;
+import net.minecraft.server.level.ServerPlayer;
+import net.minecraft.server.level.ServerPlayerGameMode;
+import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.level.Level;
+import net.minecraft.world.level.block.state.BlockState;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+import org.spongepowered.common.event.tracking.PhaseContext;
+import org.spongepowered.common.event.tracking.PhaseTracker;
+import org.spongepowered.common.event.tracking.context.transaction.EffectTransactor;
+import org.spongepowered.common.event.tracking.context.transaction.ResultingTransactionBySideEffect;
+import org.spongepowered.common.event.tracking.context.transaction.TransactionalCaptureSupplier;
+import org.spongepowered.common.event.tracking.context.transaction.effect.InventoryEffect;
+import org.spongepowered.common.event.tracking.context.transaction.inventory.PlayerInventoryTransaction;
+
+@Mixin(ServerPlayerGameMode.class)
+public class ServerPlayerGameModeMixin_Forge_Tracker {
+
+    @Shadow public ServerPlayer player;
+
+    @Redirect(
+        method = "destroyBlock",
+        at = @At(
+            value = "INVOKE",
+            target = "Lnet/minecraft/world/item/ItemStack;mineBlock(Lnet/minecraft/world/level/Level;Lnet/minecraft/world/level/block/state/BlockState;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/entity/player/Player;)V"
+        )
+    )
+    private void forgeTracker$pushTransactionAndEffect(
+        final ItemStack itemStack, final Level level, final BlockState state, final BlockPos pos,
+        final Player player
+    ) {
+        final PhaseContext<@NonNull ?> context = PhaseTracker.SERVER.getPhaseContext();
+        final TransactionalCaptureSupplier transactor = context.getTransactor();
+        // Log the prepare drops here because Forge rewrites
+        // this method to call mine block before calling destroy block
+        // and mods will possibly already have performed modifications on
+        // the block before we can capture it in level.destroy.
+        transactor.logBlockDrops(level, pos, state, level.getBlockEntity(pos));
+
+        itemStack.mineBlock(level, state, pos, player);
+        // Needs to get logged as a sideeffect under the BlockChange
+        try (EffectTransactor ignored = context.getTransactor().pushEffect(new ResultingTransactionBySideEffect(
+            InventoryEffect.getInstance()))) {
+            transactor.logPlayerInventoryChange(this.player, PlayerInventoryTransaction.EventCreator.STANDARD);
+            this.player.inventoryMenu.broadcastChanges();
+        }
+    }
+}

forge/src/mixins/resources/mixins.spongeforge.tracker.json

@@ -0,0 +1,11 @@
+{
+    "required": true,
+    "minVersion": "0.8",
+    "package": "org.spongepowered.forge.mixin.tracker",
+    "target": "@env(DEFAULT)",
+    "compatibilityLevel": "JAVA_8",
+    "priority": 1302,
+    "mixins": [
+        "server.level.ServerPlayerGameModeMixin_Forge_Tracker"
+    ]
+}

src/main/java/org/spongepowered/common/event/tracking/context/transaction/TransactionalCaptureSupplier.java

@@ -97,7 +97,7 @@ public final class TransactionalCaptureSupplier implements ICaptureSupplier, Tra
     public EffectTransactor pushEffect(final ResultingTransactionBySideEffect effect) {
         final GameTransaction<@NonNull ?> parentTransaction = Optional.ofNullable(this.effect)
             .map(child -> (GameTransaction) child.tail)
-            .orElse(Objects.requireNonNull(this.tail));
+            .orElse(Objects.requireNonNull(this.tail, "Somehow pushing a new effect without an owning Transaction"));
         final EffectTransactor effectTransactor = new EffectTransactor(effect, parentTransaction, this.effect, this);
         this.effect = effect;
         parentTransaction.addLast(effect);

src/mixins/java/org/spongepowered/common/mixin/tracker/server/level/ServerPlayerGameModeMixin_Tracker.java

@@ -31,7 +31,6 @@ import net.minecraft.server.level.ServerPlayerGameMode;
 import net.minecraft.world.InteractionHand;
 import net.minecraft.world.InteractionResult;
 import net.minecraft.world.MenuProvider;
-import net.minecraft.world.entity.player.Player;
 import net.minecraft.world.inventory.AbstractContainerMenu;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.context.UseOnContext;
@@ -53,17 +52,13 @@ 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.Redirect;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;
 import org.spongepowered.common.bridge.world.inventory.container.ContainerBridge;
 import org.spongepowered.common.event.SpongeCommonEventFactory;
 import org.spongepowered.common.event.inventory.InventoryEventFactory;
 import org.spongepowered.common.event.tracking.PhaseContext;
 import org.spongepowered.common.event.tracking.PhaseTracker;
-import org.spongepowered.common.event.tracking.context.transaction.EffectTransactor;
-import org.spongepowered.common.event.tracking.context.transaction.ResultingTransactionBySideEffect;
 import org.spongepowered.common.event.tracking.context.transaction.TransactionalCaptureSupplier;
-import org.spongepowered.common.event.tracking.context.transaction.effect.InventoryEffect;
 import org.spongepowered.common.event.tracking.context.transaction.inventory.PlayerInventoryTransaction;
 import org.spongepowered.common.registry.provider.DirectionFacingProvider;
 import org.spongepowered.common.util.VecHelper;
@@ -187,17 +182,4 @@ public abstract class ServerPlayerGameModeMixin_Tracker {
         }
     }
 
-    @Redirect(method = "destroyBlock", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/item/ItemStack;mineBlock(Lnet/minecraft/world/level/Level;Lnet/minecraft/world/level/block/state/BlockState;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/entity/player/Player;)V"))
-    public void impl$onMineBlock(final ItemStack itemStack, final Level param0, final BlockState param1, final BlockPos param2, final Player param3) {
-        final PhaseContext<@NonNull ?> context = PhaseTracker.SERVER.getPhaseContext();
-        final TransactionalCaptureSupplier transactor = context.getTransactor();
-        itemStack.mineBlock(param0, param1, param2, param3);
-        // Needs to get logged as a sideeffect under the BlockChange
-        try (EffectTransactor ignored = context.getTransactor().pushEffect(new ResultingTransactionBySideEffect(InventoryEffect.getInstance()))) {
-            transactor.logPlayerInventoryChange(this.player, PlayerInventoryTransaction.EventCreator.STANDARD);
-            this.player.inventoryMenu.broadcastChanges();
-        }
-    }
-
-
 }

vanilla/gradle.properties

@@ -2,4 +2,4 @@ name=SpongeVanilla
 implementation=Vanilla
 description=The SpongeAPI implementation for Vanilla Minecraft
 
-mixinConfigs=mixins.spongevanilla.api.json,mixins.spongevanilla.core.json
+mixinConfigs=mixins.spongevanilla.api.json,mixins.spongevanilla.core.json,mixins.spongevanilla.tracker.json

vanilla/src/mixins/java/org/spongepowered/vanilla/mixin/tracker/server/level/ServerPlayerGameModeMixin_Vanilla_Tracker.java

@@ -0,0 +1,64 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.vanilla.mixin.tracker.server.level;
+
+import net.minecraft.core.BlockPos;
+import net.minecraft.server.level.ServerPlayer;
+import net.minecraft.server.level.ServerPlayerGameMode;
+import net.minecraft.world.entity.player.Player;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.level.Level;
+import net.minecraft.world.level.block.state.BlockState;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.spongepowered.asm.mixin.Mixin;
+import org.spongepowered.asm.mixin.Shadow;
+import org.spongepowered.asm.mixin.injection.At;
+import org.spongepowered.asm.mixin.injection.Redirect;
+import org.spongepowered.common.event.tracking.PhaseContext;
+import org.spongepowered.common.event.tracking.PhaseTracker;
+import org.spongepowered.common.event.tracking.context.transaction.EffectTransactor;
+import org.spongepowered.common.event.tracking.context.transaction.ResultingTransactionBySideEffect;
+import org.spongepowered.common.event.tracking.context.transaction.TransactionalCaptureSupplier;
+import org.spongepowered.common.event.tracking.context.transaction.effect.InventoryEffect;
+import org.spongepowered.common.event.tracking.context.transaction.inventory.PlayerInventoryTransaction;
+
+@Mixin(ServerPlayerGameMode.class)
+public class ServerPlayerGameModeMixin_Vanilla_Tracker {
+
+    @Shadow public ServerPlayer player;
+
+    @Redirect(method = "destroyBlock", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/item/ItemStack;mineBlock(Lnet/minecraft/world/level/Level;Lnet/minecraft/world/level/block/state/BlockState;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/entity/player/Player;)V"))
+    public void impl$onMineBlock(final ItemStack itemStack, final Level param0, final BlockState param1, final BlockPos param2, final Player param3) {
+        final PhaseContext<@NonNull ?> context = PhaseTracker.SERVER.getPhaseContext();
+        final TransactionalCaptureSupplier transactor = context.getTransactor();
+        itemStack.mineBlock(param0, param1, param2, param3);
+        // Needs to get logged as a sideeffect under the BlockChange
+        try (EffectTransactor ignored = context.getTransactor().pushEffect(new ResultingTransactionBySideEffect(
+            InventoryEffect.getInstance()))) {
+            transactor.logPlayerInventoryChange(this.player, PlayerInventoryTransaction.EventCreator.STANDARD);
+            this.player.inventoryMenu.broadcastChanges();
+        }
+    }
+}

vanilla/src/mixins/resources/mixins.spongevanilla.tracker.json

@@ -0,0 +1,11 @@
+{
+    "required": true,
+    "minVersion": "0.8",
+    "package": "org.spongepowered.vanilla.mixin.tracker",
+    "target": "@env(DEFAULT)",
+    "compatibilityLevel": "JAVA_8",
+    "priority": 1302,
+    "mixins": [
+        "server.level.ServerPlayerGameModeMixin_Vanilla_Tracker"
+    ]
+}