From e8434230cec5ed675df5c8fac9eccc603bd3f296 Mon Sep 17 00:00:00 2001 From: deirn <deirn@bai.lol> Date: Thu, 11 Jan 2024 22:44:19 +0700 Subject: [PATCH] fix jukebox state getting changed mid-transaction (#3517) --- .../transfer/JukeboxBlockEntityMixin.java | 61 +++++++++++++++++++ .../fabric-transfer-api-v1.mixins.json | 1 + .../gametests/VanillaStorageTests.java | 20 ++++++ 3 files changed, 82 insertions(+) create mode 100644 fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/mixin/transfer/JukeboxBlockEntityMixin.java diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/mixin/transfer/JukeboxBlockEntityMixin.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/mixin/transfer/JukeboxBlockEntityMixin.java new file mode 100644 index 000000000..eb5d1da8d --- /dev/null +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/mixin/transfer/JukeboxBlockEntityMixin.java @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2016, 2017, 2018, 2019 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.fabric.mixin.transfer; + +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.callback.CallbackInfo; + +import net.minecraft.block.entity.JukeboxBlockEntity; +import net.minecraft.item.ItemStack; + +import net.fabricmc.fabric.impl.transfer.item.SpecialLogicInventory; + +@Mixin(JukeboxBlockEntity.class) +public abstract class JukeboxBlockEntityMixin implements SpecialLogicInventory { + @Shadow + private ItemStack recordStack; + + @Shadow + public abstract void setStack(ItemStack stack); + + @Unique + private boolean fabric_suppressSpecialLogic = false; + + @Override + public void fabric_setSuppress(boolean suppress) { + fabric_suppressSpecialLogic = suppress; + } + + @Inject(method = "setStack", at = @At("HEAD"), cancellable = true) + private void setStackBypass(ItemStack stack, CallbackInfo ci) { + if (fabric_suppressSpecialLogic) { + recordStack = stack; + ci.cancel(); + } + } + + @Override + public void fabric_onFinalCommit(int slot, ItemStack oldStack, ItemStack newStack) { + // Call setStack again without suppressing vanilla logic, + // where now the record will actually getting played/stopped. + setStack(newStack); + } +} diff --git a/fabric-transfer-api-v1/src/main/resources/fabric-transfer-api-v1.mixins.json b/fabric-transfer-api-v1/src/main/resources/fabric-transfer-api-v1.mixins.json index 626673dab..f74b8199b 100644 --- a/fabric-transfer-api-v1/src/main/resources/fabric-transfer-api-v1.mixins.json +++ b/fabric-transfer-api-v1/src/main/resources/fabric-transfer-api-v1.mixins.json @@ -12,6 +12,7 @@ "FluidMixin", "HopperBlockEntityMixin", "ItemMixin", + "JukeboxBlockEntityMixin", "LootableContainerBlockEntityMixin", "SimpleInventoryMixin" ] diff --git a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java index 3080a914f..8c33e301e 100644 --- a/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java +++ b/fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java @@ -23,6 +23,7 @@ import net.minecraft.block.BlockState; import net.minecraft.block.Blocks; import net.minecraft.block.ComparatorBlock; import net.minecraft.block.ComposterBlock; +import net.minecraft.block.JukeboxBlock; import net.minecraft.block.entity.BrewingStandBlockEntity; import net.minecraft.block.entity.ChiseledBookshelfBlockEntity; import net.minecraft.block.entity.FurnaceBlockEntity; @@ -356,4 +357,23 @@ public class VanillaStorageTests { context.complete(); } + + /** + * Regression test for <a href="https://github.com/FabricMC/fabric/issues/3485">jukeboxes having their state changed mid-transaction</a>. + */ + @GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE) + public void testJukeboxState(TestContext context) { + BlockPos pos = new BlockPos(2, 2, 2); + context.setBlockState(pos, Blocks.JUKEBOX.getDefaultState()); + Storage<ItemVariant> storage = ItemStorage.SIDED.find(context.getWorld(), context.getAbsolutePos(pos), Direction.UP); + + try (Transaction tx = Transaction.openOuter()) { + storage.insert(ItemVariant.of(Items.MUSIC_DISC_11), 1, tx); + context.checkBlockState(pos, state -> !state.get(JukeboxBlock.HAS_RECORD), () -> "Jukebox should not have its state changed mid-transaction"); + tx.commit(); + } + + context.checkBlockState(pos, state -> state.get(JukeboxBlock.HAS_RECORD), () -> "Jukebox should have its state changed"); + context.complete(); + } }