From 50aafba2fcecbb48a2c0494b02c8a853bce22eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 18 May 2026 11:18:55 +0200 Subject: [PATCH] Add slider velocity control to toolbox (#37746) https://github.com/user-attachments/assets/2a511e0d-51f8-4abf-a3ab-de0992618b6b This implements a rather opinionated UX that's designed to be a middle ground between the previous lazer behaviour of unconditionally inheriting the last slider's velocity and just a textbox that you'd need to manually fiddle with every time. As to what that means, precisely: - By default, the control follows the last slider's velocity (updates on seeks as well as changes to existing objects). - When the slider control in the toolbox is manually adjusted, the control decouples from the last slider's velocity and instead uses the last manually-specified value. - There is a button that allows the user to couple back to the last slider's velocity if they consider to have made a mistake in adjusting it. - Upon successful placement of a slider, the control reverts to following the last slider's velocity. Of note, this control *only interacts with and affects the next placed slider*. It is in no way coupled to any selected objects. This may be confusing to users but was an intentional choice to limit complexity (what if there are multiple selected objects with multiple velocities?) For adjusting existing objects you can use the green pieces on the timeline, which notably do support changing multiple selected objects at once. --- - Closes https://github.com/ppy/osu/issues/36844 - Supersedes / closes https://github.com/ppy/osu/pull/33707 --- .../Editor/TestSceneSliderVelocityAdjust.cs | 72 ++++++ .../Sliders/SliderPlacementBlueprint.cs | 12 +- .../Edit/FreehandSliderToolboxGroup.cs | 2 +- .../Edit/OsuHitObjectComposer.cs | 4 + .../Edit/OsuSliderVelocityToolboxGroup.cs | 213 ++++++++++++++++++ 5 files changed, 294 insertions(+), 9 deletions(-) create mode 100644 osu.Game.Rulesets.Osu/Edit/OsuSliderVelocityToolboxGroup.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderVelocityAdjust.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderVelocityAdjust.cs index 175cbeca6e..f38de96477 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderVelocityAdjust.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderVelocityAdjust.cs @@ -7,6 +7,9 @@ using osu.Framework.Input; using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Graphics.UserInterface; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Osu.Edit; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.UI; using osu.Game.Screens.Edit; @@ -130,5 +133,74 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor AddAssert("slider has correct velocity", () => slider!.Velocity, () => Is.EqualTo(velocityBefore)); AddAssert("slider has correct duration", () => slider!.Duration, () => Is.EqualTo(durationBefore)); } + + [Test] + public void TestVelocityToolbox() + { + ExpandableSlider velocitySlider = null!; + ExpandableButton useLastSliderButton = null!; + + AddStep("enter editor", () => Game.ScreenStack.Push(new EditorLoader())); + AddUntilStep("wait for editor load", () => editor?.ReadyForUse == true); + AddStep("retrieve controls", () => + { + var toolbox = this.ChildrenOfType().Single(); + velocitySlider = toolbox.ChildrenOfType>().Single(); + useLastSliderButton = toolbox.ChildrenOfType().Single(); + }); + + AddAssert("velocity slider at 1x", () => velocitySlider.Current.Value, () => Is.EqualTo(1)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button disabled", () => useLastSliderButton.Enabled.Value, () => Is.False); + + AddStep("seek to 5000", () => editorClock.Seek(5000)); + AddStep("set 2x velocity", () => velocitySlider.Current.Value = 2); + placeSlider(); + AddAssert("placed slider has 2x velocity", () => editorBeatmap.HitObjects.OfType().Last().SliderVelocityMultiplier, () => Is.EqualTo(2)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button enabled", () => useLastSliderButton.Enabled.Value, () => Is.True); + + AddStep("seek to 6000", () => editorClock.Seek(6000)); + placeSlider(); + AddAssert("placed slider has 2x velocity", () => editorBeatmap.HitObjects.OfType().Last().SliderVelocityMultiplier, () => Is.EqualTo(2)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button enabled", () => useLastSliderButton.Enabled.Value, () => Is.True); + + AddStep("seek to 9000", () => editorClock.Seek(9000)); + AddStep("set 3x velocity", () => velocitySlider.Current.Value = 3); + placeSlider(); + AddAssert("placed slider has 3x velocity", () => editorBeatmap.HitObjects.OfType().Last().SliderVelocityMultiplier, () => Is.EqualTo(3)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button enabled", () => useLastSliderButton.Enabled.Value, () => Is.True); + + AddStep("seek to 10000", () => editorClock.Seek(10000)); + AddStep("set 1x velocity", () => velocitySlider.Current.Value = 1); + AddStep("use last slider velocity instead", () => useLastSliderButton.TriggerClick()); + placeSlider(); + AddAssert("placed slider has 3x velocity", () => editorBeatmap.HitObjects.OfType().Last().SliderVelocityMultiplier, () => Is.EqualTo(3)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button disabled", () => useLastSliderButton.Enabled.Value, () => Is.False); + + AddStep("seek back to 7000", () => editorClock.Seek(7000)); + placeSlider(); + AddAssert("placed slider has 2x velocity", () => editorBeatmap.HitObjects.OfType().ElementAt(2).SliderVelocityMultiplier, () => Is.EqualTo(2)); + AddStep("expand right toolbox", () => InputManager.MoveMouseTo(this.ChildrenOfType().Last())); + AddUntilStep("wait for expand", () => useLastSliderButton.Expanded.Value, () => Is.True); + AddAssert("use last slider button disabled", () => useLastSliderButton.Enabled.Value, () => Is.False); + + void placeSlider() + { + AddStep("enter slider placement mode", () => InputManager.Key(Key.Number3)); + AddStep("move mouse to top left", () => InputManager.MoveMouseTo(editor.ChildrenOfType().First().ScreenSpaceDrawQuad.TopLeft + new Vector2(50))); + AddStep("start placement", () => InputManager.Click(MouseButton.Left)); + AddStep("move mouse to bottom right", () => InputManager.MoveMouseTo(editor.ChildrenOfType().First().ScreenSpaceDrawQuad.BottomRight - new Vector2(50))); + AddStep("end placement", () => InputManager.Click(MouseButton.Right)); + } + } } } diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs index b5b4c8c87d..2f7f1d1c09 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs @@ -54,6 +54,9 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders [Resolved] private EditorClock? editorClock { get; set; } + [Resolved] + private OsuSliderVelocityToolboxGroup? sliderVelocityToolbox { get; set; } + private Bindable limitedDistanceSnap { get; set; } = null!; private readonly IncrementalBSplineBuilder bSplineBuilder = new IncrementalBSplineBuilder { Degree = 4 }; @@ -111,9 +114,6 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders } } - [Resolved] - private EditorBeatmap editorBeatmap { get; set; } = null!; - public override SnapResult UpdateTimeAndPosition(Vector2 screenSpacePosition, double fallbackTime) { var result = composer?.TrySnapToNearbyObjects(screenSpacePosition, fallbackTime); @@ -129,11 +129,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders case SliderPlacementState.Initial: BeginPlacement(); - double? nearestSliderVelocity = (editorBeatmap - .HitObjects - .LastOrDefault(h => h is Slider && h.GetEndTime() < HitObject.StartTime) as Slider)?.SliderVelocityMultiplier; - - HitObject.SliderVelocityMultiplier = nearestSliderVelocity ?? 1; + HitObject.SliderVelocityMultiplier = sliderVelocityToolbox?.SliderVelocity.Value ?? 1; HitObject.Position = ToLocalSpace(result.ScreenSpacePosition); // Replacing the DifficultyControlPoint above doesn't trigger any kind of invalidation. diff --git a/osu.Game.Rulesets.Osu/Edit/FreehandSliderToolboxGroup.cs b/osu.Game.Rulesets.Osu/Edit/FreehandSliderToolboxGroup.cs index 6f8c58e1e4..fcc1ec829e 100644 --- a/osu.Game.Rulesets.Osu/Edit/FreehandSliderToolboxGroup.cs +++ b/osu.Game.Rulesets.Osu/Edit/FreehandSliderToolboxGroup.cs @@ -15,7 +15,7 @@ namespace osu.Game.Rulesets.Osu.Edit public partial class FreehandSliderToolboxGroup : EditorToolboxGroup { public FreehandSliderToolboxGroup() - : base("slider") + : base("freehand") { } diff --git a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs index 6ff762b82f..c0b5803ac9 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs @@ -75,6 +75,9 @@ namespace osu.Game.Rulesets.Osu.Edit [Cached(typeof(IDistanceSnapProvider))] public readonly OsuDistanceSnapProvider DistanceSnapProvider = new OsuDistanceSnapProvider(); + [Cached] + private readonly OsuSliderVelocityToolboxGroup sliderVelocityToolboxGroup = new OsuSliderVelocityToolboxGroup(); + [Cached] protected readonly OsuGridToolboxGroup OsuGridToolboxGroup = new OsuGridToolboxGroup(); @@ -111,6 +114,7 @@ namespace osu.Game.Rulesets.Osu.Edit RightToolbox.AddRange(new Drawable[] { + sliderVelocityToolboxGroup, OsuGridToolboxGroup, new TransformToolboxGroup { diff --git a/osu.Game.Rulesets.Osu/Edit/OsuSliderVelocityToolboxGroup.cs b/osu.Game.Rulesets.Osu/Edit/OsuSliderVelocityToolboxGroup.cs new file mode 100644 index 0000000000..c8e5bc13d5 --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/OsuSliderVelocityToolboxGroup.cs @@ -0,0 +1,213 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Caching; +using osu.Framework.Extensions.LocalisationExtensions; +using osu.Framework.Extensions.ObjectExtensions; +using osu.Framework.Graphics; +using osu.Framework.Localisation; +using osu.Game.Graphics.UserInterface; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Screens.Edit; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Edit +{ + public partial class OsuSliderVelocityToolboxGroup : EditorToolboxGroup + { + /// + /// Whether the last slider's velocity should be used (if available). + /// + private bool useLastSliderVelocity; + + /// + /// The slider velocity to be used for new object placements. + /// + public IBindable SliderVelocity => sliderVelocity; + + private readonly BindableDouble sliderVelocity = new BindableDouble(1) + { + Precision = 0.01, + MinValue = 0.1, + MaxValue = 10, + }; + + private ExpandableSlider slider = null!; + private ExpandableButton useLastSliderButton = null!; + + [Resolved] + private EditorBeatmap editorBeatmap { get; set; } = null!; + + [Resolved] + private EditorClock editorClock { get; set; } = null!; + + private bool syncingBindables; + private double lastClockPosition = double.NegativeInfinity; + private readonly Cached sliderVelocitySourceObject = new Cached(); + + public OsuSliderVelocityToolboxGroup() + : base("velocity") + { + } + + [BackgroundDependencyLoader] + private void load() + { + Spacing = new Vector2(5); + Children = new Drawable[] + { + slider = new ExpandableSlider + { + ExpandedLabelText = "Slider velocity", + Current = new BindableDouble(1) + { + Precision = 0.01, + MinValue = 0.1, + MaxValue = 10, + }, + KeyboardStep = 0.1f, + }, + useLastSliderButton = new ExpandableButton + { + RelativeSizeAxes = Axes.X, + Action = () => + { + useLastSliderVelocity = true; + sliderVelocitySourceObject.Invalidate(); + }, + } + }; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + // set unconditionally to true initially. + // if there is no object available to get the slider velocity from, the code in `Update()` will handle that. + useLastSliderVelocity = true; + + sliderVelocity.BindValueChanged(_ => updateSliderFromVelocity(), true); + slider.Current.BindValueChanged(_ => + { + updateVelocityFromSlider(); + updateContractedText(); + }); + updateContractedText(); + useLastSliderButton.Expanded.BindValueChanged(_ => sliderVelocitySourceObject.Invalidate()); + + editorBeatmap.HitObjectAdded += invalidateSliderVelocitySourceObject; + editorBeatmap.HitObjectUpdated += invalidateSliderVelocitySourceObject; + editorBeatmap.HitObjectRemoved += invalidateSliderVelocitySourceObject; + } + + private void updateContractedText() + { + slider.ContractedLabelText = LocalisableString.Interpolate($@"SV: {slider.Current.Value.ToLocalisableString("N2")}x"); + } + + /// + /// Updates the displayed value of this toolbox's slider from a change to + /// (which is the source-of-truth used for new object placements). + /// This is only relevant when is true, + /// in which case this code is responsible for propagating the velocity from to the slider. + /// + private void updateSliderFromVelocity() + { + if (syncingBindables) + return; + + if (!useLastSliderVelocity) + return; + + syncingBindables = true; + slider.Current.Value = sliderVelocity.Value; + syncingBindables = false; + } + + /// + /// Updates the value of from a change to the slider's state. + /// This change is assumed to be user-provoked, and therefore is switched unconditionally off + /// as the presumed intent is to override the velocity from . + /// + private void updateVelocityFromSlider() + { + if (syncingBindables) + return; + + syncingBindables = true; + useLastSliderVelocity = false; + sliderVelocity.Value = slider.Current.Value; + syncingBindables = false; + sliderVelocitySourceObject.Invalidate(); + } + + private void invalidateSliderVelocitySourceObject(HitObject _) => sliderVelocitySourceObject.Invalidate(); + + protected override void Update() + { + base.Update(); + + if (editorClock.CurrentTime != lastClockPosition) + { + sliderVelocitySourceObject.Invalidate(); + lastClockPosition = editorClock.CurrentTime; + } + + // Three possible causes of invalidation: + // - The user seeked the clock, which means a different velocity source object needs to be used. + // - Some change to the beatmap was made, which means the previously-used velocity source object may no longer be the most relevant one. + // - The user is interacting with the toolbox in a way that requires a visual state update + // (hovered to expand it, clicked the button to use last slider's velocity, or dragged the manual velocity slider). + // This is a procedural one, because `sliderVelocitySourceObject` will have been pointing at the correct object already, + // but to decrease unnecessary work being done every frame, the invalidation is explicitly re-triggered to update the toolbox state. + if (!sliderVelocitySourceObject.IsValid) + { + var lastSlider = getLastSlider(); + sliderVelocitySourceObject.Value = lastSlider; + + if (lastSlider == null) + { + useLastSliderButton.Enabled.Value = false; + useLastSliderButton.ExpandedLabelText = "No sliders to get velocity from"; + useLastSliderButton.ContractedLabelText = default; + } + else + { + useLastSliderButton.Enabled.Value = useLastSliderButton.Expanded.Value && !useLastSliderVelocity; + useLastSliderButton.ExpandedLabelText = useLastSliderVelocity + ? "Using last slider's velocity" + : LocalisableString.Interpolate($@"Use last slider's velocity ({lastSlider.SliderVelocityMultiplier.ToLocalisableString("N2")}x)"); + useLastSliderButton.ContractedLabelText = $@"current {lastSlider.SliderVelocityMultiplier.ToLocalisableString("N2")}x"; + if (useLastSliderVelocity) + sliderVelocity.Value = lastSlider.SliderVelocityMultiplier; + } + } + } + + private Slider? getLastSlider() + { + return editorBeatmap + .HitObjects + .OfType() + .LastOrDefault(h => h.StartTime <= editorClock.CurrentTime); + } + + protected override void Dispose(bool isDisposing) + { + if (editorBeatmap.IsNotNull()) + { + editorBeatmap.HitObjectAdded -= invalidateSliderVelocitySourceObject; + editorBeatmap.HitObjectUpdated -= invalidateSliderVelocitySourceObject; + editorBeatmap.HitObjectRemoved -= invalidateSliderVelocitySourceObject; + } + + base.Dispose(isDisposing); + } + } +}