From cd10d4fc03f68fecced2ab7886a1ec3d979252e4 Mon Sep 17 00:00:00 2001 From: Technici4n <13494793+Technici4n@users.noreply.github.com> Date: Sun, 20 Nov 2022 14:35:10 +0100 Subject: [PATCH] Fix #2657: Transfer API edge case with bad isValid overrides (#2659) Co-authored-by: modmuss50 --- .../transfer/item/InventorySlotWrapper.java | 20 ++++++++ .../gametests/VanillaStorageTests.java | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/InventorySlotWrapper.java b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/InventorySlotWrapper.java index 03045adf2..e4f84a55b 100644 --- a/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/InventorySlotWrapper.java +++ b/fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/item/InventorySlotWrapper.java @@ -16,8 +16,11 @@ package net.fabricmc.fabric.impl.transfer.item; +import net.minecraft.block.entity.AbstractFurnaceBlockEntity; +import net.minecraft.block.entity.BrewingStandBlockEntity; import net.minecraft.block.entity.ShulkerBoxBlockEntity; import net.minecraft.item.ItemStack; +import net.minecraft.item.Items; import net.fabricmc.fabric.api.transfer.v1.item.base.SingleStackStorage; import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant; @@ -82,8 +85,25 @@ class InventorySlotWrapper extends SingleStackStorage { } } + /** + * Special cases because vanilla checks the current stack in the following functions (which it shouldn't): + * + */ @Override public int getCapacity(ItemVariant variant) { + // Special case to limit buckets to 1 in furnace fuel inputs. + if (storage.inventory instanceof AbstractFurnaceBlockEntity && slot == 1 && variant.isOf(Items.BUCKET)) { + return 1; + } + + // Special case to limit brewing stand "bottle inputs" to 1. + if (storage.inventory instanceof BrewingStandBlockEntity && slot < 3) { + return 1; + } + return Math.min(storage.inventory.getMaxCountPerStack(), variant.getItem().getMaxCount()); } 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 8bc61a31c..bebf12633 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 @@ -18,9 +18,11 @@ package net.fabricmc.fabric.test.transfer.gametests; 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.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.test.GameTest; @@ -137,4 +139,50 @@ public class VanillaStorageTests { context.complete(); } + + /** + * {@link Inventory#isValid(int, ItemStack)} is supposed to be independent of the stack size. + * However, to limit some stackable inputs to a size of 1, brewing stands and furnaces don't follow this rule in all cases. + * This test ensures that the Transfer API works around this issue for furnaces. + */ + @GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE) + public void testBadFurnaceIsValid(TestContext context) { + BlockPos pos = new BlockPos(0, 1, 0); + context.setBlockState(pos, Blocks.FURNACE.getDefaultState()); + FurnaceBlockEntity furnace = (FurnaceBlockEntity) context.getBlockEntity(pos); + InventoryStorage furnaceWrapper = InventoryStorage.of(furnace, null); + + try (Transaction tx = Transaction.openOuter()) { + if (furnaceWrapper.getSlot(1).insert(ItemVariant.of(Items.BUCKET), 2, tx) != 1) { + throw new GameTestException("Exactly 1 bucket should have been inserted"); + } + } + + context.complete(); + } + + /** + * Same as {@link #testBadFurnaceIsValid(TestContext)}, but for brewing stands. + */ + @GameTest(templateName = FabricGameTest.EMPTY_STRUCTURE) + public void testBadBrewingStandIsValid(TestContext context) { + BlockPos pos = new BlockPos(0, 1, 0); + context.setBlockState(pos, Blocks.BREWING_STAND.getDefaultState()); + BrewingStandBlockEntity brewingStand = (BrewingStandBlockEntity) context.getBlockEntity(pos); + InventoryStorage brewingStandWrapper = InventoryStorage.of(brewingStand, null); + + try (Transaction tx = Transaction.openOuter()) { + for (int bottleSlot = 0; bottleSlot < 3; ++bottleSlot) { + if (brewingStandWrapper.getSlot(bottleSlot).insert(ItemVariant.of(Items.GLASS_BOTTLE), 2, tx) != 1) { + throw new GameTestException("Exactly 1 glass bottle should have been inserted"); + } + } + + if (brewingStandWrapper.getSlot(3).insert(ItemVariant.of(Items.REDSTONE), 2, tx) != 2) { + throw new GameTestException("Brewing ingredient insertion should not be limited"); + } + } + + context.complete(); + } }