Another wave of transfer API improvements (#1801)

* Another wave of transfer API improvements

* Cleaner implementation of FilteringStorage#...Of

* Undo colored name for water and lava variants

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/client/fluid/FluidVariantRenderHandler.java

@@ -60,22 +60,36 @@ public interface FluidVariantRenderHandler {
 	}
 
 	/**
-	 * Return the sprite that should be used to render the passed fluid variant, for use in baked models, (block) entity renderers, or user interfaces.
+	 * Return an array of size at least 2 containing the sprites that should be used to render the passed fluid variant,
+	 * for use in baked models, (block) entity renderers, or user interfaces.
+	 * The first sprite in the array is the still sprite, and the second is the flowing sprite.
 	 *
-	 * <p>Null may be returned if the fluid variant should not be rendered.
+	 * <p>Null may be returned if the fluid variant should not be rendered, but if an array is returned it must have at least two entries and
+	 * they may not be null.
 	 */
 	@Nullable
-	default Sprite getSprite(FluidVariant fluidVariant) {
+	default Sprite[] getSprites(FluidVariant fluidVariant) {
 		// Use the fluid render handler by default.
 		FluidRenderHandler fluidRenderHandler = FluidRenderHandlerRegistry.INSTANCE.get(fluidVariant.getFluid());
 
 		if (fluidRenderHandler != null) {
-			return fluidRenderHandler.getFluidSprites(null, null, fluidVariant.getFluid().getDefaultState())[0];
+			return fluidRenderHandler.getFluidSprites(null, null, fluidVariant.getFluid().getDefaultState());
 		} else {
 			return null;
 		}
 	}
 
+	/**
+	 * @deprecated Use and implement {@linkplain #getSprites(FluidVariant) the other more general overload}.
+	 * This one will be removed in a future iteration of the API.
+	 */
+	@Deprecated(forRemoval = true)
+	@Nullable
+	default Sprite getSprite(FluidVariant fluidVariant) {
+		Sprite[] sprites = getSprites(fluidVariant);
+		return sprites != null ? sprites[0] : null;
+	}
+
 	/**
 	 * @deprecated Use and implement {@linkplain #getColor(FluidVariant, BlockRenderView, BlockPos) the other more general overload}.
 	 * This one will be removed in a future iteration of the API.

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/client/fluid/FluidVariantRendering.java

@@ -18,6 +18,7 @@ package net.fabricmc.fabric.api.transfer.v1.client.fluid;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Nullable;
@@ -116,12 +117,24 @@ public class FluidVariantRendering {
 	}
 
 	/**
-	 * Return the sprite that should be used to render the passed fluid variant, or null if it's not available.
+	 * Return the still and the flowing sprite that should be used to render the passed fluid variant, or null if they are not available.
+	 * The sprites should be rendered using the color returned by {@link #getColor}.
+	 *
+	 * @see FluidVariantRenderHandler#getSprites
+	 */
+	@Nullable
+	public static Sprite[] getSprites(FluidVariant fluidVariant) {
+		return getHandlerOrDefault(fluidVariant.getFluid()).getSprites(fluidVariant);
+	}
+
+	/**
+	 * Return the still sprite that should be used to render the passed fluid variant, or null if it's not available.
 	 * The sprite should be rendered using the color returned by {@link #getColor}.
 	 */
 	@Nullable
 	public static Sprite getSprite(FluidVariant fluidVariant) {
-		return getHandlerOrDefault(fluidVariant.getFluid()).getSprite(fluidVariant);
+		Sprite[] sprites = getSprites(fluidVariant);
+		return sprites != null ? Objects.requireNonNull(sprites[0]) : null;
 	}
 
 	/**

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

@@ -134,7 +134,7 @@ public interface ContainerItemContext {
 	 * for example to simulate how much fluid could be extracted from the variant and amount.
 	 */
 	static ContainerItemContext withInitial(ItemVariant initialVariant, long initialAmount) {
-		StoragePreconditions.notBlankNotNegative(initialVariant, initialAmount);
+		StoragePreconditions.notNegative(initialAmount);
 		return new InitialContentsContainerItemContext(initialVariant, initialAmount);
 	}
 

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/fluid/FluidStorage.java

@@ -16,6 +16,7 @@
 
 package net.fabricmc.fabric.api.transfer.v1.fluid;
 
+import com.google.common.base.Preconditions;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Nullable;
 
@@ -128,6 +129,12 @@ public final class FluidStorage {
 	}
 
 	static {
+		// Ensure that the lookup is only queried on the server side.
+		FluidStorage.SIDED.registerFallback((world, pos, state, blockEntity, context) -> {
+			Preconditions.checkArgument(!world.isClient(), "Sided fluid storage may only be queried for a server world.");
+			return null;
+		});
+
 		// Initialize vanilla cauldron wrappers
 		CauldronFluidContent.getForFluid(Fluids.WATER);
 

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

@@ -18,6 +18,7 @@ package net.fabricmc.fabric.api.transfer.v1.item;
 
 import java.util.List;
 
+import com.google.common.base.Preconditions;
 import org.jetbrains.annotations.ApiStatus;
 
 import net.minecraft.block.Blocks;
@@ -79,6 +80,12 @@ public final class ItemStorage {
 	}
 
 	static {
+		// Ensure that the lookup is only queried on the server side.
+		ItemStorage.SIDED.registerFallback((world, pos, state, blockEntity, context) -> {
+			Preconditions.checkArgument(!world.isClient(), "Sided item storage may only be queried for a server world.");
+			return null;
+		});
+
 		// Composter support.
 		ItemStorage.SIDED.registerForBlocks((world, pos, state, blockEntity, direction) -> ComposterWrapper.get(world, pos, direction), Blocks.COMPOSTER);
 

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

@@ -16,6 +16,7 @@
 
 package net.fabricmc.fabric.api.transfer.v1.storage;
 
+import java.util.List;
 import java.util.function.Predicate;
 
 import org.jetbrains.annotations.ApiStatus;
@@ -26,6 +27,7 @@ import net.minecraft.screen.ScreenHandler;
 import net.minecraft.util.math.MathHelper;
 
 import net.fabricmc.fabric.api.transfer.v1.storage.base.ResourceAmount;
+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;
 
@@ -117,6 +119,32 @@ public final class StorageUtil {
 		return totalMoved;
 	}
 
+	/**
+	 * Try to insert up to some amount of a resource into a list of storage slots, trying to "stack" first,
+	 * i.e. prioritizing slots that already contain the resource.
+	 *
+	 * @return How much was inserted.
+	 * @see Storage#insert
+	 */
+	public static <T> long insertStacking(List<SingleSlotStorage<T>> slots, T resource, long maxAmount, TransactionContext transaction) {
+		StoragePreconditions.notNegative(maxAmount);
+		long amount = 0;
+
+		for (SingleSlotStorage<T> slot : slots) {
+			if (!slot.isResourceBlank()) {
+				amount += slot.insert(resource, maxAmount - amount, transaction);
+				if (amount == maxAmount) return amount;
+			}
+		}
+
+		for (SingleSlotStorage<T> slot : slots) {
+			amount += slot.insert(resource, maxAmount - amount, transaction);
+			if (amount == maxAmount) return amount;
+		}
+
+		return amount;
+	}
+
 	/**
 	 * Attempt to find a resource stored in the passed storage.
 	 *

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

@@ -40,7 +40,10 @@ public interface StorageView<T> {
 	long extract(T resource, long maxAmount, TransactionContext transaction);
 
 	/**
-	 * @return {@code true} if the {@link #getResource} contained in this storage view is blank, or {@code false} otherwise.
+	 * Return {@code true} if the {@link #getResource} contained in this storage view is blank, or {@code false} otherwise.
+	 *
+	 * <p>This function is mostly useful when dealing with storages of arbitrary types.
+	 * For transfer variant storages, this should always be equivalent to {@code getResource().isBlank()}.
 	 */
 	boolean isResourceBlank();
 

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

@@ -34,6 +34,8 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
  * If one of these two functions is overridden to always return false, implementors may also wish to override
  * {@link #supportsInsertion} and/or {@link #supportsExtraction}.
  *
+ * <p>The static functions can be used when insertion or/and extraction should be blocked entirely.
+ *
  * @param <T> The type of the stored resources.
  *
  * <b>Experimental feature</b>, we reserve the right to remove or change it without further notice.
@@ -41,6 +43,63 @@ import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
  */
 @ApiStatus.Experimental
 public abstract class FilteringStorage<T> implements Storage<T> {
+	/**
+	 * Return a wrapper over the passed storage that prevents extraction.
+	 */
+	public static <T> Storage<T> insertOnlyOf(Storage<T> backingStorage) {
+		return of(backingStorage, true, false);
+	}
+
+	/**
+	 * Return a wrapper over the passed storage that prevents insertion.
+	 */
+	public static <T> Storage<T> extractOnlyOf(Storage<T> backingStorage) {
+		return of(backingStorage, false, true);
+	}
+
+	/**
+	 * Return a wrapper over the passed storage that prevents insertion and extraction.
+	 */
+	public static <T> Storage<T> readOnlyOf(Storage<T> backingStorage) {
+		return of(backingStorage, false, false);
+	}
+
+	/**
+	 * Return a wrapper over the passed storage that may prevent insertion or extraction, depending on the boolean parameters.
+	 * For more fine-grained control, a custom subclass of {@link FilteringStorage} should be used.
+	 *
+	 * @param backingStorage Storage to wrap.
+	 * @param allowInsert True to allow insertion, false to block insertion.
+	 * @param allowExtract True to allow extraction, false to block extraction.
+	 */
+	public static <T> Storage<T> of(Storage<T> backingStorage, boolean allowInsert, boolean allowExtract) {
+		if (allowInsert && allowExtract) {
+			return backingStorage;
+		}
+
+		return new FilteringStorage<>(backingStorage) {
+			@Override
+			protected boolean canInsert(T resource) {
+				return allowInsert;
+			}
+
+			@Override
+			protected boolean canExtract(T resource) {
+				return allowExtract;
+			}
+
+			@Override
+			public boolean supportsInsertion() {
+				return allowInsert && super.supportsInsertion();
+			}
+
+			@Override
+			public boolean supportsExtraction() {
+				return allowExtract && super.supportsExtraction();
+			}
+		};
+	}
+
 	protected final Supplier<Storage<T>> backingStorage;
 
 	/**

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

@@ -26,6 +26,7 @@ import net.minecraft.util.Hand;
 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.StorageUtil;
 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;
@@ -63,18 +64,8 @@ class PlayerInventoryStorageImpl extends InventoryStorageImpl implements PlayerI
 			}
 		}
 
-		// Otherwise insert into the main slots, first iteration tries to stack, second iteration inserts into empty slots.
-		for (int iteration = 0; iteration < 2; iteration++) {
-			boolean allowEmptySlots = iteration == 1;
-
-			for (SingleSlotStorage<ItemVariant> slot : mainSlots) {
-				if (!slot.isResourceBlank() || allowEmptySlots) {
-					amount -= slot.insert(resource, amount, tx);
-				}
-
-				if (amount == 0) return initialAmount;
-			}
-		}
+		// Otherwise insert into the main slots, stacking first.
+		amount -= StorageUtil.insertStacking(mainSlots, resource, amount, tx);
 
 		return initialAmount - amount;
 	}

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/ingame/client/FluidVariantRenderTest.java

@@ -16,9 +16,12 @@
 
 package net.fabricmc.fabric.test.transfer.ingame.client;
 
+import java.util.List;
+
 import com.mojang.blaze3d.systems.RenderSystem;
 
 import net.minecraft.client.MinecraftClient;
+import net.minecraft.client.font.TextRenderer;
 import net.minecraft.client.render.BufferBuilder;
 import net.minecraft.client.render.BufferRenderer;
 import net.minecraft.client.render.GameRenderer;
@@ -30,9 +33,8 @@ import net.minecraft.client.texture.SpriteAtlasTexture;
 import net.minecraft.client.util.math.MatrixStack;
 import net.minecraft.entity.player.PlayerEntity;
 import net.minecraft.fluid.Fluids;
-import net.minecraft.util.math.BlockPos;
+import net.minecraft.text.Text;
 import net.minecraft.util.math.Matrix4f;
-import net.minecraft.world.World;
 
 import net.fabricmc.api.ClientModInitializer;
 import net.fabricmc.fabric.api.client.rendering.v1.HudRenderCallback;
@@ -48,17 +50,39 @@ public class FluidVariantRenderTest implements ClientModInitializer {
 		HudRenderCallback.EVENT.register((matrices, tickDelta) -> {
 			PlayerEntity player = MinecraftClient.getInstance().player;
 			if (player == null) return;
-			drawFluidInGui(matrices, FluidVariant.of(Fluids.WATER), player.world, player.getBlockPos(), 0, 0);
+
+			int renderY = 0;
+			List<FluidVariant> variants = List.of(FluidVariant.of(Fluids.WATER), FluidVariant.of(Fluids.LAVA));
+
+			for (FluidVariant variant : variants) {
+				Sprite[] sprites = FluidVariantRendering.getSprites(variant);
+				int color = FluidVariantRendering.getColor(variant, player.world, player.getBlockPos());
+
+				if (sprites != null) {
+					drawFluidInGui(matrices, sprites[0], color, 0, renderY);
+					renderY += 16;
+					drawFluidInGui(matrices, sprites[1], color, 0, renderY);
+					renderY += 16;
+				}
+
+				List<Text> tooltip = FluidVariantRendering.getTooltip(variant);
+				TextRenderer textRenderer = MinecraftClient.getInstance().textRenderer;
+
+				renderY += 2;
+
+				for (Text line : tooltip) {
+					textRenderer.draw(matrices, line, 4, renderY, -1);
+					renderY += 10;
+				}
+			}
 		});
 	}
 
-	private static void drawFluidInGui(MatrixStack ms, FluidVariant fluid, World world, BlockPos pos, int i, int j) {
-		RenderSystem.setShaderTexture(0, SpriteAtlasTexture.BLOCK_ATLAS_TEXTURE);
-		Sprite sprite = FluidVariantRendering.getSprite(fluid);
-		int color = FluidVariantRendering.getColor(fluid, world, pos);
-
+	private static void drawFluidInGui(MatrixStack ms, Sprite sprite, int color, int i, int j) {
 		if (sprite == null) return;
 
+		RenderSystem.setShaderTexture(0, SpriteAtlasTexture.BLOCK_ATLAS_TEXTURE);
+
 		float r = ((color >> 16) & 255) / 255f;
 		float g = ((color >> 8) & 255) / 255f;
 		float b = (color & 255) / 255f;

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

@@ -48,6 +48,9 @@ class FluidItemTests {
 		testFluidItemApi();
 		testWaterPotion();
 		testSimpleContentsQuery();
+
+		// Ensure this doesn't throw an error due to the empty stack.
+		assertEquals(null, ContainerItemContext.withInitial(ItemStack.EMPTY).find(FluidStorage.ITEM));
 	}
 
 	private static void testFluidItemApi() {