-
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.
| public static final UpdateReason HOPPER = new UpdateReason() {}; | ||
| public static final UpdateReason VACUUM = new UpdateReason() {}; |
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 can use MachineUpdateReason instead of these 2 now
| public VacuumHopper(@NotNull Block block, @NotNull BlockCreateContext context) { | ||
| super(block, context); | ||
|
|
||
| this.inventory = new VirtualInventory(5); |
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 don't need to manually store inventories when using PylonGuiBlock
Fixes #490
Fixes #489