Transfer API: continuous fluid-containing items and other base implementations. (#1665)

* Transfer API: continuous fluid-containing items and other base implementations

* Update player inventory storage TODO

* Add PlayerInventoryStorage test

* getHandSlot() and small fixes

* Use simulateExtract in findExtractableContent

* Apply review

* Post-rebase fixes

* Add tentative InventoryProvider support

Co-authored-by: Player <player@player.to>

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

@@ -71,4 +71,11 @@ public interface InventoryStorage extends Storage<ItemVariant> {
 	 * Each wrapper corresponds to a single slot in the inventory.
 	 */
 	List<SingleSlotStorage<ItemVariant>> getSlots();
+
+	/**
+	 * Retrieve a wrapper around a specific slot of the inventory.
+	 */
+	default SingleSlotStorage<ItemVariant> getSlot(int slot) {
+		return getSlots().get(slot);
+	}
 }

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

@@ -22,6 +22,7 @@ import org.jetbrains.annotations.ApiStatus;
 
 import net.minecraft.block.Blocks;
 import net.minecraft.block.ChestBlock;
+import net.minecraft.block.InventoryProvider;
 import net.minecraft.block.entity.ChestBlockEntity;
 import net.minecraft.inventory.Inventory;
 import net.minecraft.inventory.SidedInventory;
@@ -56,6 +57,8 @@ public final class ItemStorage {
 	 *
 	 * <p>Block entities directly implementing {@link Inventory} or {@link SidedInventory} are automatically handled by a fallback provider,
 	 * and don't need to do anything.
+	 * Blocks that implement {@link InventoryProvider} and whose returned inventory is constant (it's the same for two subsequent calls)
+	 * are also handled automatically and don't need to do anything.
 	 * The fallback provider assumes that the {@link Inventory} "owns" its contents. If that's not the case,
 	 * for example because it redirects all function calls to another inventory, then implementing {@link Inventory} should be avoided.
 	 *
@@ -84,6 +87,16 @@ public final class ItemStorage {
 		ItemStorage.SIDED.registerFallback((world, pos, state, blockEntity, direction) -> {
 			Inventory inventoryToWrap = null;
 
+			if (state.getBlock() instanceof InventoryProvider provider) {
+				SidedInventory first = provider.getInventory(state, world, pos);
+				SidedInventory second = provider.getInventory(state, world, pos);
+
+				// Hopefully we can trust the sided inventory not to change.
+				if (first == second && first != null) {
+					return InventoryStorage.of(first, direction);
+				}
+			}
+
 			if (blockEntity instanceof Inventory inventory) {
 				if (blockEntity instanceof ChestBlockEntity && state.getBlock() instanceof ChestBlock chestBlock) {
 					inventoryToWrap = ChestBlock.getInventory(chestBlock, state, world, pos, true);

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

@@ -21,6 +21,7 @@ import org.jetbrains.annotations.ApiStatus;
 import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.entity.player.PlayerInventory;
 import net.minecraft.screen.ScreenHandler;
+import net.minecraft.util.Hand;
 
 import net.fabricmc.fabric.api.transfer.v1.storage.base.CombinedStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
@@ -43,6 +44,8 @@ import net.fabricmc.fabric.impl.transfer.item.CursorSlotWrapper;
 @ApiStatus.Experimental
 @Deprecated
 @ApiStatus.NonExtendable
+// TODO: Consider explicitly syncing stacks by sending a ScreenHandlerSlotUpdateS2CPacket if that proves to be necessary.
+// TODO: Vanilla doesn't seem to be doing it reliably, so we ignore it for now.
 public interface PlayerInventoryStorage extends InventoryStorage {
 	/**
 	 * Return an instance for the passed player's inventory.
@@ -67,7 +70,7 @@ public interface PlayerInventoryStorage extends InventoryStorage {
 	}
 
 	/**
-	 * Add items to the inventory if possible, and drop any leftover items in the world, similar to {@link PlayerInventory#offerOrDrop}
+	 * Add items to the inventory if possible, and drop any leftover items in the world, similar to {@link PlayerInventory#offerOrDrop}.
 	 *
 	 * <p>Note: This function has full transaction support, and will not actually drop the items until the outermost transaction is committed.
 	 *
@@ -75,5 +78,42 @@ public interface PlayerInventoryStorage extends InventoryStorage {
 	 * @param amount How many of the variant to insert.
 	 * @param transaction The transaction this operation is part of.
 	 */
-	void offerOrDrop(ItemVariant variant, long amount, TransactionContext transaction);
+	default void offerOrDrop(ItemVariant variant, long amount, TransactionContext transaction) {
+		long offered = offer(variant, amount, transaction);
+		drop(variant, amount - offered, transaction);
+	}
+
+	/**
+	 * Try to add items to the inventory if possible, stacking like {@link PlayerInventory#offer}.
+	 * Unlike {@link #offerOrDrop}, this function will not drop excess items.
+	 *
+	 * <p>The exact behavior is:
+	 * <ol>
+	 *     <li>Try to stack inserted items with existing items in the main hand, then the offhand.</li>
+	 *     <li>Try to stack remaining inserted items with existing items in the player main inventory.</li>
+	 *     <li>Try to insert the remainder into empty slots of the player main inventory.</li>
+	 * </ol>
+	 *
+	 * @param variant The variant to insert.
+	 * @param maxAmount How many of the variant to insert, at most.
+	 * @param transaction The transaction this operation is part of.
+	 * @return How many items could be inserted.
+	 */
+	long offer(ItemVariant variant, long maxAmount, TransactionContext transaction);
+
+	/**
+	 * Drop items in the world at the player's location.
+	 *
+	 * <p>Note: This function has full transaction support, and will not actually drop the items until the outermost transaction is committed.
+	 *
+	 * @param variant The variant to drop.
+	 * @param amount How many of the variant to drop.
+	 * @param transaction The transaction this operation is part of.
+	 */
+	void drop(ItemVariant variant, long amount, TransactionContext transaction);
+
+	/**
+	 * Return a wrapper around the current slot of the passed hand.
+	 */
+	SingleSlotStorage<ItemVariant> getHandSlot(Hand hand);
 }

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java

@@ -96,6 +96,17 @@ public interface Storage<T> {
 	 */
 	long insert(T resource, long maxAmount, TransactionContext transaction);
 
+	/**
+	 * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see #insert
+	 */
+	default long simulateInsert(T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return insert(resource, maxAmount, simulateTransaction);
+		}
+	}
+
 	/**
 	 * Return false if calling {@link #extract} will absolutely always return 0, or true otherwise or in doubt.
 	 *
@@ -116,6 +127,17 @@ public interface Storage<T> {
 	 */
 	long extract(T resource, long maxAmount, TransactionContext transaction);
 
+	/**
+	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see #extract
+	 */
+	default long simulateExtract(T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return extract(resource, maxAmount, simulateTransaction);
+		}
+	}
+
 	/**
 	 * Iterate through the contents of this storage, for the scope of the passed transaction.
 	 * Every visited {@link StorageView} represents a stored resource and an amount.

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/StorageUtil.java

@@ -188,12 +188,10 @@ public final class StorageUtil {
 		T extractableResource = findExtractableResource(storage, transaction);
 
 		if (extractableResource != null) {
-			try (Transaction nested = Transaction.openNested(transaction)) {
-				long extractableAmount = storage.extract(extractableResource, Long.MAX_VALUE, nested);
+			long extractableAmount = storage.simulateExtract(extractableResource, Long.MAX_VALUE, transaction);
 
-				if (extractableAmount > 0) {
-					return new ResourceAmount<>(extractableResource, extractableAmount);
-				}
+			if (extractableAmount > 0) {
+				return new ResourceAmount<>(extractableResource, extractableAmount);
 			}
 		}
 

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/base/FilteringStorage.java

@@ -0,0 +1,166 @@
+/*
+ * 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.api.transfer.v1.storage.base;
+
+import java.util.Iterator;
+import java.util.function.Supplier;
+
+import com.google.common.collect.Iterators;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.Nullable;
+
+import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
+import net.fabricmc.fabric.api.transfer.v1.storage.StorageView;
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+
+/**
+ * A base {@link Storage} implementation that delegates every call to another storage,
+ * except that it only allows insertion or extraction if {@link #canInsert} or {@link #canExtract} allows it respectively.
+ * This can for example be used to wrap the internal storage of some device behind additional insertion or extraction checks.
+ * If one of these two functions is overridden to always return false, implementors may also wish to override
+ * {@link #supportsInsertion} and/or {@link #supportsExtraction}.
+ *
+ * @param <T> The type of the stored resources.
+ *
+ * @deprecated Experimental feature, we reserve the right to remove or change it without further notice.
+ * The transfer API is a complex addition, and we want to be able to correct possible design mistakes.
+ */
+@ApiStatus.Experimental
+@Deprecated
+public abstract class FilteringStorage<T> implements Storage<T> {
+	protected final Supplier<Storage<T>> backingStorage;
+
+	/**
+	 * Create a new filtering storage, with a fixed backing storage.
+	 */
+	public FilteringStorage(Storage<T> backingStorage) {
+		this(() -> backingStorage);
+	}
+
+	/**
+	 * Create a new filtering storage, with a supplier for the backing storage.
+	 * This allows the backing storage to change without having to create a new filtering storage.
+	 * If that is unnecessary, the other overload can be used for convenience.
+	 */
+	public FilteringStorage(Supplier<Storage<T>> backingStorage) {
+		this.backingStorage = backingStorage;
+	}
+
+	/**
+	 * Return true if insertion of the passed resource should be forwarded to the backing storage, or false if it should fail.
+	 */
+	protected boolean canInsert(T resource) {
+		return true;
+	}
+
+	/**
+	 * Return true if extraction of the passed resource should be forwarded to the backing storage, or false if it should fail.
+	 */
+	protected boolean canExtract(T resource) {
+		return true;
+	}
+
+	@Override
+	public boolean supportsInsertion() {
+		return backingStorage.get().supportsInsertion();
+	}
+
+	@Override
+	public long insert(T resource, long maxAmount, TransactionContext transaction) {
+		if (canInsert(resource)) {
+			return backingStorage.get().insert(resource, maxAmount, transaction);
+		} else {
+			return 0;
+		}
+	}
+
+	@Override
+	public boolean supportsExtraction() {
+		return backingStorage.get().supportsExtraction();
+	}
+
+	@Override
+	public long extract(T resource, long maxAmount, TransactionContext transaction) {
+		if (canExtract(resource)) {
+			return backingStorage.get().extract(resource, maxAmount, transaction);
+		} else {
+			return 0;
+		}
+	}
+
+	@Override
+	public Iterator<StorageView<T>> iterator(TransactionContext transaction) {
+		return Iterators.transform(backingStorage.get().iterator(transaction), FilteringStorageView::new);
+	}
+
+	@Override
+	@Nullable
+	public StorageView<T> exactView(TransactionContext transaction, T resource) {
+		StorageView<T> exact = backingStorage.get().exactView(transaction, resource);
+
+		if (exact != null) {
+			return new FilteringStorageView(exact);
+		} else {
+			return null;
+		}
+	}
+
+	@Override
+	public long getVersion() {
+		return backingStorage.get().getVersion();
+	}
+
+	/**
+	 * This is used to ensure extractions through storage views of the backing stored also get checked by {@link #canExtract}.
+	 */
+	private class FilteringStorageView implements StorageView<T> {
+		private final StorageView<T> backingView;
+
+		private FilteringStorageView(StorageView<T> backingView) {
+			this.backingView = backingView;
+		}
+
+		@Override
+		public long extract(T resource, long maxAmount, TransactionContext transaction) {
+			if (canExtract(resource)) {
+				return backingView.extract(resource, maxAmount, transaction);
+			} else {
+				return 0;
+			}
+		}
+
+		@Override
+		public boolean isResourceBlank() {
+			return backingView.isResourceBlank();
+		}
+
+		@Override
+		public T getResource() {
+			return backingView.getResource();
+		}
+
+		@Override
+		public long getAmount() {
+			return backingView.getAmount();
+		}
+
+		@Override
+		public long getCapacity() {
+			return backingView.getCapacity();
+		}
+	}
+}

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/base/SingleVariantItemStorage.java

@@ -0,0 +1,226 @@
+/*
+ * 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.api.transfer.v1.storage.base;
+
+import org.jetbrains.annotations.ApiStatus;
+
+import net.minecraft.item.Item;
+import net.minecraft.item.ItemStack;
+
+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.TransferVariant;
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+
+/**
+ * Base implementation of a fixed-capacity "continuous" storage for item-provided storage APIs.
+ * The item may not change, so the data has to be stored in the NBT of the stacks.
+ * This can be used for example to implement portable fluid tanks, fluid-containing jetpacks, and so on...
+ * Continuous here means that they can store any integer amount between 0 and the capacity, unlike buckets or bottles.
+ *
+ * <p>To expose the storage API for an item, you need to register a provider for your item, and pass it an instance of this class:
+ * <ul>
+ *    <li>You must override {@link #getBlankResource()}, for example {@code return FluidVariant.blank();} for fluids.</li>
+ *    <li>You must override {@link #getResource(ItemVariant)} and {@link #getAmount(ItemVariant)}.
+ *    Generally you will read the resource and the amount from the NBT of the item variant.</li>
+ *    <li>You must override {@link #getCapacity(TransferVariant)} to set the capacity of your storage.</li>
+ *    <li>You must override {@link #getUpdatedVariant}. It is used to change the resource and the amount of the item variant.
+ *    Generally you will copy the NBT, modify it, and then create a new variant from that.
+ *    Copying the NBT instead of recreating it from scratch is important to keep custom names or enchantments.</li>
+ *    <li>You may also override {@link #canInsert} and {@link #canExtract} if you want to restrict insertion and/or extraction.</li>
+ * </ul>
+ *
+ * @param <T> The type of the stored transfer variant.
+ *
+ * @deprecated Experimental feature, we reserve the right to remove or change it without further notice.
+ * The transfer API is a complex addition, and we want to be able to correct possible design mistakes.
+ */
+@ApiStatus.Experimental
+@Deprecated
+public abstract class SingleVariantItemStorage<T extends TransferVariant<?>> implements SingleSlotStorage<T> {
+	/**
+	 * Reference to the context.
+	 */
+	private final ContainerItemContext context;
+	/**
+	 * Starting item. The storage is not valid for other items.
+	 */
+	private final Item item;
+
+	public SingleVariantItemStorage(ContainerItemContext context) {
+		this.context = context;
+		this.item = context.getItemVariant().getItem();
+	}
+
+	/**
+	 * Return the blank resource.
+	 */
+	protected abstract T getBlankResource();
+
+	/**
+	 * Return the current resource by reading the NBT of the passed variant.
+	 */
+	protected abstract T getResource(ItemVariant currentVariant);
+
+	/**
+	 * Return the current amount by reading the NBT of the passed variant.
+	 */
+	protected abstract long getAmount(ItemVariant currentVariant);
+
+	/**
+	 * Return the capacity of this storage for the passed resource.
+	 * An estimate should be returned if the passed resource is blank.
+	 */
+	protected abstract long getCapacity(T variant);
+
+	/**
+	 * Return an updated variant with new resource and amount.
+	 * Implementors should generally convert the passed {@code currentVariant} to a stack,
+	 * then edit the NBT of the stack so it contains the correct resource and amount.
+	 *
+	 * <p>When the new amount is 0, it is recommended that the subtags corresponding to the resource and amount
+	 * be removed, for example using {@link ItemStack#removeSubTag}, so that newly-crafted containers can stack with
+	 * emptied containers.
+	 *
+	 * @param currentVariant Variant to which the modification should be applied.
+	 * @param newResource Resource that should be contained in the returned variant.
+	 * @param newAmount Amount that should be contained in the returned variant.
+	 * @return A modified variant containing the new resource and amount.
+	 */
+	protected abstract ItemVariant getUpdatedVariant(ItemVariant currentVariant, T newResource, long newAmount);
+
+	/**
+	 * Return {@code true} if the passed non-blank variant can be inserted, {@code false} otherwise.
+	 */
+	protected boolean canInsert(T resource) {
+		return true;
+	}
+
+	/**
+	 * Return {@code true} if the passed non-blank variant can be extracted, {@code false} otherwise.
+	 */
+	protected boolean canExtract(T resource) {
+		return true;
+	}
+
+	private boolean tryUpdateStorage(T newResource, long newAmount, TransactionContext tx) {
+		return context.exchange(getUpdatedVariant(context.getItemVariant(), newResource, newAmount), 1, tx) == 1;
+	}
+
+	@Override
+	public boolean supportsInsertion() {
+		return context.getItemVariant().isOf(item);
+	}
+
+	@Override
+	public long insert(T insertedResource, long maxAmount, TransactionContext transaction) {
+		StoragePreconditions.notBlankNotNegative(insertedResource, maxAmount);
+
+		// Check insertion.
+		if (!canInsert(insertedResource)) return 0;
+		// Check item.
+		if (!context.getItemVariant().isOf(item)) return 0;
+
+		long amount = getAmount(context.getItemVariant());
+		T resource = getResource(context.getItemVariant());
+
+		long inserted = 0;
+
+		if (resource.isBlank() || amount == 0) {
+			// Insertion into empty storage.
+			inserted = Math.min(getCapacity(insertedResource), maxAmount);
+		} else if (resource.equals(insertedResource)) {
+			// Insertion into storage with an existing resource.
+			inserted = Math.min(getCapacity(insertedResource) - amount, maxAmount);
+		}
+
+		if (inserted > 0) {
+			if (tryUpdateStorage(insertedResource, amount + inserted, transaction)) {
+				return inserted;
+			}
+		}
+
+		return 0;
+	}
+
+	@Override
+	public boolean supportsExtraction() {
+		return context.getItemVariant().isOf(item);
+	}
+
+	@Override
+	public long extract(T extractedResource, long maxAmount, TransactionContext transaction) {
+		StoragePreconditions.notBlankNotNegative(extractedResource, maxAmount);
+
+		// Check extraction.
+		if (!canExtract(extractedResource)) return 0;
+
+		// Check item.
+		if (!context.getItemVariant().isOf(item)) return 0;
+
+		long amount = getAmount(context.getItemVariant());
+		T resource = getResource(context.getItemVariant());
+
+		long extracted = 0;
+
+		if (resource.equals(extractedResource)) {
+			// Make sure the resource matches
+			extracted = Math.min(maxAmount, amount);
+		}
+
+		if (extracted > 0) {
+			if (tryUpdateStorage(resource, maxAmount - extracted, transaction)) {
+				return extracted;
+			}
+		}
+
+		return 0;
+	}
+
+	@Override
+	public boolean isResourceBlank() {
+		return getResource().isBlank();
+	}
+
+	@Override
+	public T getResource() {
+		if (context.getItemVariant().isOf(item)) {
+			return getResource(context.getItemVariant());
+		} else {
+			return getBlankResource();
+		}
+	}
+
+	@Override
+	public long getAmount() {
+		if (context.getItemVariant().isOf(item)) {
+			return getAmount(context.getItemVariant());
+		} else {
+			return 0;
+		}
+	}
+
+	@Override
+	public long getCapacity() {
+		if (context.getItemVariant().isOf(item)) {
+			return getCapacity(getResource());
+		} else {
+			return 0;
+		}
+	}
+}

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/context/PlayerContainerItemContext.java

@@ -17,10 +17,8 @@
 package net.fabricmc.fabric.impl.transfer.context;
 
 import java.util.List;
-import java.util.Objects;
 
 import net.minecraft.entity.player.PlayerEntity;
-import net.minecraft.entity.player.PlayerInventory;
 import net.minecraft.util.Hand;
 
 import net.fabricmc.fabric.api.transfer.v1.context.ContainerItemContext;
@@ -34,11 +32,8 @@ public class PlayerContainerItemContext implements ContainerItemContext {
 	private final SingleSlotStorage<ItemVariant> slot;
 
 	public PlayerContainerItemContext(PlayerEntity player, Hand hand) {
-		Objects.requireNonNull(hand, "Hand may not be null.");
-
 		this.playerWrapper = PlayerInventoryStorage.of(player);
-		int slotIndex = hand == Hand.MAIN_HAND ? player.getInventory().selectedSlot : PlayerInventory.OFF_HAND_SLOT;
-		this.slot = playerWrapper.getSlots().get(slotIndex);
+		this.slot = playerWrapper.getHandSlot(hand);
 	}
 
 	public PlayerContainerItemContext(PlayerEntity player, SingleSlotStorage<ItemVariant> slot) {

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

@@ -18,9 +18,10 @@ package net.fabricmc.fabric.impl.transfer.item;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
-import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.entity.player.PlayerInventory;
+import net.minecraft.util.Hand;
 
 import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
 import net.fabricmc.fabric.api.transfer.v1.item.PlayerInventoryStorage;
@@ -31,32 +32,30 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
 
 class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerInventoryStorage {
 	private final DroppedStacks droppedStacks;
-	private final PlayerEntity player;
+	private final PlayerInventory playerInventory;
 
 	PlayerInventoryStorageImpl(PlayerInventory playerInventory) {
 		super(playerInventory);
 		this.droppedStacks = new DroppedStacks();
-		this.player = playerInventory.player;
+		this.playerInventory = playerInventory;
 	}
 
 	@Override
-	public void offerOrDrop(ItemVariant resource, long amount, TransactionContext tx) {
+	public long offer(ItemVariant resource, long amount, TransactionContext tx) {
 		StoragePreconditions.notBlankNotNegative(resource, amount);
+		long initialAmount = amount;
 
 		List<SingleSlotStorage<ItemVariant>> mainSlots = getSlots().subList(0, PlayerInventory.MAIN_SIZE);
 
-		// Stack into the main stack first
-		SingleSlotStorage<ItemVariant> selectedSlot = getSlots().get(player.getInventory().selectedSlot);
+		// Stack into the main stack first and the offhand stack second.
+		for (Hand hand : Hand.values()) {
+			SingleSlotStorage<ItemVariant> handSlot = getHandSlot(hand);
 
-		if (selectedSlot.getResource().equals(resource)) {
-			amount -= selectedSlot.insert(resource, amount, tx);
-		}
+			if (handSlot.getResource().equals(resource)) {
+				amount -= handSlot.insert(resource, amount, tx);
 
-		// Stack into the offhand stack otherwise
-		SingleSlotStorage<ItemVariant> offHandSlot = getSlots().get(PlayerInventory.OFF_HAND_SLOT);
-
-		if (offHandSlot.getResource().equals(resource)) {
-			amount -= offHandSlot.insert(resource, amount, tx);
+				if (amount == 0) return initialAmount;
+			}
 		}
 
 		// Otherwise insert into the main slots, first iteration tries to stack, second iteration inserts into empty slots.
@@ -67,16 +66,40 @@ class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerI
 				if (!slot.isResourceBlank() || allowEmptySlots) {
 					amount -= slot.insert(resource, amount, tx);
 				}
+
+				if (amount == 0) return initialAmount;
 			}
 		}
 
-		// Drop leftover in the world on the server side (will be synced by the game with the client).
+		return initialAmount - amount;
+	}
+
+	@Override
+	public void drop(ItemVariant resource, long amount, TransactionContext tx) {
+		StoragePreconditions.notBlankNotNegative(resource, amount);
+
+		// Drop in the world on the server side (will be synced by the game with the client).
 		// Dropping items is server-side only because it involves randomness.
-		if (amount > 0 && player.world.isClient()) {
+		if (amount > 0 && !playerInventory.player.world.isClient()) {
 			droppedStacks.addDrop(resource, amount, tx);
 		}
 	}
 
+	@Override
+	public SingleSlotStorage<ItemVariant> getHandSlot(Hand hand) {
+		if (Objects.requireNonNull(hand) == Hand.MAIN_HAND) {
+			if (PlayerInventory.isValidHotbarIndex(playerInventory.selectedSlot)) {
+				return getSlot(playerInventory.selectedSlot);
+			} else {
+				throw new RuntimeException("Unexpected player selected slot: " + playerInventory.selectedSlot);
+			}
+		} else if (hand == Hand.OFF_HAND) {
+			return getSlot(PlayerInventory.OFF_HAND_SLOT);
+		} else {
+			throw new UnsupportedOperationException("Unknown hand: " + hand);
+		}
+	}
+
 	private class DroppedStacks extends SnapshotParticipant<Integer> {
 		final List<ItemVariant> droppedKeys = new ArrayList<>();
 		final List<Long> droppedCounts = new ArrayList<>();
@@ -111,7 +134,7 @@ class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerI
 
 				while (droppedCounts.get(i) > 0) {
 					int dropped = (int) Math.min(key.getItem().getMaxCount(), droppedCounts.get(i));
-					player.dropStack(key.toStack(dropped));
+					playerInventory.player.dropStack(key.toStack(dropped));
 					droppedCounts.set(i, droppedCounts.get(i) - dropped);
 				}
 			}

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

@@ -68,7 +68,7 @@ public class DropperBlockMixin {
 			}
 
 			StorageUtil.move(
-					InventoryStorage.of(dispenser, null).getSlots().get(slot),
+					InventoryStorage.of(dispenser, null).getSlot(slot),
 					target,
 					k -> true,
 					1,

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.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.test.transfer.unittests;
+
+import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BUCKET;
+import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals;
+
+import net.minecraft.fluid.Fluids;
+
+import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
+import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil;
+import net.fabricmc.fabric.api.transfer.v1.storage.base.FilteringStorage;
+import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage;
+import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
+
+public class BaseStorageTests {
+	public static void run() {
+		testFilteringStorage();
+	}
+
+	private static void testFilteringStorage() {
+		SingleVariantStorage<FluidVariant> storage = new SingleVariantStorage<>() {
+			@Override
+			protected FluidVariant getBlankVariant() {
+				return FluidVariant.blank();
+			}
+
+			@Override
+			protected long getCapacity(FluidVariant variant) {
+				return BUCKET * 10;
+			}
+		};
+		Storage<FluidVariant> noWater = new FilteringStorage<>(storage) {
+			@Override
+			protected boolean canExtract(FluidVariant resource) {
+				return !resource.isOf(Fluids.WATER);
+			}
+
+			@Override
+			protected boolean canInsert(FluidVariant resource) {
+				return !resource.isOf(Fluids.WATER);
+			}
+		};
+		FluidVariant water = FluidVariant.of(Fluids.WATER);
+		FluidVariant lava = FluidVariant.of(Fluids.LAVA);
+
+		// Insertion into the backing storage should succeed.
+		try (Transaction tx = Transaction.openOuter()) {
+			assertEquals(BUCKET, storage.insert(water, BUCKET, tx));
+			tx.commit();
+		}
+
+		// Insertion through the filter should fail.
+		assertEquals(0L, noWater.simulateInsert(water, BUCKET, null));
+		// Extraction should also fail.
+		assertEquals(0L, noWater.simulateExtract(water, BUCKET, null));
+		// The fluid should be visible.
+		assertEquals(water, StorageUtil.findStoredResource(noWater, null));
+		// But it can't be extracted, even through a storage view.
+		assertEquals(null, StorageUtil.findExtractableResource(noWater, null));
+		assertEquals(null, StorageUtil.findExtractableContent(noWater, null));
+
+		storage.amount = 0;
+		storage.variant = FluidVariant.blank();
+
+		// Lava insertion and extract should proceed just fine.
+		try (Transaction tx = Transaction.openOuter()) {
+			assertEquals(BUCKET, noWater.insert(lava, BUCKET, tx));
+			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
+			// Test that simulating doesn't change the state...
+			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
+			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
+			tx.commit();
+		}
+
+		assertEquals(BUCKET, storage.simulateExtract(lava, BUCKET, null));
+	}
+}

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

@@ -18,9 +18,9 @@ package net.fabricmc.fabric.test.transfer.unittests;
 
 import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BOTTLE;
 import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BUCKET;
+import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals;
 
 import java.util.List;
-import java.util.Objects;
 
 import net.minecraft.fluid.Fluids;
 import net.minecraft.inventory.Inventory;
@@ -176,10 +176,4 @@ class FluidItemTests {
 				)
 		);
 	}
-
-	private static void assertEquals(Object expected, Object actual) {
-		if (!Objects.equals(expected, actual)) {
-			throw new AssertionError(String.format("assertEquals failed%nexpected: %s%n but was: %s", expected, actual));
-		}
-	}
 }

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

@@ -77,6 +77,8 @@ class FluidTests {
 		try (Transaction tx = Transaction.openOuter()) {
 			// Should not allow lava (canInsert returns false)
 			if (waterStorage.insert(LAVA, BUCKET, tx) != 0) throw new AssertionError("Lava inserted");
+			// Should allow insert, but without mutating the storage.
+			if (waterStorage.simulateInsert(WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Simulated insert failed");
 			// Should allow insert
 			if (waterStorage.insert(TAGGED_WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 1 failed");
 			// Variants are different, should not allow insert
@@ -87,6 +89,8 @@ class FluidTests {
 			if (waterStorage.insert(TAGGED_WATER, BUCKET, tx) != 0) throw new AssertionError("Storage full, yet something was inserted");
 			// Should allow extraction
 			if (waterStorage.extract(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Extraction failed");
+			// Simulated extraction should succeed but do nothing
+			if (waterStorage.simulateExtract(TAGGED_WATER, Long.MAX_VALUE, tx) != BUCKET) throw new AssertionError("Simulated extraction failed");
 			// Re-insert
 			if (waterStorage.insert(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 3 failed");
 			// Test contents

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

@@ -0,0 +1,92 @@
+/*
+ * 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.test.transfer.unittests;
+
+import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals;
+
+import net.minecraft.entity.player.PlayerInventory;
+import net.minecraft.item.ItemStack;
+import net.minecraft.item.Items;
+
+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.transaction.Transaction;
+
+public class PlayerInventoryStorageTests {
+	public static void run() {
+		testStacking();
+	}
+
+	private static void testStacking() {
+		// A bit hacky... but nothing should try using the player inventory as long as we don't call drop.
+		PlayerInventory inv = new PlayerInventory(null);
+		PlayerInventoryStorage wrapper = PlayerInventoryStorage.of(inv);
+
+		// Fill everything with stone besides the first two inventory slots.
+		inv.selectedSlot = 3;
+		inv.main.set(3, new ItemStack(Items.STONE, 63));
+		inv.offHand.set(0, new ItemStack(Items.STONE, 62));
+
+		for (int i = 4; i < PlayerInventory.MAIN_SIZE; ++i) {
+			inv.main.set(i, new ItemStack(Items.STONE, 61));
+		}
+
+		ItemVariant stone = ItemVariant.of(Items.STONE);
+
+		try (Transaction tx = Transaction.openOuter()) {
+			assertEquals(1L, wrapper.offer(stone, 1, tx));
+
+			// Should have went into the main stack
+			assertEquals(64, inv.main.get(3).getCount());
+		}
+
+		try (Transaction tx = Transaction.openOuter()) {
+			assertEquals(2L, wrapper.offer(stone, 2, tx));
+
+			// Should have went into the main and offhand stacks.
+			assertEquals(64, inv.main.get(3).getCount());
+			assertEquals(63, inv.offHand.get(0).getCount());
+		}
+
+		long toInsertStacking = 1 + 2 + (PlayerInventory.MAIN_SIZE - 4) * 3;
+
+		// Should be just enough to fill existing stacks, but not touch slots 0, 1 and 2.
+		try (Transaction tx = Transaction.openOuter()) {
+			assertEquals(toInsertStacking, wrapper.offer(stone, toInsertStacking, tx));
+
+			assertEquals(64, inv.main.get(3).getCount());
+			assertEquals(64, inv.offHand.get(0).getCount());
+
+			for (int i = 4; i < PlayerInventory.MAIN_SIZE; ++i) {
+				assertEquals(64, inv.main.get(i).getCount());
+			}
+
+			for (int i = 0; i < 3; ++i) {
+				assertEquals(true, inv.main.get(i).isEmpty());
+			}
+
+			// Now insertion should fill the remaining stacks
+			assertEquals(150L, wrapper.offer(stone, 150, tx));
+			assertEquals(64, inv.main.get(0).getCount());
+			assertEquals(64, inv.main.get(1).getCount());
+			assertEquals(22, inv.main.get(2).getCount());
+
+			// Only 64 - 22 = 42 room left!
+			assertEquals(42L, wrapper.offer(stone, Long.MAX_VALUE, tx));
+		}
+	}
+}

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

@@ -0,0 +1,179 @@
+/*
+ * 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.test.transfer.unittests;
+
+import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BUCKET;
+import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals;
+
+import java.util.List;
+
+import net.minecraft.fluid.Fluids;
+import net.minecraft.inventory.Inventory;
+import net.minecraft.inventory.SimpleInventory;
+import net.minecraft.item.ItemStack;
+import net.minecraft.item.Items;
+import net.minecraft.nbt.NbtCompound;
+import net.minecraft.text.LiteralText;
+import net.minecraft.text.Text;
+
+import net.fabricmc.fabric.api.transfer.v1.context.ContainerItemContext;
+import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.item.InventoryStorage;
+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.storage.base.SingleSlotStorage;
+import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantItemStorage;
+import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+
+public class SingleVariantItemStorageTests {
+	private static final FluidVariant LAVA = FluidVariant.of(Fluids.LAVA);
+
+	public static void run() {
+		testWaterTank();
+	}
+
+	private static void testWaterTank() {
+		SimpleInventory inv = new SimpleInventory(new ItemStack(Items.DIAMOND, 2), ItemStack.EMPTY);
+		ContainerItemContext ctx = new InventoryContainerItemContext(inv);
+
+		Storage<FluidVariant> storage = createTankStorage(ctx);
+
+		try (Transaction tx = Transaction.openOuter()) {
+			// Insertion should succeed and transfer an item into the second slot.
+			assertEquals(BUCKET, storage.insert(LAVA, BUCKET, tx));
+			// Insertion should create a new stack.
+			assertEquals(1, inv.getStack(0).getCount());
+			assertEquals(null, inv.getStack(0).getNbt());
+			assertEquals(1, inv.getStack(1).getCount());
+			assertEquals(LAVA, getFluid(inv.getStack(1)));
+			assertEquals(BUCKET, getAmount(inv.getStack(1)));
+
+			// Second insertion should just insert in place as the count is now 1.
+			assertEquals(BUCKET, storage.insert(LAVA, BUCKET, tx));
+
+			for (int slot = 0; slot < 2; ++slot) {
+				assertEquals(LAVA, getFluid(inv.getStack(slot)));
+				assertEquals(BUCKET, getAmount(inv.getStack(slot)));
+			}
+
+			tx.commit();
+		}
+
+		// Make sure custom NBT is kept.
+		Text customName = new LiteralText("Lava-containing diamond!");
+		inv.getStack(0).setCustomName(customName);
+
+		try (Transaction tx = Transaction.openOuter()) {
+			// Test extract along the way.
+			assertEquals(BUCKET, storage.extract(LAVA, BUCKET, tx));
+
+			tx.commit();
+		}
+
+		// Check custom name.
+		assertEquals(customName, inv.getStack(0).getName());
+		assertEquals(FluidVariant.blank(), getFluid(inv.getStack(0)));
+		assertEquals(0L, getAmount(inv.getStack(0)));
+	}
+
+	private static FluidVariant getFluid(ItemStack stack) {
+		NbtCompound nbt = stack.getNbt();
+
+		if (nbt != null && nbt.contains("fluid")) {
+			return FluidVariant.fromNbt(nbt.getCompound("fluid"));
+		} else {
+			return FluidVariant.blank();
+		}
+	}
+
+	private static long getAmount(ItemStack stack) {
+		NbtCompound nbt = stack.getNbt();
+
+		if (nbt != null) {
+			return nbt.getLong("amount");
+		} else {
+			return 0;
+		}
+	}
+
+	private static void setContents(ItemStack stack, FluidVariant newResource, long newAmount) {
+		if (newAmount > 0) {
+			stack.getOrCreateNbt().put("fluid", newResource.toNbt());
+			stack.getOrCreateNbt().putLong("amount", newAmount);
+		} else {
+			// Make sure emptied tanks can stack with tanks without NBT.
+			stack.removeSubNbt("fluid");
+			stack.removeSubNbt("amount");
+		}
+	}
+
+	private static Storage<FluidVariant> createTankStorage(ContainerItemContext ctx) {
+		return new SingleVariantItemStorage<>(ctx) {
+			@Override
+			protected FluidVariant getBlankResource() {
+				return FluidVariant.blank();
+			}
+
+			@Override
+			protected FluidVariant getResource(ItemVariant currentVariant) {
+				return getFluid(currentVariant.toStack());
+			}
+
+			@Override
+			protected long getAmount(ItemVariant currentVariant) {
+				return SingleVariantItemStorageTests.getAmount(currentVariant.toStack());
+			}
+
+			@Override
+			protected long getCapacity(FluidVariant variant) {
+				return 2 * BUCKET;
+			}
+
+			@Override
+			protected ItemVariant getUpdatedVariant(ItemVariant currentVariant, FluidVariant newResource, long newAmount) {
+				// Operate on the stack directly to keep any other NBT data such as a custom name or enchant.
+				ItemStack stack = currentVariant.toStack();
+				setContents(stack, newResource, newAmount);
+				return ItemVariant.of(stack);
+			}
+		};
+	}
+
+	private static class InventoryContainerItemContext implements ContainerItemContext {
+		private final InventoryStorage storage;
+
+		private InventoryContainerItemContext(Inventory inventory) {
+			this.storage = InventoryStorage.of(inventory, null);
+		}
+
+		@Override
+		public SingleSlotStorage<ItemVariant> getMainSlot() {
+			return storage.getSlot(0);
+		}
+
+		@Override
+		public long insertOverflow(ItemVariant itemVariant, long maxAmount, TransactionContext transactionContext) {
+			return storage.insert(itemVariant, maxAmount, transactionContext);
+		}
+
+		@Override
+		public List<SingleSlotStorage<ItemVariant>> getAdditionalSlots() {
+			return storage.getSlots();
+		}
+	}
+}

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

@@ -0,0 +1,27 @@
+/*
+ * 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.test.transfer.unittests;
+
+import java.util.Objects;
+
+public class TestUtil {
+	public static <T> void assertEquals(T expected, T actual) {
+		if (!Objects.equals(expected, actual)) {
+			throw new AssertionError(String.format("assertEquals failed%nexpected: %s%n but was: %s", expected, actual));
+		}
+	}
+}

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

@@ -23,10 +23,14 @@ import net.fabricmc.api.ModInitializer;
 public class UnitTestsInitializer implements ModInitializer {
 	@Override
 	public void onInitialize() {
-		TransactionExceptionsTests.run();
+		BaseStorageTests.run();
+		FluidItemTests.run();
 		FluidTests.run();
 		ItemTests.run();
-		FluidItemTests.run();
+		PlayerInventoryStorageTests.run();
+		SingleVariantItemStorageTests.run();
+		TransactionExceptionsTests.run();
+
 		LogManager.getLogger("fabric-transfer-api-v1 testmod").info("Transfer API unit tests successful.");
 	}
 }