Sophisticated Backpacks

Sophisticated Backpacks

89M Downloads

Backpacks void/dupe fluids when using in tanks.

Speiger opened this issue ยท 4 comments

commented

Basically found this bug when testing the backpacks with my tanks.

The Dupe happens because backpacks do not support the "copy" of ItemStacks properly.
ForgeMethod being used (latest forge version) FluidUtil.tryEmptyContainer

@Nonnull
public static FluidActionResult tryEmptyContainer(@Nonnull ItemStack container, IFluidHandler fluidDestination, int maxAmount, @Nullable PlayerEntity player, boolean doDrain)
{
    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);
                if (transfer.isEmpty())
                    return FluidActionResult.FAILURE;

                if (doDrain && player != null)
                {
                    SoundEvent soundevent = transfer.getFluid().getAttributes().getEmptySound(transfer);
                    player.level.playSound(null, player.getX(), player.getY() + 0.5, player.getZ(), soundevent, SoundCategory.BLOCKS, 1.0F, 1.0F);
                }

                ItemStack resultContainer = containerFluidHandler.getContainer();
                return new FluidActionResult(resultContainer);
            })
            .orElse(FluidActionResult.FAILURE);
}

While the Destination gets the fluid forcefeed by this method, it doesn't affect the "container" which means you can place a wrapper around the destination that just simulates everything.

Why am I doing this?
Fairly simple: I also need to test if the output can actually fit into the output slots. (Draining buckets and stacking them without voiding them or forcefull non stacking them, helps automation a bit)

So this method (a forge standard) isn't supported.

What it allows is to dupe/void fluids depending on the action in its item form.

Any solutions you would suggest?
By the way I looked through mcjtys mods & Immersive Engineering they would have the exact same bug with your mod.

Version used:
(https://www.curseforge.com/minecraft/mc-mods/sophisticated-backpacks/files/3597548

commented

There are actually multiple discussions on this method and the fact that doDrain doesn't do anything (apart from the sound) and other issues that exist with this method.
Immersive Engineering actually has its own implementation probably because they have hit one of these issues and even though that implementation also copies the stack it only simulates the calls against the copied stack so in fact it doesn't need the copy at all (probably a relic of copying that from forge and then fixing an issue).
McJty's mods as far as I can see use the forge method directly so yes they will have this issue with the backpack.

But to be honest the expectation that the copy of stack also includes copy of the whole fluid handler doesn't seem alright to me. For example ender storage used to have their tanks as fluid item handlers (doesn't as of 1.15) and I can't think of a way that could be handled there for sure as that would require knowing that there needs to be a separate simulation network created.
And the same happens with my stuff here - data is stored separately from itemstack, copying that on stack copy would require a mixin in itemstack and then I wouldn't have a way to tell that I should actually clean that data up so the only thing this would achieve is likely really quick filling of storage data and player's running out of memory and of course a lot of redundant data on disk.

If this really needs to be solved the only "solution" from my side could be to remove fluid item handler capability from the backpacks.

I will think about this a bit more how I want to approach this, but potentially I will just add config setting that will allow modpack makers to disable the capability.

commented

Basically anything that wants to test if the output, that is created by draining a container, fits into the output is kinda forced to "not simulate" because a simulation will not create a output container.

Outside of adding that extra info, yeah its fine by me.
I just wanted to warn you that people can use your backpacks to dupe any fluids.
It goes without saying that this technically affects any capability that expect transforming the input when draining something.

Anyhow good luck on finding a solution.
I would be happy to help if wanted.

commented

config to disabled fluid item handler cap on backpack is included in the latest version

commented

o/ @P3pp3rF1y
MinecraftForge/MinecraftForge#9004
Might fix the bug. I test it when it comes out.