From bf1194ec75a264a04b5289b2675db88ed4c4a823 Mon Sep 17 00:00:00 2001
From: Technici4n <13494793+Technici4n@users.noreply.github.com>
Date: Sat, 3 Sep 2022 22:48:07 +0200
Subject: [PATCH] Fix `tryEmptyContainerAndStow` duping fluids with stackable
containers (#9004)
---
.../event/RegisterItemDecorationsEvent.java | 1 +
.../net/minecraftforge/fluids/FluidUtil.java | 10 +-
.../minecraftforge/debug/FluidUtilTest.java | 117 ++++++++++++++++++
src/test/resources/META-INF/mods.toml | 2 +
4 files changed, 127 insertions(+), 3 deletions(-)
create mode 100644 src/test/java/net/minecraftforge/debug/FluidUtilTest.java
diff --git a/src/main/java/net/minecraftforge/client/event/RegisterItemDecorationsEvent.java b/src/main/java/net/minecraftforge/client/event/RegisterItemDecorationsEvent.java
index 3c652191a0..51c9b5a8e2 100644
--- a/src/main/java/net/minecraftforge/client/event/RegisterItemDecorationsEvent.java
+++ b/src/main/java/net/minecraftforge/client/event/RegisterItemDecorationsEvent.java
@@ -2,6 +2,7 @@
* Copyright (c) Forge Development LLC and contributors
* SPDX-License-Identifier: LGPL-2.1-only
*/
+
package net.minecraftforge.client.event;
import net.minecraft.world.item.Item;
diff --git a/src/main/java/net/minecraftforge/fluids/FluidUtil.java b/src/main/java/net/minecraftforge/fluids/FluidUtil.java
index 7784443672..ad593e02b4 100644
--- a/src/main/java/net/minecraftforge/fluids/FluidUtil.java
+++ b/src/main/java/net/minecraftforge/fluids/FluidUtil.java
@@ -173,11 +173,15 @@ public class FluidUtil
ItemStack containerCopy = ItemHandlerHelper.copyStackWithSize(container, 1); // do not modify the input
return getFluidHandler(containerCopy)
.map(containerFluidHandler -> {
-
- // We are acting on a COPY of the stack, so performing changes is acceptable even if we are simulating.
- FluidStack transfer = tryFluidTransfer(fluidDestination, containerFluidHandler, maxAmount, true);
+ FluidStack transfer = tryFluidTransfer(fluidDestination, containerFluidHandler, maxAmount, doDrain);
if (transfer.isEmpty())
return FluidActionResult.FAILURE;
+ if (!doDrain)
+ {
+ // We are acting on a COPY of the stack, so performing changes on the source is acceptable even if we are simulating.
+ // We need to perform the change otherwise the call to getContainer() will be incorrect.
+ containerFluidHandler.drain(transfer, IFluidHandler.FluidAction.EXECUTE);
+ }
if (doDrain && player != null)
{
diff --git a/src/test/java/net/minecraftforge/debug/FluidUtilTest.java b/src/test/java/net/minecraftforge/debug/FluidUtilTest.java
new file mode 100644
index 0000000000..c8fd82870d
--- /dev/null
+++ b/src/test/java/net/minecraftforge/debug/FluidUtilTest.java
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) Forge Development LLC and contributors
+ * SPDX-License-Identifier: LGPL-2.1-only
+ */
+
+package net.minecraftforge.debug;
+
+import net.minecraft.world.item.Item;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.minecraft.world.level.material.Fluid;
+import net.minecraft.world.level.material.Fluids;
+import net.minecraftforge.fluids.FluidStack;
+import net.minecraftforge.fluids.FluidUtil;
+import net.minecraftforge.fluids.capability.templates.FluidTank;
+import net.minecraftforge.fml.common.Mod;
+import net.minecraftforge.fml.event.lifecycle.FMLCommonSetupEvent;
+import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext;
+import net.minecraftforge.items.ItemStackHandler;
+import net.minecraftforge.registries.ForgeRegistries;
+import org.apache.logging.log4j.LogManager;
+
+/**
+ * Various tests for {@link FluidUtil}, that run when the mod is loaded.
+ * If one of the tests fails, an expection will be thrown, and mod loading will fail with an error.
+ * If all tests pass, the mod will load successfully.
+ */
+@Mod(FluidUtilTest.MODID)
+public class FluidUtilTest
+{
+ public static final String MODID = "fluid_util_test";
+
+ public FluidUtilTest()
+ {
+ FMLJavaModLoadingContext.get().getModEventBus().addListener(FluidUtilTest::runTests);
+ }
+
+ private static void runTests(FMLCommonSetupEvent commonSetupEvent)
+ {
+ test_tryEmptyContainer();
+ test_tryEmptyContainerAndStow_stackable();
+
+ LogManager.getLogger().info("FluidUtilTest ok!");
+ }
+
+ /**
+ * Ensures that tryEmptyContainer doesn't change the target fluid handler when simulating.
+ * Regression test for the root cause of issue #6796.
+ */
+ private static void test_tryEmptyContainer()
+ {
+ var sourceStack = new ItemStack(Items.WATER_BUCKET, 2);
+ var targetTank = new FluidTank(10000);
+
+ // Simulate is not supposed to modify anything
+ var simulateResult = FluidUtil.tryEmptyContainer(sourceStack, targetTank, 1000, null, false);
+ if (!simulateResult.isSuccess())
+ throw new AssertionError("Failed to transfer.");
+ checkItemStack(simulateResult.getResult(), Items.BUCKET, 1);
+ // Tank and stack shouldn't be modified
+ checkItemStack(sourceStack, Items.WATER_BUCKET, 2);
+ checkFluidStack(targetTank.getFluid(), Fluids.EMPTY, 0);
+
+ // Execute should modify
+ var executeResult = FluidUtil.tryEmptyContainer(sourceStack, targetTank, 1000, null, true);
+ if (!executeResult.isSuccess())
+ throw new AssertionError("Failed to transfer.");
+ checkItemStack(executeResult.getResult(), Items.BUCKET, 1);
+ checkFluidStack(targetTank.getFluid(), Fluids.WATER, 1000);
+ checkItemStack(sourceStack, Items.WATER_BUCKET, 2); // Apparently the stack is not supposed to be modified
+ }
+
+ /**
+ * Ensures that tryEmptyContainerAndStow doesn't duplicate fluids in the target when the container is stackable.
+ * Regression test for issue #6796.
+ */
+ private static void test_tryEmptyContainerAndStow_stackable()
+ {
+ var sourceStack = new ItemStack(Items.WATER_BUCKET, 2);
+ var targetTank = new FluidTank(10000);
+ var overflowInventory = new ItemStackHandler(1);
+
+ // Simulate first: it's not supposed to modify anything!
+ var simulateResult = FluidUtil.tryEmptyContainerAndStow(sourceStack, targetTank, overflowInventory, 1000, null, false);
+ if (!simulateResult.isSuccess())
+ throw new AssertionError("Failed to transfer.");
+ checkItemStack(simulateResult.getResult(), Items.WATER_BUCKET, 1);
+ // Tank and inv shouldn't be modified for simulate
+ checkItemStack(sourceStack, Items.WATER_BUCKET, 2);
+ checkFluidStack(targetTank.getFluid(), Fluids.EMPTY, 0);
+ checkItemStack(overflowInventory.getStackInSlot(0), Items.AIR, 0);
+
+ // Now test with execute
+ var executeResult = FluidUtil.tryEmptyContainerAndStow(sourceStack, targetTank, overflowInventory, 1000, null, true);
+ if (!executeResult.isSuccess())
+ throw new AssertionError("Failed to transfer.");
+ checkItemStack(executeResult.getResult(), Items.WATER_BUCKET, 1);
+ checkFluidStack(targetTank.getFluid(), Fluids.WATER, 1000);
+ checkItemStack(overflowInventory.getStackInSlot(0), Items.BUCKET, 1);
+ }
+
+ private static void checkItemStack(ItemStack stack, Item item, int count)
+ {
+ if (stack.getItem() != item)
+ throw new AssertionError("Expected item " + ForgeRegistries.ITEMS.getKey(item) + ", got: " + ForgeRegistries.ITEMS.getKey(stack.getItem()));
+ if (stack.getCount() != count)
+ throw new AssertionError("Expected count " + count + ", got: " + stack.getCount());
+ }
+
+ private static void checkFluidStack(FluidStack stack, Fluid fluid, int amount)
+ {
+ if (stack.getFluid() != fluid)
+ throw new AssertionError("Expected fluid " + ForgeRegistries.FLUIDS.getKey(fluid) + ", got: " + ForgeRegistries.FLUIDS.getKey(stack.getFluid()));
+ if (stack.getAmount() != amount)
+ throw new AssertionError("Expected amount " + amount + ", got: " + stack.getAmount());
+ }
+}
diff --git a/src/test/resources/META-INF/mods.toml b/src/test/resources/META-INF/mods.toml
index 00a34bd01b..16a02b4541 100644
--- a/src/test/resources/META-INF/mods.toml
+++ b/src/test/resources/META-INF/mods.toml
@@ -221,6 +221,8 @@ modId="gametest_test"
[[mods]]
modId="many_mob_effects_test"
[[mods]]
+modId="fluid_util_test"
+[[mods]]
modId="potion_size_event_test"
[[mods]]
modId="recipe_book_extension_test"