-
Notifications
You must be signed in to change notification settings - Fork 121
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
[3222] Disabling sliders when set to random #3268
[3222] Disabling sliders when set to random #3268
Conversation
src/main/java/com/faforever/client/game/GenerateMapController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/faforever/client/game/GenerateMapController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/faforever/client/game/GenerateMapController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/faforever/client/game/GenerateMapController.java
Outdated
Show resolved
Hide resolved
private BooleanBinding customizationAllowed; | ||
|
||
@Override | ||
protected void onInitialize() { | ||
customizationAllowed = previousMapName.textProperty().isNotEmpty() | ||
.or(generationTypeComboBox.valueProperty() | ||
.isNotEqualTo(GenerationType.CASUAL)) | ||
.or(commandLineArgsText.textProperty().isNotEmpty()); | ||
|
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 is better as a private final BooleanProperty that then gets bound in onInitialize, also as is wouldn't the variable be better named noCustomizatonAllowed?
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.
Ehm, not sure if I get you on the first part. if I make property final, it won't go as it will fall into category to be autowired (there is no bean to be autowire) additionally, it depends on previousMapName textField. So therefore no way to make it final. Alternative is to make it local and then pass it as param. Or am I thinking about something else? Is there Object BooleanBinding that can be specified at later time?
hmm, yeah or forbidCustomization / disableCustomization? In general, I'm lost in these logical operators and then their binding to field/property....
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.
The property wouldn't be auto wired because we should initialize it at declaration so just 'private final BooleanProperty disableCustomization = new Simple BooleanProperty();'
And then in onInit we would just disableCustomization.bind(...).
This way the property itself is final and we don't have to worry about it being reassigned. And it fits the pattern in most of the other controllers
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.
Ouch, forgot about SimpleBooleanProperty ... jobs done.
Anyway, are these all Property() classes used nowadays in projects? I've never came upon them till now, though I'm working only on web-applications ...
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.
They are used pretty heavily in JavaFX and are specific to it. But JavaFX is a desktop application library so if you have mainly done web application development you probably don't see them very often.
But mainly they are JavaFX's general solution to reactivity and event listeners. Somewhat like promises but with change detection and the like that you expect from fields.
Did some code clean-up, though naming may not be ideal - any suggestions welcomed.
added 3 tests.
Feature:
If propCombo set to Random -> slider disabled
If resourceCombo set to Random -> slider disabled
Solving ticket #3222