From f37a56c3079ac78935069d7135c68a42d9dcc59f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 17 Feb 2025 15:01:05 +0100 Subject: [PATCH] Fix nudge operations incurring FP error from coordinate space conversions Closes https://github.com/ppy/osu/issues/31915. Reproduction of aforementioned issue requires 1280x720 resolution, which should also be a good way to confirm that this does anything. To me this is also equal-parts-bugfix, equal-parts-code-quality PR, because tell me: what on earth was this code ever doing at `ComposeBlueprintContainer` level? Nudging by one playfield-space-unit doesn't even *make sense* in something like taiko or mania. --- .../Edit/CatchSelectionHandler.cs | 62 +++++++++++++++++ .../Edit/OsuSelectionHandler.cs | 62 +++++++++++++++++ .../Components/ComposeBlueprintContainer.cs | 66 ------------------- 3 files changed, 124 insertions(+), 66 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs index a2784126ebb4..a7cd84aed50d 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; +using osu.Framework.Input.Events; using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Objects; @@ -12,6 +13,7 @@ using osu.Game.Rulesets.UI.Scrolling; using osu.Game.Screens.Edit.Compose.Components; using osuTK; +using osuTK.Input; using Direction = osu.Framework.Graphics.Direction; namespace osu.Game.Rulesets.Catch.Edit @@ -38,6 +40,13 @@ public override bool HandleMovement(MoveSelectionEvent moveEvent) return true; } + moveSelection(deltaX); + + return true; + } + + private void moveSelection(float deltaX) + { EditorBeatmap.PerformOnSelection(h => { if (!(h is CatchHitObject catchObject)) return; @@ -48,7 +57,60 @@ public override bool HandleMovement(MoveSelectionEvent moveEvent) foreach (var nested in catchObject.NestedHitObjects.OfType()) nested.OriginalX += deltaX; }); + } + + private bool nudgeMovementActive; + + protected override bool OnKeyDown(KeyDownEvent e) + { + // Until the keys below are global actions, this will prevent conflicts with "seek between sample points" + // which has a default of ctrl+shift+arrows. + if (e.ShiftPressed) + return false; + + if (e.ControlPressed) + { + switch (e.Key) + { + case Key.Left: + return nudgeSelection(-1); + + case Key.Right: + return nudgeSelection(1); + } + } + + return false; + } + + protected override void OnKeyUp(KeyUpEvent e) + { + base.OnKeyUp(e); + + if (nudgeMovementActive && !e.ControlPressed) + { + EditorBeatmap.EndChange(); + nudgeMovementActive = false; + } + } + + /// + /// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints). + /// + private bool nudgeSelection(float deltaX) + { + if (!nudgeMovementActive) + { + nudgeMovementActive = true; + EditorBeatmap.BeginChange(); + } + + var firstBlueprint = SelectedBlueprints.FirstOrDefault(); + + if (firstBlueprint == null) + return false; + moveSelection(deltaX); return true; } diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs b/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs index bac0a5e2734b..3a1ff34fb966 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs @@ -40,6 +40,8 @@ protected override void OnSelectionChanged() SelectionBox.CanReverse = EditorBeatmap.SelectedHitObjects.Count > 1 || EditorBeatmap.SelectedHitObjects.Any(s => s is Slider); } + private bool nudgeMovementActive; + protected override bool OnKeyDown(KeyDownEvent e) { if (e.Key == Key.M && e.ControlPressed && e.ShiftPressed) @@ -48,9 +50,43 @@ protected override bool OnKeyDown(KeyDownEvent e) return true; } + // Until the keys below are global actions, this will prevent conflicts with "seek between sample points" + // which has a default of ctrl+shift+arrows. + if (e.ShiftPressed) + return false; + + if (e.ControlPressed) + { + switch (e.Key) + { + case Key.Left: + return nudgeSelection(new Vector2(-1, 0)); + + case Key.Right: + return nudgeSelection(new Vector2(1, 0)); + + case Key.Up: + return nudgeSelection(new Vector2(0, -1)); + + case Key.Down: + return nudgeSelection(new Vector2(0, 1)); + } + } + return false; } + protected override void OnKeyUp(KeyUpEvent e) + { + base.OnKeyUp(e); + + if (nudgeMovementActive && !e.ControlPressed) + { + EditorBeatmap.EndChange(); + nudgeMovementActive = false; + } + } + public override bool HandleMovement(MoveSelectionEvent moveEvent) { var hitObjects = selectedMovableObjects; @@ -70,6 +106,13 @@ public override bool HandleMovement(MoveSelectionEvent moveEvent) if (hitObjects.Any(h => Precision.AlmostEquals(localDelta, -h.StackOffset))) return true; + moveObjects(hitObjects, localDelta); + + return true; + } + + private void moveObjects(OsuHitObject[] hitObjects, Vector2 localDelta) + { // this will potentially move the selection out of bounds... foreach (var h in hitObjects) h.Position += localDelta; @@ -81,7 +124,26 @@ public override bool HandleMovement(MoveSelectionEvent moveEvent) // this intentionally bypasses the editor `UpdateState()` / beatmap processor flow for performance reasons, // as the entire flow is too expensive to run on every movement. Scheduler.AddOnce(OsuBeatmapProcessor.ApplyStacking, EditorBeatmap); + } + + /// + /// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints). + /// + /// + private bool nudgeSelection(Vector2 delta) + { + if (!nudgeMovementActive) + { + nudgeMovementActive = true; + EditorBeatmap.BeginChange(); + } + + var firstBlueprint = SelectedBlueprints.FirstOrDefault(); + + if (firstBlueprint == null) + return false; + moveObjects(selectedMovableObjects, delta); return true; } diff --git a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs index e82f6395d0ac..4c57eee971b8 100644 --- a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs @@ -27,7 +27,6 @@ using osu.Game.Rulesets.UI; using osu.Game.Screens.Edit.Components.TernaryButtons; using osuTK; -using osuTK.Input; namespace osu.Game.Screens.Edit.Compose.Components { @@ -112,71 +111,6 @@ protected override void TransferBlueprintFor(HitObject hitObject, DrawableHitObj blueprint.DrawableObject = drawableObject; } - private bool nudgeMovementActive; - - protected override bool OnKeyDown(KeyDownEvent e) - { - // Until the keys below are global actions, this will prevent conflicts with "seek between sample points" - // which has a default of ctrl+shift+arrows. - if (e.ShiftPressed) - return false; - - if (e.ControlPressed) - { - switch (e.Key) - { - case Key.Left: - return nudgeSelection(new Vector2(-1, 0)); - - case Key.Right: - return nudgeSelection(new Vector2(1, 0)); - - case Key.Up: - return nudgeSelection(new Vector2(0, -1)); - - case Key.Down: - return nudgeSelection(new Vector2(0, 1)); - } - } - - return false; - } - - protected override void OnKeyUp(KeyUpEvent e) - { - base.OnKeyUp(e); - - if (nudgeMovementActive && !e.ControlPressed) - { - Beatmap.EndChange(); - nudgeMovementActive = false; - } - } - - /// - /// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints). - /// - /// - private bool nudgeSelection(Vector2 delta) - { - if (!nudgeMovementActive) - { - nudgeMovementActive = true; - Beatmap.BeginChange(); - } - - var firstBlueprint = SelectionHandler.SelectedBlueprints.FirstOrDefault(); - - if (firstBlueprint == null) - return false; - - // convert to game space coordinates - delta = firstBlueprint.ToScreenSpace(delta) - firstBlueprint.ToScreenSpace(Vector2.Zero); - - SelectionHandler.HandleMovement(new MoveSelectionEvent(firstBlueprint, delta)); - return true; - } - private void updatePlacementNewCombo() { if (CurrentHitObjectPlacement?.HitObject is IHasComboInformation c)