Skip to content
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

Fix rotate tool button in editor disabled when selecting 1 circle #26702

Merged
merged 18 commits into from Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e1f8bc9
Rename CanRotate property of SelectionRotationHandler to a more descr…
honguyenminh Jan 25, 2024
601ba9f
Change rotate tool button to be enabled on single circle.
honguyenminh Jan 25, 2024
500bed0
Split editor toolbox radio button disabling logic from EditorRadioBut…
honguyenminh Jan 25, 2024
94ada87
Un-hardcode tooltip from EditorRadioButton and add disabled tooltip f…
honguyenminh Jan 25, 2024
b87ff4d
Edit test for precise rotation popover
honguyenminh Jan 25, 2024
2fa52de
Fix formatting
honguyenminh Jan 25, 2024
d5b70ed
Move CanRotatePlayfieldOrigin bindable to generic rotation handler
honguyenminh Jan 25, 2024
113dbcd
Merge branch 'master' into fix-rotate-editor-button-disabled
honguyenminh Mar 30, 2024
9950395
Merge branch 'fix-rotate-editor-button-disabled' of https://github.co…
honguyenminh Mar 30, 2024
8d6358a
Fix editor rotation allowing spinner only bug
honguyenminh Mar 30, 2024
5d497ba
Simplify TooltipText for EditorRadioButton
honguyenminh Mar 30, 2024
6f78226
Fix inspection
honguyenminh Mar 30, 2024
b445e27
Aggregate two CanRotate bindable for enabling the Rotate button
honguyenminh Mar 30, 2024
86def7e
Change editor rotate button disabled tooltip text
honguyenminh Mar 31, 2024
1adba68
Merge branch 'fix-rotate-editor-button-disabled' of https://github.co…
honguyenminh Mar 31, 2024
ed5dd5c
Bind using local bindables to avoid potentially event pollution
peppy Apr 2, 2024
eca242c
Change tooltip text slightly
peppy Apr 2, 2024
6642702
Update plurals in editor rotate button tooltip
honguyenminh Apr 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 37 additions & 4 deletions osu.Game.Rulesets.Osu.Tests/Editor/TestScenePreciseRotation.cs
Expand Up @@ -24,14 +24,38 @@ public partial class TestScenePreciseRotation : TestSceneOsuEditor
[Test]
public void TestHotkeyHandling()
{
AddStep("select single circle", () => EditorBeatmap.SelectedHitObjects.Add(EditorBeatmap.HitObjects.OfType<HitCircle>().First()));
AddStep("deselect everything", () => EditorBeatmap.SelectedHitObjects.Clear());
AddStep("press rotate hotkey", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Key(Key.R);
InputManager.ReleaseKey(Key.ControlLeft);
});
AddUntilStep("no popover present", () => this.ChildrenOfType<PreciseRotationPopover>().Count(), () => Is.Zero);
AddUntilStep("no popover present", getPopover, () => Is.Null);

AddStep("select single circle",
() => EditorBeatmap.SelectedHitObjects.Add(EditorBeatmap.HitObjects.OfType<HitCircle>().First()));
AddStep("press rotate hotkey", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Key(Key.R);
InputManager.ReleaseKey(Key.ControlLeft);
});
AddUntilStep("popover present", getPopover, () => Is.Not.Null);
AddAssert("only playfield centre origin rotation available", () =>
{
var popover = getPopover();
var buttons = popover.ChildrenOfType<EditorRadioButton>();
return buttons.Any(btn => btn.Text == "Selection centre" && !btn.Enabled.Value)
&& buttons.Any(btn => btn.Text == "Playfield centre" && btn.Enabled.Value);
});
AddStep("press rotate hotkey", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Key(Key.R);
InputManager.ReleaseKey(Key.ControlLeft);
});
AddUntilStep("no popover present", getPopover, () => Is.Null);

AddStep("select first three objects", () =>
{
Expand All @@ -44,14 +68,23 @@ public void TestHotkeyHandling()
InputManager.Key(Key.R);
InputManager.ReleaseKey(Key.ControlLeft);
});
AddUntilStep("popover present", () => this.ChildrenOfType<PreciseRotationPopover>().Count(), () => Is.EqualTo(1));
AddUntilStep("popover present", getPopover, () => Is.Not.Null);
AddAssert("both origin rotation available", () =>
{
var popover = getPopover();
var buttons = popover.ChildrenOfType<EditorRadioButton>();
return buttons.Any(btn => btn.Text == "Selection centre" && btn.Enabled.Value)
&& buttons.Any(btn => btn.Text == "Playfield centre" && btn.Enabled.Value);
});
AddStep("press rotate hotkey", () =>
{
InputManager.PressKey(Key.ControlLeft);
InputManager.Key(Key.R);
InputManager.ReleaseKey(Key.ControlLeft);
});
AddUntilStep("no popover present", () => this.ChildrenOfType<PreciseRotationPopover>().Count(), () => Is.Zero);
AddUntilStep("no popover present", getPopover, () => Is.Null);

