Fix default fluid names for non-placeable fluids, clarify Storage iterator lifecycle with modification (#3141)

* Fix default fluid names for non-placeable fluids, clarify Storage iterator lifecycle with modification

* Fix blank variant translation accidentally changing

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

@@ -21,11 +21,15 @@ import java.util.Optional;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.Nullable;
 
+import net.minecraft.block.Block;
+import net.minecraft.block.Blocks;
 import net.minecraft.fluid.FlowableFluid;
 import net.minecraft.fluid.Fluid;
 import net.minecraft.item.BucketItem;
+import net.minecraft.registry.Registries;
 import net.minecraft.sound.SoundEvent;
 import net.minecraft.text.Text;
+import net.minecraft.util.Util;
 import net.minecraft.world.World;
 
 /**
@@ -41,7 +45,14 @@ public interface FluidVariantAttributeHandler {
 	 * Return the name that should be used for the passed fluid variant.
 	 */
 	default Text getName(FluidVariant fluidVariant) {
-		return fluidVariant.getFluid().getDefaultState().getBlockState().getBlock().getName();
+		Block fluidBlock = fluidVariant.getFluid().getDefaultState().getBlockState().getBlock();
+
+		if (!fluidVariant.isBlank() && fluidBlock == Blocks.AIR) {
+			// Some non-placeable fluids use air as their fluid block, in that case infer translation key from the fluid id.
+			return Text.translatable(Util.createTranslationKey("block", Registries.FLUID.getId(fluidVariant.getFluid())));
+		} else {
+			return fluidBlock.getName();
+		}
 	}
 
 	/**

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

@@ -146,6 +146,9 @@ public interface Storage<T> extends Iterable<StorageView<T>> {
 	 * but inventories with a dynamic or very large amount of slots should not do that to ensure timely termination of
 	 * the iteration.
 	 *
+	 * <p>If a modification is made to the storage during iteration, the iterator might become invalid at the end of the outermost transaction.
+	 * In particular, if multiple storage views are extracted from, the entire iteration should be wrapped in a transaction.
+	 *
 	 * @return An iterator over the contents of this storage. Calling remove on the iterator is not allowed.
 	 */
 	@Override

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

@@ -18,11 +18,13 @@ package net.fabricmc.fabric.test.transfer.ingame;
 
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemUsageContext;
+import net.minecraft.text.Text;
 import net.minecraft.util.ActionResult;
 
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants;
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidStorage;
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariantAttributes;
 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.transaction.Transaction;
@@ -47,6 +49,12 @@ public class ExtractStickItem extends Item {
 			boolean requireExact = context.getPlayer() != null && context.getPlayer().isSneaking();
 
 			if (!requireExact || extracted == FluidConstants.BUCKET) {
+				if (context.getPlayer() != null) {
+					context.getPlayer().sendMessage(
+							Text.literal("Extracted some ").append(FluidVariantAttributes.getName(stored)).append("."),
+							true);
+				}
+
 				transaction.commit();
 				return ActionResult.success(context.getWorld().isClient());
 			}