Skip to content

Conversation

@EmmaTheMartian
Copy link

Closes #1629

The Forge implementation feels pretty hacky. I considered trying to change ComputerCraftAPIImpl#fluidStackDetails to a DetailRegistry<IFluidTank> or similar, however it seemed like it would have been less compatible or would entail a more breaking change for addon developers.

The Fabric implementation was clean-cut.

I also made this change for 1.21.x and will make a separate PR for that.

@EmmaTheMartian EmmaTheMartian changed the title Add capacity to fluid tank data Add capacity to fluid tank data (1.20.x) Jul 10, 2025
@SquidDev SquidDev added the area-Minecraft This affects CC's Minecraft-specific content. label Jul 11, 2025
@SquidDev
Copy link
Member

SquidDev commented Jul 11, 2025

Thanks for the PR!

I'm not 100% sure about putting this in the result of tanks(). This method is somewhat meant to mirror inventory's list() method, and only return information about the fulid itself, while the capacity is much more a property of the tank itself1.

I wonder if a better approach here would be to have a getTankCapacity(tank: number), like getItemLimit. Though then again, that won't work on Fabric, unless we want to constrain ourselves to slotted storages...

I'm not sure. Thoughts welcome!

Footnotes

  1. The fluid peripheral is so badly designed. It's so tied to Forge's specific interface for fluid tasks :/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Minecraft This affects CC's Minecraft-specific content.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing a way to get the capacity of a tank/fluid storage

2 participants