Allow teapots to be filled from fluid containers, and allow milk pickup #57

jshipley · 2024-10-15T20:30:30Z

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!

jshipley · 2024-10-15T23:13:26Z

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.

KnightMiner · 2024-10-16T03:25:40Z

src/main/java/knightminer/simplytea/item/TeapotItem.java

+ BlockEntity entity = world.getBlockEntity(pos);
+ Optional<IFluidHandler> fluidHandler = Optional.empty();
+ if (entity != null) {
+ fluidHandler = entity.getCapability(ForgeCapabilities.FLUID_HANDLER, side).resolve();


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.

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.

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.

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.

KnightMiner · 2024-10-16T03:32:14Z

src/main/java/knightminer/simplytea/core/config/Teapot.java

+ fill_from_fluid_tank = builder.comment("If true, the teapot can be filled with water from a fluid tank")
+ .translation("simplytea.config.teapot.fill_from_fluid_tank")
+ .define("fill_from_fluid_tank", true);


I'm not sure this needs to be a config. The cauldron stuff was a config option mostly as cauldrons are super messy, though tbh we can clean up that cauldron code using the new vanilla API a ton. Feel free to update the cauldron logic in this PR or make a separate PR if you are interested, see CauldronInteractions.

I think for this, I would like the config to specify how much to fill. Default it to 1000, but a modpack might decide 250 makes more sense for a pot of water. If it goes down to 0 we could disable it I guess, as consuming 0 fluid does not make sense.

Removed this config.

I also added a teapot_capacity config option, and changed the CauldronInteraction for teapots to lower the level of the cauldron according to the teapot_capacity by rounding to the nearest 1/3 bucket.

When this is updated to NeoForge 1.21 the CauldronInteraction for teapots can probably be removed in favor of using the CauldronWrapper with the fluid handling API.

KnightMiner · 2024-10-16T03:35:18Z

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.

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.

KnightMiner · 2024-10-16T03:37:18Z

src/main/java/knightminer/simplytea/item/TeapotItem.java

+ 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)) {


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.

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.

KnightMiner · 2024-10-16T03:40:44Z

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.

…up. Close KnightMiner#46

jshipley · 2024-10-17T06:54:46Z

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.

jshipley · 2024-10-17T06:57:20Z

Also, the latest round of changes/testing have all been done on the 1.20 branch.

KnightMiner · 2024-10-17T08:49:29Z

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

Thats reasonable. Lets stick with teapots being fill only and disallowing empty then.

KnightMiner · 2024-10-17T08:50:30Z

src/main/java/knightminer/simplytea/fluid/FluidTeapotWrapper.java

+ public static boolean isWater(Fluid fluid) {
+ // Many modded fluids use the WATER tag to give the fluids entity interaction
+ // Only accept Fluids.WATER and not any other fluid tagged as WATER
+ return fluid.isSame(Fluids.WATER);


Probably should check fluid.isSame(Fluids.FLOWING_WATER) as well here. I doubt flowing water is actually going to show up in a tank, but just in case.

I guess it wouldn't hurt. It shouldn't change anything unless some other mod puts flowing water into a tank for some reason.

KnightMiner · 2024-10-17T08:56:26Z

src/main/java/knightminer/simplytea/core/config/Teapot.java

@@ -18,6 +22,9 @@ public Teapot(ForgeConfigSpec.Builder builder) {
 milk_cow = builder.comment("If true, cows can be milked using a teapot to fill it with milk")
 .translation("simplytea.config.teapot.milk_cow")
 .define("milk_cow", true);
+ teapot_capacity = builder.comment("Amount of fluid a teapot can contain")


Since we are not doing emptying, lets make this say something like "Amount of fluid consumed when filling a teapot from a tank". Contain suggests you can empty it into a tank

KnightMiner · 2024-10-17T08:56:58Z

src/main/java/knightminer/simplytea/core/config/Teapot.java


 public class Teapot {
 private BooleanValue infinite_water;
 private BooleanValue fill_from_cauldron;
+ private BooleanValue fill_from_fluid_tank;


I think this was forgotten during cleanup.

jshipley · 2024-10-18T18:10:59Z

I've cleaned up a couple of things and removed the last fluid transfer from TeapotItem::use()

KnightMiner reviewed Oct 16, 2024

View reviewed changes

KnightMiner mentioned this pull request Oct 16, 2024

add recipes to empty teapots (hot or cold) #58

Draft

jshipley changed the base branch from 1.19 to 1.20 October 16, 2024 14:45

jshipley added 3 commits October 16, 2024 23:22

Add FluidHandler to teapot

5564460

Allow teapots to be filled from fluid containers, and allow milk pick…

9b3f284

…up. Close KnightMiner#46

Merge branch 'fluid_teapot' into fill_from_tank

1cdb87c

jshipley force-pushed the fill_from_tank branch from a9c8888 to 1cdb87c Compare October 17, 2024 05:35

Added configurable teapot capacity

e642204

KnightMiner reviewed Oct 17, 2024

View reviewed changes

Small cleanup changes for PR

c605e79

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow teapots to be filled from fluid containers, and allow milk pickup #57

Allow teapots to be filled from fluid containers, and allow milk pickup #57

jshipley commented Oct 15, 2024 •

edited

Loading

jshipley commented Oct 15, 2024

KnightMiner Oct 16, 2024 •

edited

Loading

jshipley Oct 17, 2024

KnightMiner Oct 17, 2024

jshipley Oct 17, 2024

KnightMiner Oct 16, 2024

jshipley Oct 17, 2024

KnightMiner commented Oct 16, 2024

KnightMiner Oct 16, 2024 •

edited

Loading

jshipley Oct 17, 2024

KnightMiner commented Oct 16, 2024

jshipley commented Oct 17, 2024

jshipley commented Oct 17, 2024

KnightMiner commented Oct 17, 2024

KnightMiner Oct 17, 2024

jshipley Oct 18, 2024

KnightMiner Oct 17, 2024

jshipley Oct 18, 2024

KnightMiner Oct 17, 2024

jshipley Oct 18, 2024

jshipley commented Oct 18, 2024

Allow teapots to be filled from fluid containers, and allow milk pickup #57

Are you sure you want to change the base?

Allow teapots to be filled from fluid containers, and allow milk pickup #57

Conversation

jshipley commented Oct 15, 2024 • edited Loading

jshipley commented Oct 15, 2024

KnightMiner Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KnightMiner commented Oct 16, 2024

KnightMiner Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KnightMiner commented Oct 16, 2024

jshipley commented Oct 17, 2024

jshipley commented Oct 17, 2024

KnightMiner commented Oct 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jshipley commented Oct 18, 2024

jshipley commented Oct 15, 2024 •

edited

Loading

KnightMiner Oct 16, 2024 •

edited

Loading

KnightMiner Oct 16, 2024 •

edited

Loading