-
Notifications
You must be signed in to change notification settings - Fork 5
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 long versions of ItemStack and FluidTanks #6
Conversation
will be making new ones to be like FluidSlot.
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 for late review, and thanks for your hard work.
- IMO we can merge
ItemStackLongDelegate
andItemStackLong
;ItemStack
is basically a wrapper forItem
, damage, stacksize and NBT with convenient methods, unlikeIFluidTank
. We can add a constructor forItemStackLong
which takesItemStack
. ItemSlotLong
extendsItemSlot
which implementsIVanillaSlot
. As far as I can seeItemSlotLong
needs to deal withgetVanillaSlot
, otherwise there would be some problems.- What is the purpose of style changes like https://github.com/GTNewHorizons/ModularUI2/pull/6/files?w=1#diff-916f50b2466a77c30bc767c1aee5cffa8019917c5f340dd8590a42a809b4657cL111-L112 or https://github.com/GTNewHorizons/ModularUI2/pull/6/files?w=1#diff-7dc67f518e8c28613ec602f30288f01418bf91bb56dc50fd80c46bab69ed9d9aL60 ? Especially https://github.com/GTNewHorizons/ModularUI2/pull/6/files?w=1#diff-b655806b602111761fa0f8efdb868132f586b9c9e7168d30e203daad0fee375aR97 looks to be leftover of regex formatting changes.
- Could you set
insert_final_newline
in .editorconfig to true and fix line changes? I've synced it with upstream and that setting got changed, but brachy seems to be still inserting new lines after that and I think we can just restore old behavior to please git diff. - I start thinking that
future
package could be broken down to respective modules classified by functionality, rather than having backport things in one place. It's not like your fault nor I request you to do it in this PR, but just a reminder.
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.
Nitpicks mostly from Intellij suggestions (this review is not sponsored by Jetbrains 😉)
src/main/java/com/cleanroommc/modularui/future/SlotItemHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/utils/ItemStackItemHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/value/sync/ItemSlotSH.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/value/sync/ItemSlotSH.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/api/IFluidTankLong.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/utils/item/ItemStackLong.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/value/sync/FluidSlotLongSyncHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/value/sync/FluidSlotLongSyncHandler.java
Show resolved
Hide resolved
src/main/java/com/cleanroommc/modularui/value/sync/FluidSlotLongSyncHandler.java
Outdated
Show resolved
Hide resolved
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.
🚀
TODO: