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"