2nd attempt [1.12.x] Custom Fluids Behave Like Water

[jabelar]

Preface: This is a 2nd attempt, superseding #4478. After some failed attempts at rebasing (which wiped out my changes to vanilla classes) plus some further changes I wanted to make, I had to start new fork and do a significant re-write. I apologize to those who spent time reviewing the last one, although this PR ultimately is much the same.

Background: Custom fluids do not naturally do water-like things such as drown entities, push entities along with flow, water plants, float boats, etc. This PR addresses all these behaviors. The reason it hadn't been done previously is because Material.WATER was hard-coded throughout the vanilla code, so needs a patch for every behavior. Each patch is very simple, but water does a lot of stuff which is why so many files are affected.

Implementation Approach:

Material class has several added properties to reflect each waterlike behavior, along with setter and getter. Eg. setCanDrownEntity(). All default to false and existing liquids WATER and LAVA are set appropriately so this is entirely non-breaking.

Almost everywhere there is a check for Material.WATER was called, it is replaced with appropriate material property getter. There are a few exceptions for things related specifically to BlockLiquid as that class explicitly only handles WATER and LAVA (since custom "liquids" should use the fluid system).

The isInsideMaterial() method is replaced with some targeted ForgeHooks versions that pass a predicate instead (for test against the material property). So you can test if you're inside a drownable material, a material that can float a boat, etc. Note that there is one method that tests the entity's head and another that tests the entire bounding box.

Features Implemented:

• Flowing fluids can be enabled to push entities like water.
• Fluids can be enabled to drown entities like water. Air supply HUD overlay and DROWN damage work same as water.
• Fluids can be enabled to float boats.
• Fluids can be enabled to water plants. This affects farmland and things like reed generation.
• Fluids can be enabled to spawn water creatures. This also enables fishing.
• Fluids can be enabled to be "swimmable" meaning that any path navigation and AI will treat it as water note type.
• Fluids can be enabled to be absorbed by sponges.
• Fluids can be enabled so FOV changes like under water.
• Fluids can be enabled to emit water splash/bubble particles when moving through them. Note that even if disabled, fishing, drowing and guardian movement will still generate particles.
• Fluids can be enabled to mix with concrete powder to form concrete blocks.
• Fluids can be enabled to mix with lava to form obsidian.
• Fluids can be enabled to vaporize when certain dimensions support it.
• Fluids will extinguish fire.

Features Not Implemented (Possible Future Enhancements):

• Fluids' splash particles are the same as water, might be cool future enhancement to allow custom splash particles, or at least color them to match the fluid color.
• If a sponge is enabled to absorb your fluid, you will get regular water back from the sponge. Would be cool future enhancement for it to remember the fluid.
• If a fluid freezes it turns into standard ice (which if it melts goes to water). Would be cool future enhancement for it to support custom ice blocks.
• All liquids except lava will extinguish fire. I'll probably issue a pull request myself in future to fix this -- need to consider how it would interact with the canBurn() or if it is its own property.
• Only actual water generates dripping sounds. Would be cool future enhancement for other fluids to do that.
• Gases aren't really covered specifically, probably needs its own overhaul (being discussed by others already).
• Swimming movement is the same for all fluids/liquids. The isSwimming() method doesn't change this but rather sets whether the fluid is a valid swim path for the navigator. It would be interesting in the future to change swimming movement based on density and viscosity of the fluid.

Testing:

• Make sure water and lava still works as expected
• Place fluid with universal bucket (Misc creative tab) and try out all the various things you can do (including trying commenting out the property setters in the test mod).

Summary:

I know this is a big PR, but the logic is pretty simple and it provides a LOT of functionality with good modder control.

[jabelar]

I know the Forge folks are busy, but I'd like to make a case to get this PR implemented soon as there are a lot of other fluid-related PRs pending such as fluid sounds, fluid place events, fluid ingredients, fluids as fuels, and so forth. For the most part I don't think there are a lot of conflicts, in those other PRs but as my PR is the most far-reaching it will provide a good foundation for the rest. and should probably be implemented first.

I know other priorities like 1.13 are looming, but the sheer number of PRs related to fluids seems to indicate a healthy interest in the modding community which would be nice to fulfill sooner than later.

Thanks!

[HenryLoenwind]

It's out-of-scope for this PR, but this is the place I really, really, really want to be replaced with state.func_185904_a().getFogColor()...

[jabelar]

What do you mean? The fog color when you're inside a fluid already changes to the fluid color. I submitted a separate PR for this recently.

[jabelar]

See #4462

[HenryLoenwind]

As a modder who has spend hours on getting our fluids to even remotely behaving like water without all the bugs you get from being water, I really like this.