PreciseRotationPopover? getPopover() => this.ChildrenOfType<PreciseRotationPopover>().SingleOrDefault();
}

[Test]
Expand Down
3 changes: 2 additions & 1 deletion osu.Game.Rulesets.Osu/Edit/OsuSelectionRotationHandler.cs
Expand Up @@ -41,7 +41,8 @@ protected override void LoadComplete()
private void updateState()
{
var quad = GeometryUtils.GetSurroundingQuad(selectedMovableObjects);
CanRotate.Value = quad.Width > 0 || quad.Height > 0;
CanRotateSelectionOrigin.Value = quad.Width > 0 || quad.Height > 0;
CanRotatePlayfieldOrigin.Value = selectedMovableObjects.Any();
}

private OsuHitObject[]? objectsInRotation;
Expand Down
13 changes: 12 additions & 1 deletion osu.Game.Rulesets.Osu/Edit/PreciseRotationPopover.cs
Expand Up @@ -24,6 +24,8 @@ public partial class PreciseRotationPopover : OsuPopover
private SliderWithTextBoxInput<float> angleInput = null!;
private EditorRadioButtonCollection rotationOrigin = null!;

private RadioButton selectionCentreButton = null!;

public PreciseRotationPopover(SelectionRotationHandler rotationHandler)
{
this.rotationHandler = rotationHandler;
Expand Down Expand Up @@ -59,13 +61,17 @@ private void load()
new RadioButton("Playfield centre",
() => rotationInfo.Value = rotationInfo.Value with { Origin = RotationOrigin.PlayfieldCentre },
() => new SpriteIcon { Icon = FontAwesome.Regular.Square }),
new RadioButton("Selection centre",
selectionCentreButton = new RadioButton("Selection centre",
() => rotationInfo.Value = rotationInfo.Value with { Origin = RotationOrigin.SelectionCentre },
() => new SpriteIcon { Icon = FontAwesome.Solid.VectorSquare })
}
}
}
};
selectionCentreButton.Selected.DisabledChanged += isDisabled =>
{
selectionCentreButton.TooltipText = isDisabled ? "Select more than one object to perform selection-based rotation." : string.Empty;
};
}

protected override void LoadComplete()
Expand All @@ -76,6 +82,11 @@ protected override void LoadComplete()
angleInput.Current.BindValueChanged(angle => rotationInfo.Value = rotationInfo.Value with { Degrees = angle.NewValue });
rotationOrigin.Items.First().Select();

rotationHandler.CanRotateSelectionOrigin.BindValueChanged(e =>
{
selectionCentreButton.Selected.Disabled = !e.NewValue;
}, true);

rotationInfo.BindValueChanged(rotation =>
{
rotationHandler.Update(rotation.NewValue.Degrees, rotation.NewValue.Origin == RotationOrigin.PlayfieldCentre ? OsuPlayfield.BASE_SIZE / 2 : null);
Expand Down
16 changes: 15 additions & 1 deletion osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs
Expand Up @@ -22,6 +22,9 @@ public partial class TransformToolboxGroup : EditorToolboxGroup, IKeyBindingHand

private EditorToolButton rotateButton = null!;

private Bindable<bool> canRotatePlayfieldOrigin = null!;
private Bindable<bool> canRotateSelectionOrigin = null!;

public SelectionRotationHandler RotationHandler { get; init; } = null!;

public TransformToolboxGroup()
Expand Down Expand Up @@ -51,9 +54,20 @@ protected override void LoadComplete()
{
base.LoadComplete();

// aggregate two values into canRotate
canRotatePlayfieldOrigin = RotationHandler.CanRotatePlayfieldOrigin.GetBoundCopy();
canRotatePlayfieldOrigin.BindValueChanged(_ => updateCanRotateAggregate());

canRotateSelectionOrigin = RotationHandler.CanRotateSelectionOrigin.GetBoundCopy();
canRotateSelectionOrigin.BindValueChanged(_ => updateCanRotateAggregate());

void updateCanRotateAggregate()
{
canRotate.Value = RotationHandler.CanRotatePlayfieldOrigin.Value || RotationHandler.CanRotateSelectionOrigin.Value;
}

// bindings to `Enabled` on the buttons are decoupled on purpose
// due to the weird `OsuButton` behaviour of resetting `Enabled` to `false` when `Action` is set.
canRotate.BindTo(RotationHandler.CanRotate);
canRotate.BindValueChanged(_ => rotateButton.Enabled.Value = canRotate.Value, true);
}

Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs
Expand Up @@ -89,7 +89,7 @@ public TestSelectionRotationHandler(Func<Container> getTargetContainer)
{
this.getTargetContainer = getTargetContainer;

CanRotate.Value = true;
CanRotateSelectionOrigin.Value = true;
}

[CanBeNull]
Expand Down
Expand Up @@ -41,7 +41,7 @@ protected override void LoadComplete()

private void updateState()
{
CanRotate.Value = selectedItems.Count > 0;
CanRotateSelectionOrigin.Value = selectedItems.Count > 0;
}

private Drawable[]? objectsInRotation;
Expand Down
16 changes: 16 additions & 0 deletions osu.Game/Rulesets/Edit/HitObjectComposer.cs
Expand Up @@ -212,6 +212,14 @@ private void load(OsuConfigManager config)
.Select(t => new RadioButton(t.Name, () => toolSelected(t), t.CreateIcon))
.ToList();

foreach (var item in toolboxCollection.Items)
{
item.Selected.DisabledChanged += isDisabled =>
{
item.TooltipText = isDisabled ? "Add at least one timing point first!" : string.Empty;
};
}

TernaryStates = CreateTernaryButtons().ToArray();
togglesCollection.AddRange(TernaryStates.Select(b => new DrawableTernaryButton(b)));

Expand Down Expand Up @@ -244,6 +252,14 @@ protected override void LoadComplete()
if (!timing.NewValue)
setSelectTool();
});

