Respect item MAX_STACK_SIZE in Storage.insert (#5220)

* Respect item MAX_STACK_SIZE in Storage.insert

This fixes the issue described in #5219.

When an item has a stack size specified via
DataComponents.MAX_STACK_SIZE, if the stack size differs from the item's
default stack size, the updated `transfer` methods could accidentally
cause items to be deleted after transfers. This would occur if the
target inventory already contained an identical item. Since getCapacity
used the item type's default size (rather than the size specified by the
item DataComponents), the insert logic would see plenty of room in a
slot already containing items, merge the new item into that slot.

After this occurs, the slot would claim to hold (e.g.) 2 of a max-stack-1 item.
After the next inventory update, the extra item was silently lost.

Changing the getCapacity logic to call toStack() on the variant
correctly prevents the faulty merge and fixes the bug.

* Add getMaxStackSize to ItemVariantImpl

This is utilized in various implementations of getCapacity in the
Storage API to ensure that getCapacity respects stack sizes that have
been set on items via DataComponents.

* Tweak getCapacity docstring

* reorder imports in SingleStackStorage for linter

Author: epiccoleman

fabric-api-lookup-api-v1/src/testmod/java/net/fabricmc/fabric/test/lookup/compat/WrappedInventory.java

@@ -61,7 +61,7 @@ public ItemStack tryInsert(ItemStack input, boolean simulate) {
 				ItemStack stack = inv.getStack(i);
 
 				if (stack.isEmpty() || ItemStack.areItemsAndComponentsEqual(stack, input)) {
-					int remainingSpace = Math.min(inv.getMaxCountPerStack(), stack.getItem().getMaxCount()) - stack.getCount();
+					int remainingSpace = Math.min(inv.getMaxCountPerStack(), stack.getMaxCount()) - stack.getCount();
 					int inserted = Math.min(remainingSpace, input.getCount());
 
 					if (!simulate) {

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/item/base/SingleStackStorage.java

@@ -23,6 +23,7 @@
 import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
 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.ItemVariantImpl;
 
 /**
  * An item variant storage backed by an {@link ItemStack}.
@@ -69,12 +70,12 @@ protected boolean canExtract(ItemVariant itemVariant) {
 	 *
 	 * <p>If the capacity should be limited by the max count of the item, this function must take it into account.
 	 * For example, a storage with a maximum count of 4, or less for items that have a smaller max count,
-	 * should override this to return {@code Math.min(itemVariant.getItem().getMaxCount(), 4);}.
+	 * should override this to return {@code Math.min(super.getCapacity(itemVariant), 4);}.
 	 *
 	 * @return The maximum capacity of this storage for the passed item variant.
 	 */
 	protected int getCapacity(ItemVariant itemVariant) {
-		return itemVariant.getItem().getMaxCount();
+		return ItemVariantImpl.getMaxStackSize(itemVariant);
 	}
 
 	@Override

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

@@ -166,7 +166,7 @@ public long getAmount() {
 
 		@Override
 		public long getCapacity() {
-			return getStack().getItem().getMaxCount();
+			return getStack().getMaxCount();
 		}
 
 		@Override

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

@@ -121,7 +121,7 @@ public int getCapacity(ItemVariant variant) {
 			return 1;
 		}
 
-		return Math.min(storage.inventory.getMaxCountPerStack(), variant.getItem().getMaxCount());
+		return Math.min(storage.inventory.getMaxCountPerStack(), ItemVariantImpl.getMaxStackSize(variant));
 	}
 
 	// We override updateSnapshots to also schedule a markDirty call for the backing inventory.

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

@@ -22,6 +22,7 @@
 
 import net.minecraft.component.ComponentChanges;
 import net.minecraft.component.ComponentMap;
+import net.minecraft.component.DataComponentTypes;
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
 import net.minecraft.item.Items;
@@ -108,6 +109,14 @@ public int hashCode() {
 		return hashCode;
 	}
 
+	/**
+	 * Return the max stack size for this variant, respecting component overrides such as
+	 * {@link DataComponentTypes#MAX_STACK_SIZE}.
+	 */
+	public static int getMaxStackSize(ItemVariant variant) {
+		return variant.getComponentMap().getOrDefault(DataComponentTypes.MAX_STACK_SIZE, variant.getItem().getMaxCount());
+	}
+
 	public ItemStack getCachedStack() {
 		ItemStack ret = cachedStack;
 

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

@@ -132,7 +132,7 @@ protected void onFinalCommit() {
 				long remainder = entry.amount;
 
 				while (remainder > 0) {
-					int dropped = (int) Math.min(entry.key.getItem().getMaxCount(), remainder);
+					int dropped = (int) Math.min(ItemVariantImpl.getMaxStackSize(entry.key), remainder);
 					playerInventory.player.dropItem(entry.key.toStack(dropped), entry.throwRandomly, entry.retainOwnership);
 					remainder -= dropped;
 				}

fabric-transfer-api-v1/src/test/java/net/fabricmc/fabric/test/transfer/unittests/ItemTests.java

@@ -245,6 +245,87 @@ public void testLimitedStackCountItem() {
 		}
 	}
 
+	/**
+	 * Test that {@link DataComponentTypes#MAX_STACK_SIZE} overrides on an item variant are respected.
+	 * Items that are normally stackable (e.g. diamonds, max 64) but have a component override
+	 * reducing their max stack size (e.g. to 1) should not be merged into existing stacks.
+	 *
+	 * <p>This is a regression test for a bug where {@code Item.getMaxCount()} was used instead of
+	 * {@code ItemStack.getMaxCount()}, causing the component override to be ignored.
+	 */
+	@Test
+	public void testComponentMaxStackSizeOverride() {
+		// Create a variant of a normally-stackable item (diamond, default max 64)
+		// with MAX_STACK_SIZE overridden to 1 via components.
+		ComponentChanges components = ComponentChanges.builder()
+				.add(DataComponentTypes.MAX_STACK_SIZE, 1)
+				.build();
+		ItemVariant unstackableDiamond = ItemVariant.of(Items.DIAMOND, components);
+
+		// Also create a normal diamond variant for comparison.
+		ItemVariant normalDiamond = ItemVariant.of(Items.DIAMOND);
+
+		// Test 1: Inserting into a fresh inventory should place 1 per slot.
+		SimpleInventory inventory = new SimpleInventory(3);
+		InventoryStorage wrapper = InventoryStorage.of(inventory, null);
+
+		try (Transaction transaction = Transaction.openOuter()) {
+			// Try to insert 3 unstackable diamonds. Should go into 3 separate slots.
+			long inserted = wrapper.insert(unstackableDiamond, 3, transaction);
+
+			if (inserted != 3) {
+				throw new AssertionError("Should have inserted 3 unstackable diamonds, but inserted " + inserted);
+			}
+
+			// Verify each slot has exactly 1.
+			for (int i = 0; i < 3; i++) {
+				ItemStack stack = inventory.getStack(i);
+
+				if (stack.getCount() != 1) {
+					throw new AssertionError("Slot " + i + " should have count 1, but has " + stack.getCount());
+				}
+			}
+
+			transaction.commit();
+		}
+
+		// Test 2: Inserting into a slot that already has one should NOT merge.
+		SimpleInventory inventory2 = new SimpleInventory(1);
+		InventoryStorage wrapper2 = InventoryStorage.of(inventory2, null);
+
+		try (Transaction transaction = Transaction.openOuter()) {
+			// Insert one first.
+			long first = wrapper2.insert(unstackableDiamond, 1, transaction);
+
+			if (first != 1) {
+				throw new AssertionError("First insert should have succeeded.");
+			}
+
+			// Try to insert another into the same single-slot inventory. Should fail.
+			long second = wrapper2.insert(unstackableDiamond, 1, transaction);
+
+			if (second != 0) {
+				throw new AssertionError("Second insert should have returned 0 (slot full), but inserted " + second);
+			}
+
+			transaction.commit();
+		}
+
+		// Test 3: Normal diamonds (no component override) should still stack to 64.
+		SimpleInventory inventory3 = new SimpleInventory(1);
+		InventoryStorage wrapper3 = InventoryStorage.of(inventory3, null);
+
+		try (Transaction transaction = Transaction.openOuter()) {
+			long inserted = wrapper3.insert(normalDiamond, 100, transaction);
+
+			if (inserted != 64) {
+				throw new AssertionError("Normal diamonds should stack to 64, but inserted " + inserted);
+			}
+
+			transaction.commit();
+		}
+	}
+
 	private static class LimitedStackCountInventory extends SimpleInventory {
 		LimitedStackCountInventory(int size) {
 			super(size);