Two Transfer API fixes ()

* Fix : Double chest wrapper not always updating both halves

* Fix : Make creative ContainerItemContext give unique items to the play
This commit is contained in:
Technici4n 2023-01-05 13:49:51 +01:00 committed by GitHub
parent 3fc4752e4f
commit ccd377ba6b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 318 additions and 7 deletions
fabric-transfer-api-v1/src
main/java/net/fabricmc/fabric
testmod
java/net/fabricmc/fabric/test/transfer
resources/data/fabric-transfer-api-v1-testmod/gametest/structures

View file

@ -24,6 +24,7 @@ import org.jetbrains.annotations.Nullable;
import net.minecraft.entity.player.PlayerEntity;
import net.minecraft.entity.player.PlayerInventory;
import net.minecraft.item.ItemStack;
import net.minecraft.item.ItemUsage;
import net.minecraft.screen.ScreenHandler;
import net.minecraft.util.Hand;
@ -35,6 +36,8 @@ import net.fabricmc.fabric.api.transfer.v1.storage.StoragePreconditions;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
import net.fabricmc.fabric.impl.transfer.context.ConstantContainerItemContext;
import net.fabricmc.fabric.impl.transfer.context.CreativeInteractionContainerItemContext;
import net.fabricmc.fabric.impl.transfer.context.InitialContentsContainerItemContext;
import net.fabricmc.fabric.impl.transfer.context.PlayerContainerItemContext;
import net.fabricmc.fabric.impl.transfer.context.SingleSlotContainerItemContext;
@ -90,17 +93,29 @@ public interface ContainerItemContext {
/**
* Returns a context for interaction with a player's hand. This is recommended for item use interactions.
*
* <p>In creative mode, {@link #withInitial(ItemStack)} is used to avoid modifying the item in hand.
* <p>In creative mode, {@link #forCreativeInteraction} is used with the hand stack.
* Otherwise, {@link #ofPlayerHand} is used.
* This matches the behavior of {@link ItemUsage#exchangeStack}.
*/
static ContainerItemContext forPlayerInteraction(PlayerEntity player, Hand hand) {
if (player.getAbilities().creativeMode) {
return withInitial(player.getStackInHand(hand));
return forCreativeInteraction(player, player.getStackInHand(hand));
} else {
return ofPlayerHand(player, hand);
}
}
/**
* Returns a context for creative interaction.
*
* <p>The stack will never be modified, and any updated stack will only be added to the player's inventory
* if the player's inventory doesn't already contain it.
* This matches the creative behavior of {@link ItemUsage#exchangeStack}.
*/
static ContainerItemContext forCreativeInteraction(PlayerEntity player, ItemStack interactingStack) {
return new CreativeInteractionContainerItemContext(ItemVariant.of(interactingStack), interactingStack.getCount(), player);
}
/**
* Return a context for the passed player's hand.
*/
@ -132,12 +147,36 @@ public interface ContainerItemContext {
}
/**
* Return a context that can accept anything, and will accept (and destroy) any overflow items, with some initial content.
* Return a context that always has some content, and will accept (and destroy) any overflow items.
* This can typically be used to check if a stack provides an API, or simulate operations on the returned API,
* for example to simulate how much fluid could be extracted from the stack.
*
* <p>Note that the stack can never be mutated by this function: its contents are copied directly.
*/
static ContainerItemContext withConstant(ItemStack constantContent) {
return withConstant(ItemVariant.of(constantContent), constantContent.getCount());
}
/**
* Return a context that always has some content, and will accept (and destroy) any overflow items.
* This can typically be used to check if a stack provides an API, or simulate operations on the returned API,
* for example to simulate how much fluid could be extracted from the variant and amount.
*/
static ContainerItemContext withConstant(ItemVariant constantVariant, long constantAmount) {
StoragePreconditions.notNegative(constantAmount);
return new ConstantContainerItemContext(constantVariant, constantAmount);
}
/**
* Return a context that can accept anything, and will accept (and destroy) any overflow items, with some initial content.
* This can typically be used to check if a stack provides an API, or simulate operations on the returned API,
* for example to simulate how much fluid could be extracted from the stack.
*
* <p>Note that the stack can never be mutated by this function: its contents are copied directly.
*
* @deprecated Use {@link #withConstant(ItemStack)} instead.
*/
@Deprecated(forRemoval = true)
static ContainerItemContext withInitial(ItemStack initialContent) {
return withInitial(ItemVariant.of(initialContent), initialContent.getCount());
}
@ -146,7 +185,10 @@ public interface ContainerItemContext {
* Return a context that can accept anything, and will accept (and destroy) any overflow items, with some initial variant and amount.
* This can typically be used to check if a variant provides an API, or simulate operations on the returned API,
* for example to simulate how much fluid could be extracted from the variant and amount.
*
* @deprecated Use {@link #withConstant(ItemVariant, long)} instead.
*/
@Deprecated(forRemoval = true)
static ContainerItemContext withInitial(ItemVariant initialVariant, long initialAmount) {
StoragePreconditions.notNegative(initialAmount);
return new InitialContentsContainerItemContext(initialVariant, initialAmount);

View file

@ -48,7 +48,8 @@ public final class FluidStorageUtil {
* Then, it tries to fill that item from the storage. If that fails, it tries to fill the storage from that item.
*
* <p>Only up to one fluid variant will be moved, and the corresponding emptying/filling sound will be played.
* In creative mode, the player's inventory will not be modified.
* In creative mode, the original container item is not modified,
* and the player's inventory will additionally receive a copy of the modified container, if it doesn't have it yet.
*
* @param storage The storage that the player is interacting with.
* @param player The player.

View file

@ -0,0 +1,79 @@
/*
* 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.impl.transfer.context;
import java.util.Collections;
import java.util.List;
import net.fabricmc.fabric.api.transfer.v1.context.ContainerItemContext;
import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.storage.StoragePreconditions;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
public class ConstantContainerItemContext implements ContainerItemContext {
private final SingleVariantStorage<ItemVariant> backingSlot = new SingleVariantStorage<>() {
@Override
protected ItemVariant getBlankVariant() {
return ItemVariant.blank();
}
@Override
protected long getCapacity(ItemVariant variant) {
return Long.MAX_VALUE;
}
@Override
public long insert(ItemVariant insertedVariant, long maxAmount, TransactionContext transaction) {
StoragePreconditions.notBlankNotNegative(insertedVariant, maxAmount);
// Pretend we can't insert anything to route every insertion through insertOverflow.
return 0;
}
@Override
public long extract(ItemVariant extractedVariant, long maxAmount, TransactionContext transaction) {
StoragePreconditions.notBlankNotNegative(extractedVariant, maxAmount);
// Pretend we can extract anything, but never actually do it.
return maxAmount;
}
};
public ConstantContainerItemContext(ItemVariant initialVariant, long initialAmount) {
backingSlot.variant = initialVariant;
backingSlot.amount = initialAmount;
}
@Override
public SingleSlotStorage<ItemVariant> getMainSlot() {
return backingSlot;
}
@Override
public long insertOverflow(ItemVariant itemVariant, long maxAmount, TransactionContext transactionContext) {
StoragePreconditions.notBlankNotNegative(itemVariant, maxAmount);
// Always allow anything to be inserted.
return maxAmount;
}
@Override
public List<SingleSlotStorage<ItemVariant>> getAdditionalSlots() {
return Collections.emptyList();
}
}

View file

@ -0,0 +1,59 @@
/*
* 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.impl.transfer.context;
import net.minecraft.entity.player.PlayerEntity;
import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.item.PlayerInventoryStorage;
import net.fabricmc.fabric.api.transfer.v1.storage.StoragePreconditions;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
public class CreativeInteractionContainerItemContext extends ConstantContainerItemContext {
private final PlayerInventoryStorage playerInventory;
public CreativeInteractionContainerItemContext(ItemVariant initialVariant, long initialAmount, PlayerEntity player) {
super(initialVariant, initialAmount);
this.playerInventory = PlayerInventoryStorage.of(player);
}
@Override
public long insertOverflow(ItemVariant itemVariant, long maxAmount, TransactionContext transactionContext) {
StoragePreconditions.notBlankNotNegative(itemVariant, maxAmount);
if (maxAmount > 0) {
// Only add the item to the player inventory if it's not already in the inventory.
boolean hasItem = false;
for (SingleSlotStorage<ItemVariant> slot : playerInventory.getSlots()) {
if (slot.getResource().equals(itemVariant) && slot.getAmount() > 0) {
hasItem = true;
break;
}
}
if (!hasItem) {
playerInventory.offer(itemVariant, 1, transactionContext);
}
}
// Insertion always succeeds from the POV of the context user.
return maxAmount;
}
}

View file

@ -26,6 +26,7 @@ import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage;
import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
@Deprecated(forRemoval = true)
public class InitialContentsContainerItemContext implements ContainerItemContext {
private final SingleVariantStorage<ItemVariant> backingSlot = new SingleVariantStorage<>() {
@Override

View file

@ -16,11 +16,15 @@
package net.fabricmc.fabric.impl.transfer.item;
import net.minecraft.block.ChestBlock;
import net.minecraft.block.entity.AbstractFurnaceBlockEntity;
import net.minecraft.block.entity.BrewingStandBlockEntity;
import net.minecraft.block.entity.ChestBlockEntity;
import net.minecraft.block.entity.ShulkerBoxBlockEntity;
import net.minecraft.block.enums.ChestType;
import net.minecraft.item.ItemStack;
import net.minecraft.item.Items;
import net.minecraft.util.math.BlockPos;
import net.fabricmc.fabric.api.transfer.v1.item.base.SingleStackStorage;
import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
@ -121,6 +125,15 @@ class InventorySlotWrapper extends SingleStackStorage {
public void updateSnapshots(TransactionContext transaction) {
storage.markDirtyParticipant.updateSnapshots(transaction);
super.updateSnapshots(transaction);
// For chests: also schedule a markDirty call for the other half
if (storage.inventory instanceof ChestBlockEntity chest && chest.getCachedState().get(ChestBlock.CHEST_TYPE) != ChestType.SINGLE) {
BlockPos otherChestPos = chest.getPos().offset(ChestBlock.getFacing(chest.getCachedState()));
if (chest.getWorld().getBlockEntity(otherChestPos) instanceof ChestBlockEntity otherChest) {
((InventoryStorageImpl) InventoryStorageImpl.of(otherChest, null)).markDirtyParticipant.updateSnapshots(transaction);
}
}
}
@Override

View file

@ -16,6 +16,8 @@
package net.fabricmc.fabric.test.transfer.gametests;
import org.apache.commons.lang3.mutable.MutableInt;
import net.minecraft.block.Block;
import net.minecraft.block.BlockState;
import net.minecraft.block.Blocks;
@ -37,7 +39,9 @@ import net.minecraft.world.World;
import net.fabricmc.fabric.api.gametest.v1.FabricGameTest;
import net.fabricmc.fabric.api.transfer.v1.item.InventoryStorage;
import net.fabricmc.fabric.api.transfer.v1.item.ItemStorage;
import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
import net.fabricmc.fabric.test.transfer.mixin.AbstractFurnaceBlockEntityAccessor;
@ -269,4 +273,39 @@ public class VanillaStorageTests {
context.complete();
}
/**
* Regression test for <a href="https://github.com/FabricMC/fabric/issues/2810">double chest wrapper only updating modified halves</a>.
*/
@GameTest(templateName = "fabric-transfer-api-v1-testmod:double_chest_comparators")
public void testDoubleChestComparator(TestContext context) {
BlockPos chestPos = new BlockPos(2, 2, 2);
Storage<ItemVariant> storage = ItemStorage.SIDED.find(context.getWorld(), context.getAbsolutePos(chestPos), Direction.UP);
context.assertTrue(storage != null, "Storage must not be null");
// Insert one item
try (Transaction tx = Transaction.openOuter()) {
context.assertTrue(storage.insert(ItemVariant.of(Items.DIAMOND), 1, tx) == 1, "Diamond should have been inserted");
tx.commit();
}
// Check that an update is queued for every single comparator
MutableInt comparatorCount = new MutableInt();
context.forEachRelativePos(relativePos -> {
if (context.getBlockState(relativePos).getBlock() != Blocks.COMPARATOR) {
return;
}
comparatorCount.increment();
if (!context.getWorld().getBlockTickScheduler().isQueued(context.getAbsolutePos(relativePos), Blocks.COMPARATOR)) {
throw new GameTestException("Comparator at " + relativePos + " should have an update scheduled");
}
});
context.assertTrue(comparatorCount.intValue() == 6, "Expected exactly 6 comparators");
context.complete();
}
}

View file

@ -50,7 +50,7 @@ class FluidItemTests {
testSimpleContentsQuery();
// Ensure this doesn't throw an error due to the empty stack.
assertEquals(null, ContainerItemContext.withInitial(ItemStack.EMPTY).find(FluidStorage.ITEM));
assertEquals(null, ContainerItemContext.withConstant(ItemStack.EMPTY).find(FluidStorage.ITEM));
}
private static void testFluidItemApi() {
@ -174,7 +174,7 @@ class FluidItemTests {
assertEquals(
new ResourceAmount<>(FluidVariant.of(Fluids.WATER), BUCKET),
StorageUtil.findExtractableContent(
ContainerItemContext.withInitial(new ItemStack(Items.WATER_BUCKET)).find(FluidStorage.ITEM),
ContainerItemContext.withConstant(new ItemStack(Items.WATER_BUCKET)).find(FluidStorage.ITEM),
null
)
);
@ -182,7 +182,7 @@ class FluidItemTests {
assertEquals(
null,
StorageUtil.findExtractableContent(
ContainerItemContext.withInitial(new ItemStack(Items.WATER_BUCKET)).find(FluidStorage.ITEM),
ContainerItemContext.withConstant(new ItemStack(Items.WATER_BUCKET)).find(FluidStorage.ITEM),
FluidVariant::hasNbt, // Only allow NBT -> won't match anything.
null
)

View file

@ -0,0 +1,77 @@
{
DataVersion: 3218,
size: [5, 2, 6],
data: [
{pos: [0, 0, 0], state: "minecraft:air"},
{pos: [0, 0, 1], state: "minecraft:air"},
{pos: [0, 0, 2], state: "minecraft:stone"},
{pos: [0, 0, 3], state: "minecraft:stone"},
{pos: [0, 0, 4], state: "minecraft:air"},
{pos: [0, 0, 5], state: "minecraft:air"},
{pos: [1, 0, 0], state: "minecraft:air"},
{pos: [1, 0, 1], state: "minecraft:air"},
{pos: [1, 0, 2], state: "minecraft:air"},
{pos: [1, 0, 3], state: "minecraft:air"},
{pos: [1, 0, 4], state: "minecraft:air"},
{pos: [1, 0, 5], state: "minecraft:air"},
{pos: [2, 0, 0], state: "minecraft:stone"},
{pos: [2, 0, 1], state: "minecraft:air"},
{pos: [2, 0, 2], state: "minecraft:air"},
{pos: [2, 0, 3], state: "minecraft:air"},
{pos: [2, 0, 4], state: "minecraft:air"},
{pos: [2, 0, 5], state: "minecraft:stone"},
{pos: [3, 0, 0], state: "minecraft:air"},
{pos: [3, 0, 1], state: "minecraft:air"},
{pos: [3, 0, 2], state: "minecraft:air"},
{pos: [3, 0, 3], state: "minecraft:air"},
{pos: [3, 0, 4], state: "minecraft:air"},
{pos: [3, 0, 5], state: "minecraft:air"},
{pos: [4, 0, 0], state: "minecraft:air"},
{pos: [4, 0, 1], state: "minecraft:air"},
{pos: [4, 0, 2], state: "minecraft:stone"},
{pos: [4, 0, 3], state: "minecraft:stone"},
{pos: [4, 0, 4], state: "minecraft:air"},
{pos: [4, 0, 5], state: "minecraft:air"},
{pos: [0, 1, 0], state: "minecraft:air"},
{pos: [0, 1, 1], state: "minecraft:air"},
{pos: [0, 1, 2], state: "minecraft:comparator{facing:east,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [0, 1, 3], state: "minecraft:comparator{facing:east,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [0, 1, 4], state: "minecraft:air"},
{pos: [0, 1, 5], state: "minecraft:air"},
{pos: [1, 1, 0], state: "minecraft:air"},
{pos: [1, 1, 1], state: "minecraft:air"},
{pos: [1, 1, 2], state: "minecraft:stone"},
{pos: [1, 1, 3], state: "minecraft:stone"},
{pos: [1, 1, 4], state: "minecraft:air"},
{pos: [1, 1, 5], state: "minecraft:air"},
{pos: [2, 1, 0], state: "minecraft:comparator{facing:south,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [2, 1, 1], state: "minecraft:stone"},
{pos: [2, 1, 2], state: "minecraft:chest{facing:east,type:left,waterlogged:false}", nbt: {Items: [], id: "minecraft:chest"}},
{pos: [2, 1, 3], state: "minecraft:chest{facing:east,type:right,waterlogged:false}", nbt: {Items: [], id: "minecraft:chest"}},
{pos: [2, 1, 4], state: "minecraft:stone"},
{pos: [2, 1, 5], state: "minecraft:comparator{facing:north,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [3, 1, 0], state: "minecraft:air"},
{pos: [3, 1, 1], state: "minecraft:air"},
{pos: [3, 1, 2], state: "minecraft:stone"},
{pos: [3, 1, 3], state: "minecraft:stone"},
{pos: [3, 1, 4], state: "minecraft:air"},
{pos: [3, 1, 5], state: "minecraft:air"},
{pos: [4, 1, 0], state: "minecraft:air"},
{pos: [4, 1, 1], state: "minecraft:air"},
{pos: [4, 1, 2], state: "minecraft:comparator{facing:west,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [4, 1, 3], state: "minecraft:comparator{facing:west,mode:compare,powered:false}", nbt: {OutputSignal: 0, id: "minecraft:comparator"}},
{pos: [4, 1, 4], state: "minecraft:air"},
{pos: [4, 1, 5], state: "minecraft:air"}
],
entities: [],
palette: [
"minecraft:stone",
"minecraft:air",
"minecraft:comparator{facing:east,mode:compare,powered:false}",
"minecraft:comparator{facing:south,mode:compare,powered:false}",
"minecraft:chest{facing:east,type:left,waterlogged:false}",
"minecraft:chest{facing:east,type:right,waterlogged:false}",
"minecraft:comparator{facing:north,mode:compare,powered:false}",
"minecraft:comparator{facing:west,mode:compare,powered:false}"
]
}