From 2fda45cad4f6632409a396368e472fcfc7b2e859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 25 Jun 2024 15:01:43 +0200 Subject: [PATCH 1/2] Fix crashes when opening scale/rotation popovers during selection box operations --- .../Edit/OsuSelectionRotationHandler.cs | 12 ++++++++---- .../Edit/OsuSelectionScaleHandler.cs | 12 ++++++++---- osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs | 6 ++++-- .../Compose/Components/SelectionBoxRotationHandle.cs | 3 +++ .../Compose/Components/SelectionBoxScaleHandle.cs | 3 +++ .../Compose/Components/SelectionRotationHandler.cs | 7 +++++++ .../Edit/Compose/Components/SelectionScaleHandler.cs | 7 +++++++ 7 files changed, 40 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSelectionRotationHandler.cs b/osu.Game.Rulesets.Osu/Edit/OsuSelectionRotationHandler.cs index 7624b2f27e..62a39d3702 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuSelectionRotationHandler.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuSelectionRotationHandler.cs @@ -53,9 +53,11 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Begin() { - if (objectsInRotation != null) + if (OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Begin)} a rotate operation while another is in progress!"); + base.Begin(); + changeHandler?.BeginChange(); objectsInRotation = selectedMovableObjects.ToArray(); @@ -68,10 +70,10 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Update(float rotation, Vector2? origin = null) { - if (objectsInRotation == null) + if (!OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Update)} a rotate operation without calling {nameof(Begin)} first!"); - Debug.Assert(originalPositions != null && originalPathControlPointPositions != null && defaultOrigin != null); + Debug.Assert(objectsInRotation != null && originalPositions != null && originalPathControlPointPositions != null && defaultOrigin != null); Vector2 actualOrigin = origin ?? defaultOrigin.Value; @@ -91,11 +93,13 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Commit() { - if (objectsInRotation == null) + if (!OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Commit)} a rotate operation without calling {nameof(Begin)} first!"); changeHandler?.EndChange(); + base.Commit(); + objectsInRotation = null; originalPositions = null; originalPathControlPointPositions = null; diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs b/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs index de00aa6ad3..f4fd48f183 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuSelectionScaleHandler.cs @@ -72,9 +72,11 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Begin() { - if (objectsInScale != null) + if (OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Begin)} a scale operation while another is in progress!"); + base.Begin(); + changeHandler?.BeginChange(); objectsInScale = selectedMovableObjects.ToDictionary(ho => ho, ho => new OriginalHitObjectState(ho)); @@ -86,10 +88,10 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Update(Vector2 scale, Vector2? origin = null, Axes adjustAxis = Axes.Both) { - if (objectsInScale == null) + if (!OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Update)} a scale operation without calling {nameof(Begin)} first!"); - Debug.Assert(defaultOrigin != null && OriginalSurroundingQuad != null); + Debug.Assert(objectsInScale != null && defaultOrigin != null && OriginalSurroundingQuad != null); Vector2 actualOrigin = origin ?? defaultOrigin.Value; @@ -117,11 +119,13 @@ namespace osu.Game.Rulesets.Osu.Edit public override void Commit() { - if (objectsInScale == null) + if (!OperationInProgress.Value) throw new InvalidOperationException($"Cannot {nameof(Commit)} a rotate operation without calling {nameof(Begin)} first!"); changeHandler?.EndChange(); + base.Commit(); + objectsInScale = null; OriginalSurroundingQuad = null; defaultOrigin = null; diff --git a/osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs b/osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs index 28d0f8320f..a3547d45e5 100644 --- a/osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs +++ b/osu.Game.Rulesets.Osu/Edit/TransformToolboxGroup.cs @@ -77,13 +77,15 @@ namespace osu.Game.Rulesets.Osu.Edit { case GlobalAction.EditorToggleRotateControl: { - rotateButton.TriggerClick(); + if (!RotationHandler.OperationInProgress.Value || rotateButton.Selected.Value) + rotateButton.TriggerClick(); return true; } case GlobalAction.EditorToggleScaleControl: { - scaleButton.TriggerClick(); + if (!ScaleHandler.OperationInProgress.Value || scaleButton.Selected.Value) + scaleButton.TriggerClick(); return true; } } diff --git a/osu.Game/Screens/Edit/Compose/Components/SelectionBoxRotationHandle.cs b/osu.Game/Screens/Edit/Compose/Components/SelectionBoxRotationHandle.cs index 5270162189..b9383f1bad 100644 --- a/osu.Game/Screens/Edit/Compose/Components/SelectionBoxRotationHandle.cs +++ b/osu.Game/Screens/Edit/Compose/Components/SelectionBoxRotationHandle.cs @@ -67,6 +67,9 @@ namespace osu.Game.Screens.Edit.Compose.Components if (rotationHandler == null) return false; + if (rotationHandler.OperationInProgress.Value) + return false; + rotationHandler.Begin(); return true; } diff --git a/osu.Game/Screens/Edit/Compose/Components/SelectionBoxScaleHandle.cs b/osu.Game/Screens/Edit/Compose/Components/SelectionBoxScaleHandle.cs index eca0c08ba1..3f4f2c2854 100644 --- a/osu.Game/Screens/Edit/Compose/Components/SelectionBoxScaleHandle.cs +++ b/osu.Game/Screens/Edit/Compose/Components/SelectionBoxScaleHandle.cs @@ -32,6 +32,9 @@ namespace osu.Game.Screens.Edit.Compose.Components if (scaleHandler == null) return false; + if (scaleHandler.OperationInProgress.Value) + return false; + originalAnchor = Anchor; scaleHandler.Begin(); diff --git a/osu.Game/Screens/Edit/Compose/Components/SelectionRotationHandler.cs b/osu.Game/Screens/Edit/Compose/Components/SelectionRotationHandler.cs index 787716a38c..532daaf7fa 100644 --- a/osu.Game/Screens/Edit/Compose/Components/SelectionRotationHandler.cs +++ b/osu.Game/Screens/Edit/Compose/Components/SelectionRotationHandler.cs @@ -12,6 +12,11 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public partial class SelectionRotationHandler : Component { + /// + /// Whether there is any ongoing scale operation right now. + /// + public Bindable OperationInProgress { get; private set; } = new BindableBool(); + /// /// Whether rotation anchored by the selection origin can currently be performed. /// @@ -50,6 +55,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public virtual void Begin() { + OperationInProgress.Value = true; } /// @@ -85,6 +91,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public virtual void Commit() { + OperationInProgress.Value = false; } } } diff --git a/osu.Game/Screens/Edit/Compose/Components/SelectionScaleHandler.cs b/osu.Game/Screens/Edit/Compose/Components/SelectionScaleHandler.cs index a96f627e56..c91362219c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/SelectionScaleHandler.cs +++ b/osu.Game/Screens/Edit/Compose/Components/SelectionScaleHandler.cs @@ -13,6 +13,11 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public partial class SelectionScaleHandler : Component { + /// + /// Whether there is any ongoing scale operation right now. + /// + public Bindable OperationInProgress { get; private set; } = new BindableBool(); + /// /// Whether horizontal scaling (from the left or right edge) support should be enabled. /// @@ -63,6 +68,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public virtual void Begin() { + OperationInProgress.Value = true; } /// @@ -99,6 +105,7 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public virtual void Commit() { + OperationInProgress.Value = false; } } } From d722be16e39683c59738f9c66976a9b2c92f83f3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jun 2024 23:41:43 +0900 Subject: [PATCH 2/2] Add missing `base` calls for safety --- osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs | 4 ++++ osu.Game/Overlays/SkinEditor/SkinSelectionRotationHandler.cs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs index 79a808bbd2..28763051e3 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposeSelectBox.cs @@ -84,6 +84,8 @@ namespace osu.Game.Tests.Visual.Editing targetContainer = getTargetContainer(); initialRotation = targetContainer!.Rotation; + + base.Begin(); } public override void Update(float rotation, Vector2? origin = null) @@ -102,6 +104,8 @@ namespace osu.Game.Tests.Visual.Editing targetContainer = null; initialRotation = null; + + base.Commit(); } } diff --git a/osu.Game/Overlays/SkinEditor/SkinSelectionRotationHandler.cs b/osu.Game/Overlays/SkinEditor/SkinSelectionRotationHandler.cs index 6a118a73a8..36b38543d1 100644 --- a/osu.Game/Overlays/SkinEditor/SkinSelectionRotationHandler.cs +++ b/osu.Game/Overlays/SkinEditor/SkinSelectionRotationHandler.cs @@ -61,6 +61,8 @@ namespace osu.Game.Overlays.SkinEditor originalRotations = objectsInRotation.ToDictionary(d => d, d => d.Rotation); originalPositions = objectsInRotation.ToDictionary(d => d, d => d.ToScreenSpace(d.OriginPosition)); defaultOrigin = GeometryUtils.GetSurroundingQuad(objectsInRotation.SelectMany(d => d.ScreenSpaceDrawQuad.GetVertices().ToArray())).Centre; + + base.Begin(); } public override void Update(float rotation, Vector2? origin = null) @@ -99,6 +101,8 @@ namespace osu.Game.Overlays.SkinEditor originalPositions = null; originalRotations = null; defaultOrigin = null; + + base.Commit(); } } }