Neither the Tanks nor the internal tanks of all machines accept IFluidContainerItems. That's probably because they only use getFluidForFilledItem(), but it only works for conventional fluid containers. IFluidContainerItems are intentionally made a special-case, because you can decide how much to drain, and other fluid containers require enough space for all of it's fluid to prevent resource losses.

Yes, the current interface is completely fine!
And, single-use containers could be made by always reducing the stacksize is doDrain is true, and then in the code that drains the item(or universal Forge item drain interface, if there's ever one) if the stackSize is 0 set the inventory content to null.

My understanding was that IFluidContainers were handled transparently by the FluidContainerRegistry.

If that's not the case, then they can suffer in uselessness. I'm not going to special case them. Go bug Forge to fix it.

They are not different, you have drain, you have fill, is empty, is full, is container. These things can all be handled by the FluidContainerRegistry without me having to change anything on the BC side. There is absolutely no reason to special case them.

The original interface that was proposed for these kinds of items was designed to be completely hidden from mods. The LiquidContainerRegistry handled them.

Well technically Covert, they are not special casing but rather a new method to handle 😄. They are not the same as the FluidContainers in the Registry as those have a filled and empty state, the IFluidContainerItems are there to allow the modder to completely control what happens upon drain or fill and can have multiple states since they are NBT sensitive.

The FluidContainerRegistry iirc was a just a port, without really any new addition.

Bottom Line: it isn't a bug in forge as they are different things and are handled differently. (Granted, the calls could be added to the FluidContainerRegistry, but that could cause some confusion)

Aengir, though I agree with closing this issue I need to know what's missing from the fluid api. Pointers would be helpful. @Soaryn do you know?

I think a general drain() method in FluidContainerRegistry would work. Pass in the stack, maxDrain and doDrain, and it does all required operations on the stack(substitute it with it's container item if it's a regular item, use it's drain method if it's an IFluidContainerItem) and returns the FluidStack drained. A general fill() method could be implemented similarly.

Agreeing with CovertJaguar here. Special-casing is bad, mkay.

Soaryn, CJ, please explain what the issue is? Is this missing stuff in forge, BC or neither?

@cpw The FluidContainerRegistry should handle IFluidContainerItems, it currently does not. I see no point in writing a delegate layer on top of FluidContainerRegistry just so that BC can handle IFluidContainerItems when that functionality could be added to the FluidContainerRegistry.

@Soaryn The PR linked was the basis for the existing interface. Its entirely relevant and shows how it should be done.

@viliml If you want that, then you can special case that. For BC's purposes, that's not needed.

Simply adding a fill/drain function where one returns an ItemStack and the other returns different information is not exactly Ideal, as they should be handled by the mod authors. (Not all ContainerInventories are the same)

Scenario: A tank filling a container with fluid.

• With a container in the Registry, it would pull the capacity out of the tank and simply returns the filled ItemStack state and then move to a new slot (typically an output slot).
• Where as the IFluidContainerItem should either sit their filling over time or completely fill based on how the modder decides (Someone may want it to only fill 10mB per tick). Realistically acting just like a tank would, and only after it completely fills would move to the new slot (Unless the modder adds something like a logic gui for controlling the fill amount).

The two systems are separate, and I find the current system is adequate and the modder has enough information to handle it however they need it to be handled.

BC only has to add compatibility with IFluidContainerItems, it isn't special casing it is adding new information and new type to the equation (merely using "instanceof" would work). Since the FluidRegistry considers stacks and the IFluidContainer considers the stack's info (ie. amount, fluidID, capacity)

@CovertJaguar a helper would be easy enough to do, I could understand if it was several hundred lines of code but it is only like 10 or less.

Regardless I'm done with this conversation. If BC doesn't support it or it doesn't get what ever it thinks it needs into forge to handle it for them, then oh well. It will just appear inconsistent with the players.

@cpw try catching me on IRC or TS if you need anything from me.

You contradict yourself. In one sentence you state that they can't be treated the same, and in the next you claim the change is 10 lines of code. Both statements cannot be true.

Have you actually made an IFluidContainerItem?

Also I said helper, not "change" The helper would come before trying to retrieve a new itemStack

No, but I've looked at the interface, yes whatever code bridge that needs to be written to handle both types of containers will look somewhat different from the FluidContainerRegistry interface, and it will mean modifying things to conform to the new bridge. But I see no point in putting that bridge in Buildcraft, Railcraft, and every other mod that uses Fluids. The API is lacking, fix it. This should have been done before the API went live.

@cpw this is rough, and untested, but gives you an idea: http://hastebin.com/kiyigosiji.axapta

One of the problems Im seeing @cpw is, IFluidContainerItems dont return a new stack, they modify the NBT of the existing stack, where as the the FluidContainerRegistry just returns some stack based on which state you want.
Personally I think they are fine as they are. And should just be handled but oh well.

@CovertJaguar the LiquidContainerRegistry infact did not handle these kinds of Items nor were these Items possible before with mod support.

Yes, it did as written by xCompWiz:
https://github.com/MinecraftForge/MinecraftForge/pull/415/files

Completely handled transparently by the LiquidContainerRegistry.

Not completely! It wasn't, and still isn't, possible to just PARTIALLY drain or fill the "portable liquid tank", or now the Fluid Container Item. https://github.com/MinecraftForge/MinecraftForge/pull/415/files#L1R381 : we DO want to interact directly with it! Or, at least, the Forge interface must be extended to cover what is now a necessary special-case.

Don't show me a PR from 6 months ago (especially one I had discussed with him) that DIDNT get pulled in favor for a different fluid system. Either "special case" it, PR something or provide some info for @cpw for things to add, other mods are handling it just fine.

The more I look at this IFluidContainerItem interface, the more flawed I realize it is. For example, its not possible to create a single-use container using the interface. Drain should return the resultant FluidStack AND ItemStack. Fill should return the amount used and the ItemStack. Similar to how my Bridge works.

Also, doFill and doDrain are 100% pointless. If you don't want to affect the item, just copy it first.

Why would you want to create a single-use container that can store a variable amount of fluid? It makes no sense! You can always drain little less than all of it and then just fill it back in!
Actually, a sort of single-use container would be possible if you set the fill function to do nothing!
And, doFill and doDrain are probably used to maintain code similarity to real Tanks. Also, why don't you just copy the tank when you don't want to modify it? It's the same thing! DoFill and doDrain are more readable than copying!

Covert, I already do that, the IFLuidContainerItem however (if that is what we are referring to) was designed to hold variable amounts. That was the point. For the FluidRegistry there is a server linked id to the Fluid. I only use the IFluidContainer for a container that should have an internal tank.

Also, if you do the rendering correctly you dont need a new texture for every new Item when making a basic container using the FluidRegistry. It is much simpler than it used to be.

Just try not to do breaking changes to the current system. OK thanks

You missed my point entirely @Soaryn. The interface lacks the ability to make single-use containers. I know you can do fancy rendering ect... I said as much in my last post. What you can't do is make single-use containers.

And yes, the existing interface needs to be tossed and one created that actually is appropriate for items.

Its not about storing variable amounts of Fluids, its about creating a container that can store ANY type of Fluid. That is something that is very hard, if not impossible, to do with the traditional FluidContainerRegistry. All of Forestry's containers for example require you to hand craft an item and texture for each Fluid.

Using an interface, its possible to create a generic container that can store anything, even Fluids you've never even heard of before.

Copy the TileEntity? What fool thing are you suggesting. There is no such mechanic, and if there was, it would break a ton of stuff, like Chests and MultiBlocks.

And no, doFill and doAdd are not more readable by either the caller or implementer. They just add unneeded complexity.

Actually, it's possible to make a container that can store any liquid.
"Plugins for Forestry" do that.

Yes, well the hacks required to scan all the Fluids, and programatically create an Item for each Fluid, and make sure they stay consistent even when new Fluids are added is not worth it.

If you didn't notice, IC2 experimental builds have a "Universal Fluid Cell" that can pick up any possible fluid form the world or any internal tank that supports it(currently only IC2), and place it anywhere into the world or in any internal tank of any machine that supports it(again, currently only IC2). And all that without the need for a huge argument about how flawed the Forge interface is! All that with about 5 lines of code(Literally, I decompiled it! It's so simple!)!
Also, as I said, you can do single-use containers by just making the fill function do nothing and return 0, and when it's empty, just set the container stack to null. You said it should return the ItemStack too. There's no need! IFluidContainerItems modify the stack anyway! Why wouldn't they set it to null?

Set it to null? You realize that will do nothing right? All it does is set that one pointer to null, all the other pointers (including the inventory) will still point at the ItemStack. And yes, I'm aware of the IC2 container and I'm also aware that its quite buggy and unreliable because yes it does edit the stacksize of the stack.

Really, that's how it works? Oh then I guess I have major issues with my understanding of the inner workings of Java... I thought that if you set a reference object to null, all the other references to the same object will become null too. Weird...

All other references are not changed. Pretty much how all languages work. ;)

@CovertJaguar ok now I understand :) there is supposed be a split. The interface wasn't designed to be single use, not that it couldn't be made to but it was literally discussed as it shouldn't be single use.

If you want a single use container you use the container registry, if you want a variable amount container or some other special use, use the interface, they are not the same for a reason :p.

The current interface is fine.