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 IFluidHandlerItem capability for EdelwoodBucketItem #377

Open
wants to merge 2 commits into
base: 1.20.x
Choose a base branch
from

Conversation

all-yall
Copy link
Contributor

Full disclosure that I am trying to get into Minecraft modding by fixing issues on existing mods, so feedback is very much appreciated.

I took a look at this issue that talked about cross mod compatibility and the edelwood bucket. I determined the root issue to be that edelwood bucket inherits the capabilities of its parent which don't work correctly for it. So here is an implementation of the IFluidHandlerItem for the edelwood bucket.

I modeled the bucket as having two internal tanks, one for water and one for lava. This allowed properly receiving different quantities of the two fluids instead of having one max capacity. Additionally, once on of the tanks has any liquid, the other will not take any.

@all-yall
Copy link
Contributor Author

Just realized that MobBucket extends EdelwoodBucket, so I should update the PR to return null for the initCapabilities function un that class.

Right now there could be some funky interaction if one tried to fill a chicken Bucket with a create spout.

@stal111
Copy link
Owner

stal111 commented Dec 14, 2023

Hi, thanks for your effort. While this seems to fix the issue, I'm not sure whether I really like the solution. I think it would have been better if the capability only had one tank and the max capability gets passed through a constructor parameter. This would also be better for possible addon mods adding their own Edelwood Bucket item instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants