Deep Aether

Deep Aether

1M Downloads

Weird Poison Fluid Interactions caused by missing Poison Tag

VoidLeech opened this issue · 7 comments

commented

Found while messing around with Custom Fluid Mixin and getting weird results. Though CFM doesn't seem to work with the modded fluids I've tested anyhow, the issues are actually reproducible without CFM, in an instane with only Deep Aether and its dependencies.

Version

1.20.1-1.0.2

Nonexistent Fluid Tag

The 'poison' fluid tag isn't generated (or if it is, it isn't checked into the repo), so I made one myself. Doing so caused the poison onto water interaction to produce Gold Blocks.

Line

levelAccessor.setBlock(blockPos, net.minecraftforge.event.ForgeEventFactory.fireFluidPlaceBlockEvent(levelAccessor, blockPos, blockPos, Blocks.GOLD_BLOCK.defaultBlockState()), 3);
is the culprit, as that should clearly be Aersmog instead. The section has a bigger issue though, because since the poison tag doesn't exist (by default) anything deeper than the initial if in lines
if (direction == Direction.DOWN) {
FluidState fluidstate = levelAccessor.getFluidState(blockPos);
if (this.is(DATags.Fluids.POISON) && fluidstate.is(FluidTags.LAVA)) {
if (blockState.getBlock() instanceof LiquidBlock) {
levelAccessor.setBlock(blockPos, net.minecraftforge.event.ForgeEventFactory.fireFluidPlaceBlockEvent(levelAccessor, blockPos, blockPos, Blocks.CRYING_OBSIDIAN.defaultBlockState()), 3);
}
this.fizz(levelAccessor, blockPos);
return;
}
if (this.is(DATags.Fluids.POISON) && fluidstate.is(FluidTags.WATER)) {
if (blockState.getBlock() instanceof LiquidBlock) {
levelAccessor.setBlock(blockPos, net.minecraftforge.event.ForgeEventFactory.fireFluidPlaceBlockEvent(levelAccessor, blockPos, blockPos, Blocks.GOLD_BLOCK.defaultBlockState()), 3);
}
this.fizz(levelAccessor, blockPos);
return;
}
}
is never triggered and that causes the below:

Without adding the tag

If one has a setup with a poison source flowing directly down in the block below, placing a water bucket in this space below will have the water momentarily exist before being replaced by downward flowing poison.
In the same setup, placing a lava bucket creates a setup with the poison just resting above the lava, both coexisting without any interaction.

Placed

2024-02-10_18 04 45

And it's gone

2024-02-10_18 04 56

Upon adding the tag

With the same setup now, placing the water bucket produces a gold block, while placing lava doesn't trigger the crying obsidian still.
2024-02-10_18 08 15
2024-02-10_18 08 31
I checked that lava has the Lava tag in my test instance so either line

if (blockState.getBlock() instanceof LiquidBlock) {
must be failing, or net.minecraftforge.event.ForgeEventFactory.fireFluidPlaceBlockEvent is misbehaving, as undoubtably the same code works with water.

commented

That's a big damn oversight on my side indeed. It will be fixed in the near 1.0.3

commented

The tag isn't added to the dataSetup in the main class, resulting in never generating it :/

commented

I don't think 0082272 fully addresses the issue; poison flowing onto lava from above does nothing. though of course that wasn't posed in the issue title, only the body (A new issue could be opened too if you so wish)
I've made a very rudimentary test that doesn't involve me building from source (I could at some other time if desired) by giving water the lava tag and letting (tagged) poison flowing onto it.
image
Since the check for lava happens before water, this is fine.
In this test, the water gets replaced by crying obsidian but the lava remains. super.spreadTo() is clearly not being called as it was when untagged poison flowed onto water. That should confirm we're past the if (fluid below is tagged lava) and returning before that happens. That then either means lava isn't being detected as LiquidBlock, or the FluidPlaceBlockEvent isn't replacing lava.
image

commented

After some investigation, I found some odd behaviour.

2024-02-11_20 04 34

On the left is a test where Poison was placed before the Lava and the interaction works only for flowing blocks, while on the right is the Lava placed before the Poison as you did.
For some reason the method doesn't even get called on source Lava blocks with source Posion blocks, and when Poison is placed after the Lava like in your case, the direction == Direction.DOWN fails, and so I tested without it.

I'll try to figure this out soon, as it's late and can't think of a solution out of the box.

commented

The left works because of the lava flowing into a space with poison I believe, that’s handled in DAFluidInteraction.java.

So PoisonFluid.spreadTo just isn’t even being called when over lava, both source and flowing? That’s very odd.

commented

Hey I did some digging now that I'm more comfortable coding mods myself and ran into this too.
Ultimately, it seems the issue is with LavaFluid only allowing it to be replaced with water in contrast with WaterFluid's anything that isn't tagged water or ForgeFlowingFluid's anything that isn't the same fluid.

public boolean canBeReplacedWith(FluidState pFluidState, BlockGetter pBlockReader, BlockPos pPos, Fluid pFluid, Direction pDirection) {
        return pFluidState.getHeight(pBlockReader, pPos) >= 0.44444445F && pFluid.is(FluidTags.WATER);
    }

Something like the below should solve it (it did in my use case) while not changing the behaviour between another mod's fluids and lava

@Mixin(LavaFluid.class)
public abstract class LavaFluidMixin extends FlowingFluid {
    @ModifyReturnValue(method = "canBeReplacedWith", at = @At("RETURN"))
    private boolean your_mod_id_here$lavaIsReplaceableByNotJustWater(boolean original, @Local(argsOnly = true) Fluid fluid, @Local(argsOnly = true) Direction direction){
        return original || (direction == Direction.DOWN && fluid.is(YourFluidTag.Here));
    }
}
commented

Yep, I've reached that conclusion a couple of weeks ago when I put my mind on it again.
Thank you for writing the code too. I'm gonna fix it quite soon so that this issue can be finally closed