Skip to content

Commit

Permalink
Fix nudge operations incurring FP error from coordinate space convers…
Browse files Browse the repository at this point in the history 
…ions

Closes #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.
  • Loading branch information
@bdach
bdach committed Feb 17, 2025
1 parent eaf3679 commit f37a56c
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 66 deletions.
62 changes: 62 additions & 0 deletions osu.Game.Rulesets.Catch/Edit/CatchSelectionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
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;
using osu.Game.Rulesets.UI;
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
Expand All @@ -38,6 +40,13 @@ public override bool HandleMovement(MoveSelectionEvent<HitObject> moveEvent)
return true;
}

moveSelection(deltaX);

return true;
}

private void moveSelection(float deltaX)
{
EditorBeatmap.PerformOnSelection(h =>
{
if (!(h is CatchHitObject catchObject)) return;
Expand All @@ -48,7 +57,60 @@ public override bool HandleMovement(MoveSelectionEvent<HitObject> moveEvent)
foreach (var nested in catchObject.NestedHitObjects.OfType<CatchHitObject>())
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;
}
}

/// <summary>
/// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints).
/// </summary>
private bool nudgeSelection(float deltaX)
{
if (!nudgeMovementActive)
{
nudgeMovementActive = true;
EditorBeatmap.BeginChange();
}

var firstBlueprint = SelectedBlueprints.FirstOrDefault();

if (firstBlueprint == null)
return false;

moveSelection(deltaX);
return true;
}

Expand Down
62 changes: 62 additions & 0 deletions osu.Game.Rulesets.Osu/Edit/OsuSelectionHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<HitObject> moveEvent)
{
var hitObjects = selectedMovableObjects;
Expand All @@ -70,6 +106,13 @@ public override bool HandleMovement(MoveSelectionEvent<HitObject> 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;
Expand All @@ -81,7 +124,26 @@ public override bool HandleMovement(MoveSelectionEvent<HitObject> 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);
}

/// <summary>
/// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints).
/// </summary>
/// <param name="delta"></param>
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;
}

Expand Down
66 changes: 0 additions & 66 deletions osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
}
}

/// <summary>
/// Move the current selection spatially by the specified delta, in gamefield coordinates (ie. the same coordinates as the blueprints).
/// </summary>
/// <param name="delta"></param>
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<HitObject>(firstBlueprint, delta));
return true;
}

private void updatePlacementNewCombo()
{
if (CurrentHitObjectPlacement?.HitObject is IHasComboInformation c)
Expand Down

0 comments on commit f37a56c

Please sign in to comment.