-
Notifications
You must be signed in to change notification settings - Fork 178
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
MUI cover skeleton and Machine Controller Cover port #2312
Conversation
default <T extends Enum<T>> BoolValue.Dynamic boolValueOf(EnumSyncValue<T> syncValue, T value) { | ||
return new BoolValue.Dynamic(() -> syncValue.getValue() == value, $ -> syncValue.setValue(value)); | ||
} | ||
|
||
/** | ||
* Get a BoolValue for use with toggle buttons which are "linked together," | ||
* meaning only one of them can be pressed at a time. | ||
*/ | ||
default BoolValue.Dynamic boolValueOf(IntSyncValue syncValue, int value) { | ||
return new BoolValue.Dynamic(() -> syncValue.getValue() == value, $ -> syncValue.setValue(value)); | ||
} |
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.
These should probably move to a general MUI2 utils class instead, and also be made static
.
@@ -268,6 +270,19 @@ public static class IDs { | |||
.canApplyTheme() | |||
.build(); | |||
|
|||
public static final UITexture MC_BUTTON = new UITexture.Builder() | |||
.location("modularui", "gui/widgets/mc_button.png") // todo |
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.
What needs addressing for this todo
and the one for the disabled button?
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.
Technically could be nothing, but I left the todo since its using a texture file from MUI that we might want to just copy over to GT for stability/later modification
public void writeInitialSyncData(@NotNull PacketBuffer packetBuffer) { | ||
super.writeInitialSyncData(packetBuffer); | ||
packetBuffer.writeBoolean(isInverted); | ||
packetBuffer.writeShort(controllerMode.ordinal()); |
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 can be written and read from as a byte
instead of short
.
@@ -1300,18 +1300,19 @@ cover.fluid_regulator.transfer_mode.description=§eTransfer Any§r - in this mod | |||
cover.fluid_regulator.supply_exact=Supply Exact: %s | |||
cover.fluid_regulator.keep_exact=Keep Exact: %s | |||
|
|||
cover.machine_controller.title=Machine Controller Settings | |||
cover.machine_controller.normal=Normal |
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 lang entry is used in some other covers, like the Advanced Energy Detector. So removing it causes there to be a missing lang entry for that cover.
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.
Besides the lang issue, everything looks good to me
CoverWithUI
for creating similar looking cover UIsTwo API fixes/changes:
holder.addCover()
andcover.onAttachment()
so thatonAttachment()
can set up some state that can then properly sync to the client for initial stategetStackForm()
method ontoCoverableView
. Technically breaking but who is actually implementing this interface alreadyMachine Controller Cover