Fix Universal bucket handling for Fluids with NBT (Fix #3757)

src/main/java/net/minecraftforge/fluids/FluidUtil.java

@@ -29,7 +29,9 @@ import net.minecraft.block.material.Material;
 import net.minecraft.block.state.IBlockState;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.item.ItemBucket;
+import net.minecraft.init.Items;
 import net.minecraft.item.ItemStack;
+import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.tileentity.TileEntity;
 import net.minecraft.util.EnumFacing;
 import net.minecraft.util.EnumHand;
@@ -37,6 +39,7 @@ import net.minecraft.util.SoundCategory;
 import net.minecraft.util.SoundEvent;
 import net.minecraft.util.math.BlockPos;
 import net.minecraft.world.World;
+import net.minecraftforge.common.ForgeModContainer;
 import net.minecraftforge.fluids.capability.CapabilityFluidHandler;
 import net.minecraftforge.fluids.capability.IFluidHandler;
 import net.minecraftforge.fluids.capability.IFluidHandlerItem;
@@ -46,7 +49,6 @@ import net.minecraftforge.fluids.capability.wrappers.FluidBlockWrapper;
 import net.minecraftforge.items.CapabilityItemHandler;
 import net.minecraftforge.items.IItemHandler;
 import net.minecraftforge.items.ItemHandlerHelper;
-import net.minecraftforge.items.wrapper.InvWrapper;
 
 public class FluidUtil
 {
@@ -100,41 +102,6 @@ public class FluidUtil
         return false;
     }
 
-    /**
-     * Used to handle the common case of a player holding a fluid item and right-clicking on a fluid handler.
-     * First it tries to fill the container item from the fluid handler,
-     * if that action fails then it tries to drain the container item into the fluid handler.
-     *
-     * @param stack        The filled or empty fluid container.
-     *                     Will not be modified directly, if modifications are necessary a modified copy is returned in the result.
-     * @param fluidHandler The fluid handler to interact with.
-     * @param player       The player doing the interaction between the item and fluid handler.
-     * @return a {@link FluidActionResult} holding the result and resulting container.
-     *
-     * @deprecated Use {@link #interactWithFluidHandler(EntityPlayer, EnumHand, World, BlockPos, EnumFacing)} which is easier to use.
-     */
-    @Deprecated
-    @Nonnull
-    public static FluidActionResult interactWithFluidHandler(@Nonnull ItemStack stack, IFluidHandler fluidHandler, EntityPlayer player)
-    {
-        if (stack.isEmpty() || fluidHandler == null || player == null)
-        {
-            return FluidActionResult.FAILURE;
-        }
-
-        IItemHandler playerInventory = new InvWrapper(player.inventory);
-
-        FluidActionResult fillResult = tryFillContainerAndStow(stack, fluidHandler, playerInventory, Integer.MAX_VALUE, player);
-        if (fillResult.isSuccess())
-        {
-            return fillResult;
-        }
-        else
-        {
-            return tryEmptyContainerAndStow(stack, fluidHandler, playerInventory, Integer.MAX_VALUE, player);
-        }
-    }
-
     /**
      * Fill a container from the given fluidSource.
      *
@@ -681,4 +648,47 @@ public class FluidUtil
             }
         }
     }
+
+    /**
+     * @param fluidStack contents used to fill the bucket.
+     *                   FluidStack is used instead of Fluid to preserve fluid NBT, the amount is ignored.
+     * @return a filled vanilla bucket or filled universal bucket.
+     *         Returns empty itemStack if none of the enabled buckets can hold the fluid.
+     */
+    @Nonnull
+    public static ItemStack getFilledBucket(@Nonnull FluidStack fluidStack)
+    {
+        Fluid fluid = fluidStack.getFluid();
+
+        if (fluidStack.tag == null || fluidStack.tag.hasNoTags())
+        {
+            if (fluid == FluidRegistry.WATER)
+            {
+                return new ItemStack(Items.WATER_BUCKET);
+            }
+            else if (fluid == FluidRegistry.LAVA)
+            {
+                return new ItemStack(Items.LAVA_BUCKET);
+            }
+            else if (fluid.getName().equals("milk"))
+            {
+                return new ItemStack(Items.MILK_BUCKET);
+            }
+        }
+
+        if (FluidRegistry.isUniversalBucketEnabled() && FluidRegistry.getBucketFluids().contains(fluid))
+        {
+            UniversalBucket bucket = ForgeModContainer.getInstance().universalBucket;
+            ItemStack filledBucket = new ItemStack(bucket);
+            FluidStack fluidContents = new FluidStack(fluidStack, bucket.getCapacity());
+
+            NBTTagCompound tag = new NBTTagCompound();
+            fluidContents.writeToNBT(tag);
+            filledBucket.setTagCompound(tag);
+
+            return filledBucket;
+        }
+
+        return ItemStack.EMPTY;
+    }
 }

src/main/java/net/minecraftforge/fluids/UniversalBucket.java

