-
Notifications
You must be signed in to change notification settings - Fork 12
Allow teapots to be filled from fluid containers, and allow milk pickup #57
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
Conversation
|
I did most of this before the 1.20.1 PR was merged. Please let me know if you want me to resubmit this for 1.20.1 instead of 1.19.2. |
| BlockEntity entity = world.getBlockEntity(pos); | ||
| Optional<IFluidHandler> fluidHandler = Optional.empty(); | ||
| if (entity != null) { | ||
| fluidHandler = entity.getCapability(ForgeCapabilities.FLUID_HANDLER, side).resolve(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't check the block fluid handler; just make the teapot item expose a fluid handler. See vanilla water buckets as an example (we won't be able to use the same fluid handler exactly, but should be able to use something similar). We shouldn't have to change any of this pickup logic, and that has the added benefit of supporting UI based fluid tanks.
You might find this class from Tinkers' Construct useful for reference, here is how to use it. Just know that is for an empty-only handler, we want a fill only handler. For our sake we also don't need all that supplier stuff, could just statically create the fluid stack as long as it copies in the right spots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the fluid handler to the teapot item simplified the use() method quite a bit, but there are still blocks that aren't working without accessing the fluid handler.
The main blocks I found that weren't working were the MilkJarBlock and CowJarBlock from Cooking for Blockheads. The useItem() methods for those blocks specifically check for the bucket item, and the FluidUtil.tryPickUpFluid() method fails because there's no FluidState for the blocks.
So leaving a FluidUtil.tryFillContainer() attempt if FluidUtil.tryPickUpFluid() fails makes it work in cases where it would otherwise fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd argue that sounds like a bug with the mod that adds MilkJarBlock, it would likely also break other mods of mine like Ceramics. We can probably leave that case as not working for now unless its easy to fix that without hardcoding/adding redundant logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's probably due to milk not being a real fluid in Vanilla. Maybe I'll look into submitting a PR for Cooking for Blockheads to see if it can be improved from that side.
Either one is fine, I'll probably copy the PR back and forth once done as the code is mostly similar. Its probably a little easier if both PRs are on 1.20 but this is fine. |
| Fluid fluid = fluidState.getType(); | ||
| // comparing with WATER fluid instead of WATER fluid tag, because many/most modded fluids are tagged as WATER | ||
| // to get water-like interactions even if the fluid is not water | ||
| if (fluid.isSame(Fluids.WATER)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also support milk if Forge's ForgeMod#MILK.isPresent() or whatever the check is to see that the milk fluid is enabled. In world pickup is not needed unless its easy to do, but definitely support it in the fluid handler I talk about in my other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Between using the fluid handler on the teapot and using FluidUtil to pick up fluids, milk should be working without needing to specially check whether a milk fluid is enabled.
|
As another thought on fluid handlers, might be nice if the filled teapots also act as fluid handlers, so you can dump them out into tanks. Only do it for the not-hot handlers, once its hot its a food now and is relevant to the other PR's domain. |
a9c8888 to
1cdb87c
Compare
|
The only problem I see with adding empty-only fluid handlers to the (cold) filled teapots is that a teapot filled with milk will only be able to be emptied if fluid milk is enabled with other mods. So the water teapot and milk teapot will often have different behavior. I think that the suggestion from #58 to make the cold teapots dumpable with animation, sound effects, and particles would probably be better. That also keeps the teapots from becoming too much like water buckets. |
|
Also, the latest round of changes/testing have all been done on the 1.20 branch. |
Thats reasonable. Lets stick with teapots being fill only and disallowing empty then. |
src/main/java/knightminer/simplytea/fluid/FluidTeapotWrapper.java
Outdated
Show resolved
Hide resolved
|
I've cleaned up a couple of things and removed the last fluid transfer from TeapotItem::use() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one quick fix then this looks good to merge. Was sick last week so not much time to review this.
| } | ||
| } | ||
|
|
||
| // use teapot like a bucket to get fluid, should work in most cases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work if you make the config lower than 1000 for the teapot capacity? If not we need to special case that here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the way FluidUtil is handling it, it should be treated as a successful pickup as long as the item's fluid handler returns a non-zero filled amount.
The item fluid handler (FluidTeapotWrapper) uses the configured size. It will either "fill" 0 (fail) or teapotCapacity (success).
…ad of when it's not set
Changes to teapot to support picking up water or milk from any block with fluid handler capabilities.
Also added new config option to allow/disallow pickup from fluid handler blocks.
Milk (if it's enabled) is also supported in more cases.
Tested with Flopper, Cooking for Blockheads sink and cow bottle, and Mekanism fluid tank.
Compiled/tested against 1.19, but the changes should also work for the 1.20 branch.
Closes #46
Happy Modtoberfest 2024!