Add transfer API Chiseled Bookshelf support (#2685)

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/InventorySlotWrapper.java

@@ -71,9 +71,11 @@ class InventorySlotWrapper extends SingleStackStorage {
 	public long insert(ItemVariant insertedVariant, long maxAmount, TransactionContext transaction) {
 		if (!canInsert(slot, ((ItemVariantImpl) insertedVariant).getCachedStack())) {
 			return 0;
-		} else {
-			return super.insert(insertedVariant, maxAmount, transaction);
 		}
+
+		long ret = super.insert(insertedVariant, maxAmount, transaction);
+		if (specialInv != null && ret > 0) specialInv.fabric_onTransfer(slot, transaction);
+		return ret;
 	}
 
 	private boolean canInsert(int slot, ItemStack stack) {
@@ -85,6 +87,13 @@ class InventorySlotWrapper extends SingleStackStorage {
 		}
 	}
 
+	@Override
+	public long extract(ItemVariant variant, long maxAmount, TransactionContext transaction) {
+		long ret = super.extract(variant, maxAmount, transaction);
+		if (specialInv != null && ret > 0) specialInv.fabric_onTransfer(slot, transaction);
+		return ret;
+	}
+
 	/**
 	 * Special cases because vanilla checks the current stack in the following functions (which it shouldn't):
 	 * <ul>

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/SpecialLogicInventory.java

@@ -18,6 +18,8 @@ package net.fabricmc.fabric.impl.transfer.item;
 
 import net.minecraft.item.ItemStack;
 
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+
 /**
  * Internal class that allows inventory instances to defer special logic until {@link InventorySlotWrapper#onFinalCommit()} is called.
  */
@@ -28,4 +30,10 @@ public interface SpecialLogicInventory {
 	void fabric_setSuppress(boolean suppress);
 
 	void fabric_onFinalCommit(int slot, ItemStack oldStack, ItemStack newStack);
+
+	/**
+	 * Called after a slot has been modified (i.e. insert or extract with result > 0).
+	 */
+	default void fabric_onTransfer(int slot, TransactionContext transaction) {
+	}
 }

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/mixin/transfer/ChiseledBookshelfBlockEntityMixin.java

@@ -0,0 +1,93 @@
+/*
+ * 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.ChiseledBookshelfBlockEntity;
+import net.minecraft.inventory.Inventory;
+import net.minecraft.item.ItemStack;
+import net.minecraft.util.collection.DefaultedList;
+
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
+import net.fabricmc.fabric.impl.transfer.item.SpecialLogicInventory;
+
+/**
+ * This mixin tracks the last interacted slot for transaction support, defers block state updates,
+ * and allows setting empty stacks via {@link Inventory#setStack} in a transfer API context (needed for extractions).
+ */
+@Mixin(ChiseledBookshelfBlockEntity.class)
+public class ChiseledBookshelfBlockEntityMixin implements SpecialLogicInventory {
+	@Shadow
+	private DefaultedList<ItemStack> inventory;
+	@Shadow
+	private int field_41601; // last interacted slot
+	@Unique
+	private boolean fabric_suppressSpecialLogic = false;
+
+	@Override
+	public void fabric_setSuppress(boolean suppress) {
+		fabric_suppressSpecialLogic = suppress;
+	}
+
+	@Inject(at = @At("HEAD"), method = "setStack", cancellable = true)
+	public void setStackBypass(int slot, ItemStack stack, CallbackInfo ci) {
+		if (fabric_suppressSpecialLogic) {
+			inventory.set(slot, stack);
+			ci.cancel();
+		}
+	}
+
+	@Shadow
+	private void updateState(int interactedSlot) {
+		throw new AssertionError();
+	}
+
+	@Unique
+	private final SnapshotParticipant<Integer> fabric_lastInteractedParticipant = new SnapshotParticipant<>() {
+		@Override
+		protected Integer createSnapshot() {
+			return field_41601;
+		}
+
+		@Override
+		protected void readSnapshot(Integer snapshot) {
+			field_41601 = snapshot;
+		}
+
+		@Override
+		protected void onFinalCommit() {
+			updateState(field_41601);
+		}
+	};
+
+	@Override
+	public void fabric_onTransfer(int slot, TransactionContext transaction) {
+		fabric_lastInteractedParticipant.updateSnapshots(transaction);
+		field_41601 = slot;
+	}
+
+	@Override
+	public void fabric_onFinalCommit(int slot, ItemStack oldStack, ItemStack newStack) {
+	}
+}

fabric-transfer-api-v1/src/main/resources/fabric-transfer-api-v1.mixins.json

@@ -6,6 +6,7 @@
     "AbstractFurnaceBlockEntityMixin",
     "BucketItemAccessor",
     "BucketItemMixin",
+    "ChiseledBookshelfBlockEntityMixin",
     "DoubleInventoryAccessor",
     "DropperBlockMixin",
     "FluidMixin",

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java

@@ -16,15 +16,18 @@
 
 package net.fabricmc.fabric.test.transfer.gametests;
 
+import net.minecraft.block.Block;
+import net.minecraft.block.BlockState;
 import net.minecraft.block.Blocks;
 import net.minecraft.block.ComparatorBlock;
 import net.minecraft.block.entity.BrewingStandBlockEntity;
-import net.minecraft.block.entity.ChestBlockEntity;
+import net.minecraft.block.entity.ChiseledBookshelfBlockEntity;
 import net.minecraft.block.entity.FurnaceBlockEntity;
 import net.minecraft.block.entity.ShulkerBoxBlockEntity;
 import net.minecraft.inventory.Inventory;
 import net.minecraft.item.ItemStack;
 import net.minecraft.item.Items;
+import net.minecraft.state.property.Properties;
 import net.minecraft.test.GameTest;
 import net.minecraft.test.GameTestException;
 import net.minecraft.test.TestContext;
@@ -87,16 +90,18 @@ public class VanillaStorageTests {
 	}
 
 	/**
-	 * Tests that containers such as chests don't update adjacent comparators until the very end of a committed transaction.
+	 * Tests that the passed block doesn't update adjacent comparators until the very end of a committed transaction.
+	 *
+	 * @param block A block with an Inventory block entity.
+	 * @param variant The variant to try to insert (needs to be supported by the Inventory).
 	 */
-	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
-	public void testChestComparator(TestContext context) {
+	private static void testComparatorOnInventory(TestContext context, Block block, ItemVariant variant) {
 		World world = context.getWorld();
 
 		BlockPos pos = new BlockPos(0, 2, 0);
-		context.setBlockState(pos, Blocks.CHEST.getDefaultState());
-		ChestBlockEntity chest = (ChestBlockEntity) context.getBlockEntity(pos);
-		InventoryStorage storage = InventoryStorage.of(chest, null);
+		context.setBlockState(pos, block.getDefaultState());
+		Inventory inventory = (Inventory) context.getBlockEntity(pos);
+		InventoryStorage storage = InventoryStorage.of(inventory, null);
 
 		BlockPos comparatorPos = new BlockPos(1, 2, 0);
 		// support block under the comparator
@@ -105,7 +110,7 @@ public class VanillaStorageTests {
 		context.setBlockState(comparatorPos, Blocks.COMPARATOR.getDefaultState().with(ComparatorBlock.FACING, Direction.WEST));
 
 		try (Transaction transaction = Transaction.openOuter()) {
-			storage.insert(ItemVariant.of(Items.DIAMOND), 1000000, transaction);
+			storage.insert(variant, 1000000, transaction);
 
 			// uncommitted insert should not schedule an update
 			if (world.getBlockTickScheduler().isQueued(context.getAbsolutePos(comparatorPos), Blocks.COMPARATOR)) {
@@ -123,6 +128,85 @@ public class VanillaStorageTests {
 		context.complete();
 	}
 
+	/**
+	 * Tests that containers such as chests don't update adjacent comparators until the very end of a committed transaction.
+	 */
+	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
+	public void testChestComparator(TestContext context) {
+		testComparatorOnInventory(context, Blocks.CHEST, ItemVariant.of(Items.DIAMOND));
+	}
+
+	/**
+	 * Same as {@link #testChestComparator} but for chiseled bookshelves, because their implementation is very... strange.
+	 */
+	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
+	public void testChiseledBookshelfComparator(TestContext context) {
+		testComparatorOnInventory(context, Blocks.CHISELED_BOOKSHELF, ItemVariant.of(Items.BOOK));
+	}
+
+	/**
+	 * Test for chiseled bookshelves, because their implementation is very... strange.
+	 */
+	@GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE)
+	public void testChiseledBookshelf(TestContext context) {
+		ItemVariant book = ItemVariant.of(Items.BOOK);
+
+		BlockPos pos = new BlockPos(0, 1, 0);
+		context.setBlockState(pos, Blocks.CHISELED_BOOKSHELF.getDefaultState());
+		ChiseledBookshelfBlockEntity bookshelf = (ChiseledBookshelfBlockEntity) context.getBlockEntity(pos);
+		InventoryStorage storage = InventoryStorage.of(bookshelf, null);
+
+		// First, check that we can correctly undo insert operations, because vanilla's setStack doesn't permit it without our patches.
+		try (Transaction transaction = Transaction.openOuter()) {
+			if (storage.insert(book, 2, transaction) != 2) throw new GameTestException("Should have inserted 2 books");
+
+			if (bookshelf.getStack(0).getCount() != 1) throw new GameTestException("Bookshelf stack 0 should have size 1");
+			if (!book.matches(bookshelf.getStack(0))) throw new GameTestException("Bookshelf stack 0 should be a book");
+			if (bookshelf.getStack(1).getCount() != 1) throw new GameTestException("Bookshelf stack 1 should have size 1");
+			if (!book.matches(bookshelf.getStack(1))) throw new GameTestException("Bookshelf stack 1 should be a book");
+		}
+
+		if (!bookshelf.getStack(0).isEmpty()) throw new GameTestException("Bookshelf stack 0 should be empty again after aborting transaction");
+		if (!bookshelf.getStack(1).isEmpty()) throw new GameTestException("Bookshelf stack 1 should be empty again after aborting transaction");
+
+		// Second, check that we correctly update the last modified slot.
+		try (Transaction tx = Transaction.openOuter()) {
+			if (storage.getSlot(1).insert(book, 1, tx) != 1) throw new GameTestException("Should have inserted 1 book");
+			if (bookshelf.method_47887() != 1) throw new GameTestException("Last modified slot should be 1");
+
+			if (storage.getSlot(2).insert(book, 1, tx) != 1) throw new GameTestException("Should have inserted 1 book");
+			if (bookshelf.method_47887() != 2) throw new GameTestException("Last modified slot should be 2");
+
+			if (storage.getSlot(1).extract(book, 1, tx) != 1) throw new GameTestException("Should have extracted 1 book");
+			if (bookshelf.method_47887() != 1) throw new GameTestException("Last modified slot should be 1");
+
+			// Now, create an aborted nested transaction.
+			try (Transaction nested = tx.openNested()) {
+				if (storage.insert(book, 100, nested) != 5) throw new GameTestException("Should have inserted 5 books");
+				// Now, last modified slot should be 5.
+				if (bookshelf.method_47887() != 5) throw new GameTestException("Last modified slot should be 5");
+			}
+
+			// And it's back to 1 in theory.
+			if (bookshelf.method_47887() != 1) throw new GameTestException("Last modified slot should be 1");
+			tx.commit();
+		}
+
+		if (bookshelf.method_47887() != 1) throw new GameTestException("Last modified slot should be 1 after committing transaction");
+
+		// Let's also check the state properties. Only slot 2 should be occupied.
+		BlockState state = bookshelf.getCachedState();
+
+		if (state.get(Properties.SLOT_0_OCCUPIED)) throw new GameTestException("Slot 0 should not be occupied");
+		if (state.get(Properties.SLOT_1_OCCUPIED)) throw new GameTestException("Slot 1 should not be occupied");
+		if (!state.get(Properties.SLOT_2_OCCUPIED)) throw new GameTestException("Slot 2 should be occupied");
+		if (state.get(Properties.SLOT_3_OCCUPIED)) throw new GameTestException("Slot 3 should not be occupied");
+		if (state.get(Properties.SLOT_4_OCCUPIED)) throw new GameTestException("Slot 4 should not be occupied");
+		if (state.get(Properties.SLOT_5_OCCUPIED)) throw new GameTestException("Slot 5 should not be occupied");
+
+		context.complete();
+	}
+
 	/**
 	 * Tests that shulker boxes cannot be inserted into other shulker boxes.
 	 */