feat: allow selecting difficulty in the gameplay menu - #1471
Conversation
AlmasB
left a comment
There was a problem hiding this comment.
Thanks, it looks good. Please can you take a look at the comments
| difficultyBox.styleClass.add("fxgl-difficulty-choice-box") | ||
|
|
||
| difficultyBox.value = getSettings().gameDifficulty | ||
| getSettings().gameDifficultyProperty().bindBidirectional(difficultyBox.valueProperty()) |
There was a problem hiding this comment.
Should this be the other way around? I'm thinking of a case where the user sets the difficulty in the main menu, then opens the game menu. At this point does the above mean the settings difficulty will rebound to the UI box's value property?
There was a problem hiding this comment.
I believe it is in the correct order. For example, if the user sets the difficulty in the main menu, that will update getSettings().gameDifficulty. Then later, the game menu creates its own difficultyBox and executes difficultyBox.value = getSettings().gameDifficulty ->bindBidirectional(difficultyBox.valueProperty(), the box will show the same difficulty that was set in the main menu, any changes in either place after that will update the other.
I havent' tested the reverse order, but if the value was bound first and then assigned to one side, the assigned value should still propagate to the other menu because of the bidirectional binding.
There was a problem hiding this comment.
Hmm, 2 questions:
-
is
createDifficultyMenu()rebuilt on each menu creation or is it built only once and cached? For example, main menu -> game menu -> main menu (at this last step do we still have the samedifficultyBoxthat was created the first time main menu was constructed?) -
can one property be bidirectionally bound to multiple objects?
If the answer to question 1 is "no, the menu items are cached rather than rebuilt" and if the answer to question 2 is also "no", then I can see a potential issue:
- User opens main menu settings difficulty is bound to main menu box
- User opens game menu settings difficult is bound to game menu box, losing the binding to main menu box. If we change the difficulty here, then at the next step does the difficulty in main menu also change?
- User opens main menu again...
Are you able to test this scenario and see if it works as expected?
There was a problem hiding this comment.
To answer question 1: The createDifficultyMenu() is rebuilt on each menu creation. The bound difficulty value persists as that setting lives outside of the UI controls (settings.kt).
As for question 2: As far as I'm aware the answer is no, but I have just tested it and these were the results:
- Difficulty is set to 'nightmare' in main menu
- New game is started, opening game menu shows that the difficulty is still set to 'nightmare'. Difficulty was then changed to 'easy'.
- Upon leaving the game and returning back to the main menu the difficulty is still set to 'easy'.
Perhaps this is due to the two seperate bidirectional bindings from both the main menu and game menu to the same difficulty property in the settings, allowing the difficulty to persist between each menu type.
There was a problem hiding this comment.
Sounds good, thanks for testing.
I've looked up documentation of Property. Turns out we can have multiple bidirectional bindings. There is a note about weak listeners and garbage collection, but I don't expect any issues if the menu is rebuilt every time.
| getSettings().gameDifficultyProperty().bindBidirectional(difficultyBox.valueProperty()) | ||
|
|
||
| difficultyBox.valueProperty().addListener { _, _, _ -> | ||
| switchMenuContentTo(EMPTY) |
There was a problem hiding this comment.
Could you remind me what this does / why this is needed?
There was a problem hiding this comment.
Yes, in hindsight I should've left this in a comment.
Line 367 binds the difficulty option in the UI to the built in difficulty settings. So that when the difficulty is changed through the menu, it changes it in the settings.
The rest of the code adds a listener that reacts whenever the difficulty selection changes, hiding the dropdown menu and returning the user to the previous menu. Which was the alternative to adding a seperate back button somewhere else in the menu, increasing visual clutter. I tested adding a back button but it made more sense to me to keep it so when the difficulty is changed (or in the event of not changing, clicking the already-selected difficulty) returns the user back to the previous menu.
There was a problem hiding this comment.
Or we could just leave the user in the same view, i.e. the difficulty menu remains where it is and the user can select the difficult value multiple times. Will this cause an issue do you think?
There was a problem hiding this comment.
That's fine, it won't cause an issue - it was that way initially during testing. How would you suggest leaving the difficulty menu after the changes have been made? Perhaps when another menu item (options, extras..) is selected, it hides the difficulty choicebox? As opposed to having a back button or something of that sort.
There was a problem hiding this comment.
Let's keep it as is for now. We can always action this separately if users raise an issue.
AlmasB
left a comment
There was a problem hiding this comment.
All looks good, thanks for this PR!
Closes #1449
New menu for selecting difficulty in both the main menu and the in-game menu.
Added specific CSS styling to fix the issue of the difficulty choice-box not accepting the general style.
Updated
MenuSample.javato showcase the new difficulty menu.Uses existing
gameDifficultyPropertyfrom theReadOnlyGameSettingsclass insettings.kt.