Fix #2657: Transfer API edge case with bad isValid overrides (#2659)

Co-authored-by: modmuss50 <modmuss50@gmail.com>
This commit is contained in:
Technici4n 2022-11-20 14:35:10 +01:00 committed by GitHub
parent fa140d5976
commit cd10d4fc03
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 68 additions and 0 deletions

View file

@ -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):
* <ul>
* <li>{@link AbstractFurnaceBlockEntity#isValid(int, ItemStack)}.</li>
* <li>{@link BrewingStandBlockEntity#isValid(int, ItemStack)}.</li>
* </ul>
*/
@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());
}

View file

@ -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();
}
}