PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

43M Downloads

Enderio "tank" is not a tank

tyler489 opened this issue ยท 6 comments

commented

For feature requests, just erase this template and clearly describe the feature you'd like to see

Minecraft Version

1.12.2

Forge Version

14.23.2.2615

Mod Version

0.5.1-163

Describe your problem, including steps to reproduce it

Amadron tablet doesnt recognise Enderio's Tank as a valid place for fluids

Any other comments?

if this is on enderio let me know how to add support please

commented

Confirmed. The Amadron tablet can select an EnderIO tank as a fluid source/dest, but when you open the tablet's GUI, any fluid trades are still marked red (unavailable), including trades where you have the emeralds and want the fluids.

This looks like EnderIO's problem, because:

  • EnderIO tank returns a SmartTankFluidHandler as its IFluidHandler implementation
  • When setting up offers in the Amadron GUI, PNC calls handler.fill(fluid, false) to test if the receiving tank can accept the offered fluid (this is the case where the player is getting a fluid from the trade)
  • handler.fill(fluid, false) is always returning 0 for the EnderIO tank, indicating that the tank can't accept any fluid, even when it's empty.

It seems like the EnderIO tank hasn't properly implemented the IFluidHandler#fill() method. One thing to note is that the Amadron system just gets the fluid handler from the null side, regardless of the actual clicked side of the block. However, this hasn't been a problem with any other mod's tanks, and the EnderIO tank is still giving us what appears to be a valid IFluidHandler object when we call tileEntity.getCapability(CapabilityFluidHandler.FLUID_HANDLER_CAPABILITY, null); - if null isn't a valid side (although there's no reason it shouldn't be here), the tile entity shouldn't really be returning a fluid handler at all.

commented

So, EnderIO is more picky than most mods about sidedness, and its null fluid handler doesn't allow any kind of insertion/extraction (and that's not likely to change).

The other option is to add sidedness to the Amadron tablet; record not just the location & dimension of the inventory/fluid handlers, but also the clicked side. And use that side for accessing the handler capabilities of the TE later on. That's not hard to add, so should be in the next update.

commented

One extra complication is Amadron custom (player->player) offers, where the "Add Trade" GUI can use GPS tools for setting item/fluid inventory locations. GPS tools don't store sidedness, so we may need to add "Side" dropdown widgets for selecting which face resource providing & returning inventories are accessed on.

(Amadron custom offer handling also appears to be quite bugged right now, and I'm addressing that too. Given that no one has reported the obvious problems there yet, I'm guessing custom offers aren't much used at this point...)

commented

I personally don't see a need for adding sidedness to the capability retrieval. The javadoc for the ICapabilityProvider states that null is a valid value:

@param facing The Side to check from:
CAN BE NULL. Null is defined to represent 'internal' or 'self'

I think adding sidedness unnecessarily complicates it. And if the tank does not work, as a last resort you could just hook up something like a Liquid Hopper which extracts from the tank, and assign that to the Amadron Tablet.

commented

This is a forge issue, not yours. The reason we don't return an internal inventory is the chance of some dodgy code meaning adjacent blocks access the inventory of the tank despite side rules. It would be nice if there was an enum or something for that, referring to remote/internal access instead of just "null".

I am going to suggest a change to forge for the 1.13 update so that capability has ANY and INTERNAL as options to distinguish your case from bad coding/cheaty accessing. There are some mods that ANY would make sense for, and modders can act accordingly then.

commented

OK, for now we'll stick with MineMaarten's suggestion, which works fine (the Liquid Hopper can hold 16 buckets anyway, so is suitable to use on its own for the vast majority of trades).