Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.21] Fix fluid pushing, update wasTouchingWater flag again #1586

Open
wants to merge 7 commits into
base: 1.21.x
Choose a base branch
from

Conversation

GizmoTheMoonPig
Copy link
Contributor

@GizmoTheMoonPig GizmoTheMoonPig commented Oct 10, 2024

Hello once again!

As people are probably well aware of by now, I'm working on a Betweenlands port. Betweenlands has a "swamp water" fluid that is mostly similar to vanilla water, but I can't use vanilla water for a few various reasons unrelated to this discussion. I noticed while working through the fluid logic that there was no way to get vanilla aquatic animals to swim in this water. I tracked down the issue and found it was related to the isInWater method in the entity class, which uses the wasTouchingWater boolean. This boolean will never be set to true if the entity is not in vanilla water. Now, the method responsible for changing this uses the vanilla water tag, but for some reason, Neo changes the tag check to simply just be a fluid type check. I'm not quite sure why this is, so I created a PR to somewhat change it back.

This PR also fixes #1575. I was originally gonna make that its own PR but it turned out I needed to mess with the updateFluidHeightAndDoFluidPushing for that as well so I figured it would be fine. I can split it off if desired.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@neoforged-compatibility-checks
Copy link

neoforged-compatibility-checks bot commented Oct 10, 2024

@GizmoTheMoonPig, this PR introduces breaking changes.
Fortunately, this project is currently accepting breaking changes, but if they are not intentional, please revert them.
Last checked commit: ff12e90ebdfefde915bcbd2a21a670a34e02a3f3.

neoforge (:neoforge)

  • net/minecraft/world/entity/Entity
    • updateFluidHeightAndDoFluidPushing()V: ❗ API method was removed

@GizmoTheMoonPig
Copy link
Contributor Author

After a little discussion on the neo discord, I moved this to use a fluid type property instead of the vanilla tag. The name isnt final by means (I'm very bad at naming things, please give me suggestions!), but this is unfortunately still a breaking change as I needed to return whether this property is true or not in the updateFluidHeightAndDoFluidPushing method.

@TelepathicGrunt TelepathicGrunt added bug A bug or error breaking change Breaks binary compatibility labels Nov 9, 2024
TelepathicGrunt
TelepathicGrunt previously approved these changes Nov 9, 2024
@GizmoTheMoonPig GizmoTheMoonPig changed the title [1.21] Update wasTouchingWater flag again [1.21] Fix fluid pushing, update wasTouchingWater flag again Nov 10, 2024
@GizmoTheMoonPig
Copy link
Contributor Author

UPDATE: someone asked me about looking into another fluid issue and I fell down a rabbit hole trying to fix that. I had to adjust updateFluidHeightAndDoFluidPushing once again to get it functional so I ended up wrapping the change into this PR as well. I will split it into its own PR if desired, just let me know

vec3 = vec3.multiply((double)f, 0.8F, (double)f);
this.setDeltaMovement(this.getFluidFallingAdjustedMovement(d1, flag, vec3));
- } else {
+ } else if (this.isInLava() && this.isAffectedByFluids() && !this.canStandOnFluid(fluidState)) {
Copy link
Contributor

@TelepathicGrunt TelepathicGrunt Nov 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this isInLava method call supposed to be (this.isInFluidType(fluidstate) && fluidstate.getFluidType() != net.neoforged.neoforge.common.NeoForgeMod.LAVA_TYPE.value()) to match what you did above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all honesty I have no idea. I just replicated all the changes as they used to be in 1.21.1 and it works fine. I'll see if its necessary for it to be like this

@neoforged-automation
Copy link

@GizmoTheMoonPig, this pull request has conflicts, please resolve them for this PR to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaks binary compatibility bug A bug or error needs rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entities pushed by lava move more quickly than in vanilla
2 participants