@@ -37,7 +37,6 @@ import net.minecraft.util.text.translation.I18n;
 import net.minecraft.world.World;
 import net.minecraftforge.common.capabilities.ICapabilityProvider;
 import net.minecraftforge.event.entity.player.FillBucketEvent;
-import net.minecraftforge.fluids.capability.IFluidHandler;
 import net.minecraftforge.fluids.capability.IFluidHandlerItem;
 import net.minecraftforge.fluids.capability.wrappers.FluidBucketWrapper;
 import net.minecraftforge.fml.common.eventhandler.Event;
@@ -254,29 +253,14 @@ public class UniversalBucket extends Item
         }
     }
 
+    /**
+     * @deprecated use the NBT-sensitive version {@link FluidUtil#getFilledBucket(FluidStack)}
+     */
+    @Deprecated
     @Nonnull
     public static ItemStack getFilledBucket(@Nonnull UniversalBucket item, Fluid fluid)
     {
-        ItemStack bucket = new ItemStack(item);
-
-        if (FluidRegistry.getBucketFluids().contains(fluid))
-        {
-            // fill the container
-            NBTTagCompound tag = new NBTTagCompound();
-            FluidStack fluidStack = new FluidStack(fluid, item.getCapacity());
-            fluidStack.writeToNBT(tag);
-            bucket.setTagCompound(tag);
-        }
-        else if (fluid == FluidRegistry.WATER)
-        {
-            return new ItemStack(Items.WATER_BUCKET);
-        }
-        else if (fluid == FluidRegistry.LAVA)
-        {
-            return new ItemStack(Items.LAVA_BUCKET);
-        }
-
-        return bucket;
+        return FluidUtil.getFilledBucket(new FluidStack(fluid, Fluid.BUCKET_VOLUME));
     }
 
     @Nullable

src/main/java/net/minecraftforge/fluids/capability/wrappers/FluidBucketWrapper.java

@@ -33,12 +33,11 @@ import net.minecraftforge.common.capabilities.ICapabilityProvider;
 import net.minecraftforge.fluids.Fluid;
 import net.minecraftforge.fluids.FluidRegistry;
 import net.minecraftforge.fluids.FluidStack;
+import net.minecraftforge.fluids.FluidUtil;
 import net.minecraftforge.fluids.capability.FluidTankProperties;
 import net.minecraftforge.fluids.capability.IFluidHandlerItem;
 import net.minecraftforge.fluids.capability.IFluidTankProperties;
-import net.minecraftforge.fluids.UniversalBucket;
 import net.minecraftforge.fluids.capability.CapabilityFluidHandler;
-import net.minecraftforge.fluids.capability.IFluidHandler;
 
 /**
  * Wrapper for vanilla and forge buckets.
@@ -96,27 +95,20 @@ public class FluidBucketWrapper implements IFluidHandlerItem, ICapabilityProvide
         }
     }
 
+    /**
+     * @deprecated use the NBT-sensitive version {@link #setFluid(FluidStack)}
+     */
+    @Deprecated
     protected void setFluid(@Nullable Fluid fluid) {
-        if (fluid == null)
-        {
+        setFluid(new FluidStack(fluid, Fluid.BUCKET_VOLUME));
+    }
+
+    protected void setFluid(@Nullable FluidStack fluidStack)
+    {
+        if (fluidStack == null)
             container = new ItemStack(Items.BUCKET);
-        }
-        else if (fluid == FluidRegistry.WATER)
-        {
-            container = new ItemStack(Items.WATER_BUCKET);
-        }
-        else if (fluid == FluidRegistry.LAVA)
-        {
-            container = new ItemStack(Items.LAVA_BUCKET);
-        }
-        else if (fluid.getName().equals("milk"))
-        {
-            container = new ItemStack(Items.MILK_BUCKET);
-        }
-        else if (FluidRegistry.isUniversalBucketEnabled() && FluidRegistry.getBucketFluids().contains(fluid))
-        {
-            container = UniversalBucket.getFilledBucket(ForgeModContainer.getInstance().universalBucket, fluid);
-        }
+        else
+            container = FluidUtil.getFilledBucket(fluidStack);
     }
 
     @Override
@@ -135,7 +127,7 @@ public class FluidBucketWrapper implements IFluidHandlerItem, ICapabilityProvide
 
         if (doFill)
         {
-            setFluid(resource.getFluid());
+            setFluid(resource);
         }
 
         return Fluid.BUCKET_VOLUME;
@@ -155,7 +147,7 @@ public class FluidBucketWrapper implements IFluidHandlerItem, ICapabilityProvide
         {
             if (doDrain)
             {
-                setFluid(null);
+                setFluid((FluidStack) null);
             }
             return fluidStack;
         }
@@ -177,7 +169,7 @@ public class FluidBucketWrapper implements IFluidHandlerItem, ICapabilityProvide
         {
             if (doDrain)
             {
-                setFluid(null);
+                setFluid((FluidStack) null);
             }
             return fluidStack;
         }