Just one thing I noticed: isSwimmable() is used both for allowing an entity to swim and for the AI to route into the liquid, isn't it? I think there should be a difference between the two things as there are liquids you can swim in but that do damage you when you do.

[jabelar]

Okay, that is a good suggestion. Most of my work was of course replicating water capabilities, but the tricky thing was how to handle lava-like (meaning ones that damage). Even though both are technically liquids, lava really only has a couple behaviors like water. So in the few places in vanilla code where lava was called out I have a method to distinguish (liquid other than lava).

However, your request makes me wonder if maybe there are any other lava-like properties that should be teased apart more specifically. Let me look.

Thanks for the feedback. Please help test if you can -- need to confirm that vanilla liquids still behave as expected in addition to being to fully customize our own fluids.

[jabelar]

Looking at it further, the isSwimmable() I implemented is really about path navigation. The ability to swim in a fluid is the same for all fluids. I think this is the right thing to do (for now).

Now arguably you might want something else to happen but frankly there is no vanilla movement mechanism for a liquid you can't swim in. The Fluid system tries to allow for gases though, but as mentioned in other Issue submissions the isGaseous is pretty broken. We really need a major overhaul to get that to work.

You can also argue that based on physics the density and viscosity of the fluid should be considered for swimming. But again someone would have to come up with a whole new movement mechanism. Like should you sink faster in low density fluids? Would high density fluids automatically float you? Swim slower in high viscosity fluids?

So, at this point my PR has all fluids allow for swimming movement. The isSwimmable() method is meant to indicate the fluid should be allowed for swimming path finding.

I think gases and alternative physics for swimming in different densities would be really cool. But I highly recommend we leave that to a separate PR.

[HenryLoenwind]

I'm fine with it as it is, just, plz, adapt the JavaDoc?

[jabelar]

Okay, I'll update the java doc to clarify what "swimmable" means in the morning.

[jabelar]

Dang it, people keep pushing liquid-related PRs while my overhaul is pending. I'll resolve the merge conflicts when I get a chance over the next day or so.

[Terrails]

There's quite a bit of fluid related PRs and others that are waiting to be merged, but can this be merged soon since a lot of people are waiting for this.

[jabelar]

Certainly interested in feedback and having people try it. I'll also try to resolve conflicts this week.

[nihiluis]

i want to express how this sounds like a very good pr to me, it's very smart, has clear purpose and author is really trying to improve something even though he is ignored for 8 months and still has good will, that's tremendous.

[LexManos]

This is a massive overhaul that needs more people's input.
Right now, from my end it just looks like a 'replace all ==Material.WATER with unique functions'. Where the most obvious question doesn't seem to be answered.
Why not just make your custom fluid's material == water? What downside does that have that makes it worth the maintenance cost of this change? Especially considering we KNOW that this is all going to explode in 1.13 anyways?

Moving down the diffs so this is more of a train of thoughts:
Your separation of things also doesn't quite make sense, Why force everything that isnt lava to allow for respiration/WATER_BREATHING? Why are there a ton of separate functions like 'canWaterPlants' and 'canMixWithConcrete' instead of one canHydrate(target) or something like that? Same for any other canMixWith* methods.
There are also places that are just asking for explosions, such as your lily pad changes, and the blockstate not having BlockLiquid.LEVEL properties.
Should we look into absorption holding more then just water? As this is a lossy operation, Perhaps do something similiar to buckets and make a sponge that holds the specific fluid?
Firstly all your public set functions is just asking for people to moidfy the values post constructors. Maybe move to a builder system or a lock system?
canDrownEntity, seems more appropriate that EVERYTHING can drown a entity and the better method would be Entity.canBreath(Material).
canPushEntity, would there be a usecase for this ever being false? If a fluid is flowing it should push things. One thing may be changing this from a boolean to a push force of some kind.. simplest would be the speed to move them per tick. But maybe something related to weight/'target' categories. Slow moving water pushes items but not mobs, Fast moving water pushes both... dunno...
canFloatBoat, not sure about this, there isnt much to it as it's hardcoded. But in the modding community we have concepts of different types of boats.. This is stone boat that floats in lava, or a wooden boat that buns in it? Would stone boats sink in lighter materials?
canSpawnWaterCreatures, Seems to be used for two things. Spawning, and fishing. Why are these combined? Should we allow different fishing loot tables based on material?
isSwimmable, why not just PathNodeType and let individual entities determine if they should be 'swimming' in the fluid not material.
canVaporize, Not a fan of this, should be part the placement function, also doesn't seem to be implemented anywhere.
hasLiquidParticles, Ya, used for both drip and splash/swim particles. Better to move this to a system to allow for custom particles. Would be weird to see your bright green fluid create blue drops.
canFreeze, needs to have accompanying freeze function that allows the fluid to specify what it's frozen form is.
changesFOV, only used in one place, but worth being a float so modders can edit the value.

