-
Notifications
You must be signed in to change notification settings - Fork 9
Fluid controllers #496
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
base: master
Are you sure you want to change the base?
Fluid controllers #496
Conversation
|
Forgot it had to wait till idra changed some fluid code, mb |
|
Ok, you should be able to update this now since the fluid valve changes are in master. Let me know if you need any help or context on this. I am also happy to make the relevant changes if you prefer just lmk |
LordIdra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 things rq
1 - this can be removed in both
Preconditions.checkState(context instanceof BlockCreateContext.PlayerPlace, "Fluid filter can only be placed by a player");
Player player = ((BlockCreateContext.PlayerPlace) context).getPlayer();
2 - this should be yeeted and just replaced with a 1000 mb buffer
// a bit of a hack - treat capacity as effectively infinite and override
// fluidAmountRequested to control how much fluid comes in
setCapacity(1.0e9);
LordIdra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more stuff
src/main/java/io/github/pylonmc/pylon/base/content/machines/fluid/FluidAccumulator.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/machines/fluid/FluidAccumulator.java
Outdated
Show resolved
Hide resolved
| event.setUseInteractedBlock(Event.Result.DENY); | ||
| event.setUseItemInHand(Event.Result.DENY); | ||
|
|
||
| Window.single() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use PylonGuiBlock here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no because it doesn't allow me to add close/open handlers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You won't need them after swapping to FluidThresholdButton as I mentioned in my next comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this, rebase off my latest PR (even-more-cargo-block), and do what I did with FluidThresholdButton in CargoFluidAccumulator. It's really simple and you can basically just copy paste it. It's a lot more convenient to just have 1 button with 4 controls (and maintains consistency)
src/main/java/io/github/pylonmc/pylon/base/content/machines/fluid/FluidAccumulator.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/machines/fluid/FluidAccumulator.java
Outdated
Show resolved
Hide resolved
| event.setUseInteractedBlock(Event.Result.DENY); | ||
| event.setUseItemInHand(Event.Result.DENY); | ||
|
|
||
| Window.single() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use PylonGuiBlock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same reason as above
Progress:
Most of the display entities were always done by idra, so gonna delegate that thing to him, maybe in another PR