-
Notifications
You must be signed in to change notification settings - Fork 9
[ref:vaan/task/vacuum-hopper-enhancement] Vacuum hopper enhancement #493
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?
Conversation
|
requires pylonmc/pylon-core#529 |
Just use the |
|
I would personally put this off until we have a proper multi-inventoriy serialization system |
oh alr |
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.
Should use PylonGuiBlock here
Waiting once multi inv is a thing |
# Conflicts: # src/main/java/io/github/pylonmc/pylon/base/content/machines/simple/VacuumHopper.java # src/main/resources/lang/en.yml
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.
Sorry to be a pain here, but I would suggest changing the way the GUI is laid out here. If you have a look at how the cargo filter works on the even-more-cargo-blocks branch, I think something similar would work well for this PR.
Essentially, the way it works is just making the filter a virtual inventory and embedding it into the GUI instead of having a whole separate 'settings' GUI. (The blacklist/whitelist button can go in the same GUI, no need for a different one). This is easier for the player since they can see everything at once rather than having to click another button to get more detail, but also will simplify the code a lot because you don't have to deal with nested inventories which is nice :)
Another thought: It seems that according to the invui inventory docs (https://docs.xenondevs.xyz/invui/inventory/) you can actually embed a vanilla inventory into a invui one. I think it would be worth doing this so you can avoid having to add all the extra manual hopper logic.
src/main/java/io/github/pylonmc/pylon/base/content/machines/simple/VacuumHopper.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/pylonmc/pylon/base/content/machines/simple/VacuumHopper.java
Outdated
Show resolved
Hide resolved
21d76f8 to
99cf2a1
Compare
| } | ||
|
|
||
| { | ||
| var hopper = (org.bukkit.block.Hopper) getBlock().getState(); |
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.
This should be getState(false), you shouldn't be using snapshots
Fixes #490
Fixes #489