https://github.com/MinecraftForge/MinecraftForge/pull/4619/files#diff-81128f9af712143e66ecac33b43ebd31R20 Could also use a note somewhere saying you change this, a seperate PR to fix this bug?
Also, you have a lot for issues like so: https://github.com/MinecraftForge/MinecraftForge/pull/4619/files#diff-97673260fb0239af80fa467c2f5f5086R1595

[jabelar]

Hey Lex, thanks for the detailed consideration. Yes, definitely a fairly big thing so needs input/testing.

High Level Discussion Points

There are a lot of reasons to not use WATER material, but best reason is that there are lots of fluids people want to do that shouldn't replicate all the behavior. Imagine something like gasoline that that you want to flow and push you like water, but don't want it to nourish plants, put out fire, or have fish in it.

Most things you might want to do can probably be done with creative coding, however the most difficult thing is having the fluid push you like water. To replicate that you have to do a bunch of event handling including reflection and copying extensive portions of vanilla behavior. You can get the pushing by simply making the material WATER, but then you get everything else that water gets.

Implementation Style Comments

I agree about the issue with my setters causing post-constructor changes. While functionally I think that would be safe to allow it does seem counter to the idea of representing a physical property. I was mainly trying to avoid needing complicated constructors. So you're right a builder system would be better. Let me see about implementing that (I'm not that familiar with doing that, but will check out the builders you all have been adding lately). Alternatively, probably easy to create a locking system.

Regarding why there are separate functions for everything instead of saying "canHydrate()" is because there are lots of examples where you don't want it all to come together. Fluids are not all water-like.

Functional Points To Discuss

Regarding whether there is a case where fluids wouldn't push entities, there is the bigger discussion about how the Fluid system represents density to the point of gasses and such. For example, I would say that if you wanted to use the system to create a "dry ice" like fog that flows like a liquid you wouldn't want it to push you. If there was a full API for heavier-than-air gasses then you could argue that most/all fluids should push. I like your idea of returning a push force though.

For isSwimmable, I like your idea of returning PathNodeType, but I'll need to see where that would hook in. Will definitely look at doing that.

Regarding sponge absorption holding more than water, that is definitely something I would have liked to have implemented, but seemed like a big thing on its own. I guess there would need to be a "universal sponge" similar to the universal bucket concept. So on a "to do" list.

The canFreeze() is easier than the sponge problem because the result of freezing would clearly be a distinct block -- I like your idea of having way of associating fluids with their "frozen" version. Bit of work but would be worth it. It had been on my "to do" as well.

For something like the canFloatBoat() your feedback is good -- the actual boat type should be passed in and therefore you could conditionally float it. Let me look at doing that.

For the canDrownEntity() flipping to canBreath() based on material is a good idea. I'll implement that.

Whether you can fish in a fluid can certainly be separated out from canSpawnWaterCreatures(). Since fish themselves aren't really an entity on their own, I kinda figured "wherever there are water creatures you can probably fish". But easy enough to separate. I do like the idea of having ability to adjust fishing loot tables based on the fluid so I'll look at that.

Conclusion

I appreciate your time on giving feedback. I'm willing to code up almost all the above, however as you mentioned with 1.13 coming maybe I should just reset/wait and see about re-implementing after that?

[stale]

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[MajorTuvok]

Not stale, give it more time...

[stale]

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[nihiluis]

not stale

[stale]

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[nihiluis]

I think 1.13 is coming closer...

[nihiluis]

I think 1.13 is coming closer...

[Alpvax]

This should probably be tagged for 1.14, as most fluid stuff is being delayed until then, due to Mojang's rewrite.

[kierajreed]

Yeah - 1.13 is more like a "stepping stone" version than a version to really play on.

[tterrag1098]

It is a bit of a catch-22 that the largest PRs that take the most work to maintain are the ones that stay open for comments the longest. There's not much that can be done to fix that.

As it stands this PR has bad merges in the patch files, and has had little input/review other than the same 3 people. It feels like a bit late in the 1.12 lifecycle to merge such a large overhaul. It might be time to consider targeting 1.13. While a lot of the fluid system is WIP in 1.13, the actual interaction with entities is pretty similar AFAIK.

[FreneticScribbler]

The suggested feature of custom loot tables for fluid materials would be really useful imho

[stale]

This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions.

[stale]

This pull request has been automatically closed because it has not had recent activity. Please feel free to update or reopen it.