EditorBeatmap.HasTiming.BindValueChanged(hasTiming =>
{
foreach (var item in toolboxCollection.Items)
{
item.Selected.Disabled = !hasTiming.NewValue;
}
}, true);
}

protected override void Update()
Expand Down
Expand Up @@ -33,9 +33,6 @@ public partial class EditorRadioButton : OsuButton, IHasTooltip

private Drawable icon = null!;

[Resolved]
private EditorBeatmap? editorBeatmap { get; set; }

public EditorRadioButton(RadioButton button)
{
Button = button;
Expand Down Expand Up @@ -76,8 +73,6 @@ protected override void LoadComplete()
Selected?.Invoke(Button);
};

editorBeatmap?.HasTiming.BindValueChanged(hasTiming => Button.Selected.Disabled = !hasTiming.NewValue, true);

Button.Selected.BindDisabledChanged(disabled => Enabled.Value = !disabled, true);
updateSelectionState();
}
Expand All @@ -99,6 +94,6 @@ private void updateSelectionState()
X = 40f
};

public LocalisableString TooltipText => Enabled.Value ? string.Empty : "Add at least one timing point first!";
public LocalisableString TooltipText => Button.TooltipText;
}
}
5 changes: 5 additions & 0 deletions osu.Game/Screens/Edit/Components/RadioButtons/RadioButton.cs
Expand Up @@ -4,13 +4,15 @@
using System;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Localisation;

namespace osu.Game.Screens.Edit.Components.RadioButtons
{
public class RadioButton
{
/// <summary>
/// Whether this <see cref="RadioButton"/> is selected.
/// Disable this bindable to disable the button.
/// </summary>
public readonly BindableBool Selected;

Expand Down Expand Up @@ -50,5 +52,8 @@ public void Select()
/// Deselects this <see cref="RadioButton"/>.
/// </summary>
public void Deselect() => Selected.Value = false;

// Tooltip text that will be shown when hovered over
public LocalisableString TooltipText { get; set; } = string.Empty;
}
}
2 changes: 1 addition & 1 deletion osu.Game/Screens/Edit/Compose/Components/SelectionBox.cs
Expand Up @@ -174,7 +174,7 @@ public string Text
private void load()
{
if (rotationHandler != null)
canRotate.BindTo(rotationHandler.CanRotate);
canRotate.BindTo(rotationHandler.CanRotateSelectionOrigin);

canRotate.BindValueChanged(_ => recreate(), true);
}
Expand Down
Expand Up @@ -13,9 +13,14 @@ namespace osu.Game.Screens.Edit.Compose.Components
public partial class SelectionRotationHandler : Component
{
/// <summary>
/// Whether the rotation can currently be performed.
/// Whether rotation anchored by the selection origin can currently be performed.
/// </summary>
public Bindable<bool> CanRotate { get; private set; } = new BindableBool();
public Bindable<bool> CanRotateSelectionOrigin { get; private set; } = new BindableBool();
honguyenminh marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Whether rotation anchored by the center of the playfield can currently be performed.
/// </summary>
public Bindable<bool> CanRotatePlayfieldOrigin { get; private set; } = new BindableBool();

/// <summary>
/// Performs a single, instant, atomic rotation operation.
Expand Down