Skip to content
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

Implement Bucket Behavior to Cells #2660

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

ghzdude
Copy link
Contributor

@ghzdude ghzdude commented Nov 13, 2024

What

allows GT cells to pick up and place in world fluids

Implementation Details

add behavior to ItemFluidContainer

Outcome

"Finally, a weapon to surpass Thermal Expansion Reservoirs"

@ghzdude ghzdude added the type: feature New feature or request label Nov 13, 2024
@ghzdude ghzdude requested a review from a team as a code owner November 13, 2024 03:35

blockHandler = createHandler(cellFluid, world, pos.offset(facing));
success = GTTransferUtils.transferFluids(cellHandler, blockHandler) > 0;
isFill = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these isFill backwards? Right now you are setting isFill to true when you are attempting to transfer fluid from the cell into the world, which is emptying, not filling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i interpreted it as "filling the block", but it really could be either or.


var blockHandler = FluidUtil.getFluidHandler(world, result.getBlockPos().offset(result.sideHit),
result.sideHit);
int freeSpace = cellHandler.getTankProperties()[0].getCapacity() - (cellFluid == null ? 0 : cellFluid.amount);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic feels fragile to me-- if cellhandler.drain() fails to drain due to the handler saying no, but the handler contains fluid, that situation isn't accounted for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, if we're unable to drain for some reason, then tryPlace() will return false and then we try to fill the held container. if both fail then nothing will happen

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this earlier check exist then if it can fail with no repercussions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the desired behavior is to pickup fluid if there's still space in the container, rather than prioritizing placing down the fluid.

now that i think about it, i think i could just flip tryPlace() and fillCell() and just use rayTrace(world, player, true), though there's still the edge case of placing fluid in other fluid.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to do that, or just approve and merge?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm gonna try to see if i can simplify things, don't merge yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants