Add combo colour override control to editor #31473
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes ppy#25608. Logic mostly matching stable. All operations are done on `ComboOffset` which still makes overridden combo colours weirdly relatively dependent on each other rather than them be an "absolute" choice, but alas... As per stable, two consecutive new combos can use the same colour only if they are separated by a break: https://github.com/peppy/osu-stable-reference/blob/52f3f75ed7efd7b9eb56e1e45c95bb91504337be/osu!/GameModes/Edit/Modes/EditorModeCompose.cs#L4564-L4571 This control is only available once the user has changed the combo colours from defaults; additionally, only a single new combo object must be selected for the colour selector to show up.
Is this a requirement due to how it works internally? It's a super rare case that a user hasn't changed from the defaults anyway, but just curious. |
peppy
reviewed
Jan 14, 2025
Comment on lines
53
to
70
| mainButtonContainer = new Container | ||
| { | ||
| RelativeSizeAxes = Axes.X, | ||
| AutoSizeAxes = Axes.Y, | ||
| Padding = new MarginPadding { Right = 30 }, | ||
| Child = new DrawableTernaryButton | ||
| { | ||
| Current = Current, | ||
| Description = "New combo", | ||
| CreateIcon = () => new SpriteIcon { Icon = OsuIcon.EditorNewComboA }, | ||
| }, | ||
| }, | ||
| pickerButton = new ColourPickerButton | ||
| { | ||
| Anchor = Anchor.CentreRight, | ||
| Origin = Anchor.CentreRight, | ||
| Width = 25, | ||
| ComboColours = { BindTarget = comboColours } |
There was a problem hiding this comment.
I'd probably change the default state to the one where the button is hidden, I think this is generally how we handle these cases:
diff --git a/osu.Game/Screens/Edit/Components/TernaryButtons/NewComboTernaryButton.cs b/osu.Game/Screens/Edit/Components/TernaryButtons/NewComboTernaryButton.cs
index 8c64480b43..1f95d5f239 100644
--- a/osu.Game/Screens/Edit/Components/TernaryButtons/NewComboTernaryButton.cs
+++ b/osu.Game/Screens/Edit/Components/TernaryButtons/NewComboTernaryButton.cs
@@ -54,7 +54,6 @@ private void load(EditorBeatmap editorBeatmap)
{
RelativeSizeAxes = Axes.X,
AutoSizeAxes = Axes.Y,
- Padding = new MarginPadding { Right = 30 },
Child = new DrawableTernaryButton
{
Current = Current,
@@ -66,6 +65,7 @@ private void load(EditorBeatmap editorBeatmap)
{
Anchor = Anchor.CentreRight,
Origin = Anchor.CentreRight,
+ Alpha = 0,
Width = 25,
ComboColours = { BindTarget = comboColours }
}
There was a problem hiding this comment.
Applied in 8ffd254 👍
This "combo index with offsets" stuff can only ever work for combo colour lookups on the beatmap skin, and the beatmap skin won't be present until the user actually changes combo colours. See also: #12683. |
peppy
approved these changes
Jan 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #25608.
2025-01-10.13-33-08.mp4
Logic mostly matching stable. All operations are done on
ComboOffsetwhich still makes overridden combo colours weirdly relatively dependent on each other rather than them be an "absolute" choice, but alas...As per stable, two consecutive new combos can use the same colour only if they are separated by a break.
This control is only available once the user has changed the combo colours from defaults; additionally, only a single new combo object must be selected for the colour selector to show up.