Merge pull request #15608 from bdach/sample-point-multiple

Allow adjusting sample volume & bank of multiple objects at once

osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs

@@ -0,0 +1,252 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System.Linq;
+using Humanizer;
+using NUnit.Framework;
+using osu.Framework.Testing;
+using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.ControlPoints;
+using osu.Game.Graphics.UserInterface;
+using osu.Game.Graphics.UserInterfaceV2;
+using osu.Game.Rulesets;
+using osu.Game.Rulesets.Osu;
+using osu.Game.Rulesets.Osu.Objects;
+using osu.Game.Rulesets.Osu.UI;
+using osu.Game.Screens.Edit.Compose.Components.Timeline;
+using osu.Game.Screens.Edit.Timing;
+using osu.Game.Tests.Beatmaps;
+using osuTK;
+using osuTK.Input;
+
+namespace osu.Game.Tests.Visual.Editing
+{
+    public class TestSceneHitObjectSamplePointAdjustments : EditorTestScene
+    {
+        protected override Ruleset CreateEditorRuleset() => new OsuRuleset();
+
+        protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false);
+
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+
+            AddStep("add test objects", () =>
+            {
+                EditorBeatmap.Add(new HitCircle
+                {
+                    StartTime = 0,
+                    Position = (OsuPlayfield.BASE_SIZE - new Vector2(100, 0)) / 2,
+                    SampleControlPoint = new SampleControlPoint
+                    {
+                        SampleBank = "normal",
+                        SampleVolume = 80
+                    }
+                });
+
+                EditorBeatmap.Add(new HitCircle
+                {
+                    StartTime = 500,
+                    Position = (OsuPlayfield.BASE_SIZE + new Vector2(100, 0)) / 2,
+                    SampleControlPoint = new SampleControlPoint
+                    {
+                        SampleBank = "soft",
+                        SampleVolume = 60
+                    }
+                });
+            });
+        }
+
+        [Test]
+        public void TestSingleSelection()
+        {
+            clickSamplePiece(0);
+            samplePopoverHasSingleBank("normal");
+            samplePopoverHasSingleVolume(80);
+
+            dismissPopover();
+
+            // select first object to ensure that sample pieces for unselected objects
+            // work independently from selection state.
+            AddStep("select first object", () => EditorBeatmap.SelectedHitObjects.Add(EditorBeatmap.HitObjects.First()));
+
+            clickSamplePiece(1);
+            samplePopoverHasSingleBank("soft");
+            samplePopoverHasSingleVolume(60);
+
+            setVolumeViaPopover(90);
+            hitObjectHasSampleVolume(1, 90);
+
+            setBankViaPopover("drum");
+            hitObjectHasSampleBank(1, "drum");
+        }
+
+        [Test]
+        public void TestMultipleSelectionWithSameSampleVolume()
+        {
+            AddStep("unify sample volume", () =>
+            {
+                foreach (var h in EditorBeatmap.HitObjects)
+                    h.SampleControlPoint.SampleVolume = 50;
+            });
+
+            AddStep("select both objects", () => EditorBeatmap.SelectedHitObjects.AddRange(EditorBeatmap.HitObjects));
+            clickSamplePiece(0);
+            samplePopoverHasSingleVolume(50);
+
+            dismissPopover();
+
+            clickSamplePiece(1);
+            samplePopoverHasSingleVolume(50);
+
+            setVolumeViaPopover(75);
+            hitObjectHasSampleVolume(0, 75);
+            hitObjectHasSampleVolume(1, 75);
+        }
+
+        [Test]
+        public void TestMultipleSelectionWithDifferentSampleVolume()
+        {
+            AddStep("select both objects", () => EditorBeatmap.SelectedHitObjects.AddRange(EditorBeatmap.HitObjects));
+            clickSamplePiece(0);
+            samplePopoverHasIndeterminateVolume();
+
+            dismissPopover();
+
+            clickSamplePiece(1);
+            samplePopoverHasIndeterminateVolume();
+
+            setVolumeViaPopover(30);
+            hitObjectHasSampleVolume(0, 30);
+            hitObjectHasSampleVolume(1, 30);
+        }
+
+        [Test]
+        public void TestMultipleSelectionWithSameSampleBank()
+        {
+            AddStep("unify sample bank", () =>
+            {
+                foreach (var h in EditorBeatmap.HitObjects)
+                    h.SampleControlPoint.SampleBank = "soft";
+            });
+
+            AddStep("select both objects", () => EditorBeatmap.SelectedHitObjects.AddRange(EditorBeatmap.HitObjects));
+            clickSamplePiece(0);
+            samplePopoverHasSingleBank("soft");
+
+            dismissPopover();
+
+            clickSamplePiece(1);
+            samplePopoverHasSingleBank("soft");
+
+            setBankViaPopover(string.Empty);
+            hitObjectHasSampleBank(0, "soft");
+            hitObjectHasSampleBank(1, "soft");
+            samplePopoverHasSingleBank("soft");
+
+            setBankViaPopover("drum");
+            hitObjectHasSampleBank(0, "drum");
+            hitObjectHasSampleBank(1, "drum");
+            samplePopoverHasSingleBank("drum");
+        }
+
+        [Test]
+        public void TestMultipleSelectionWithDifferentSampleBank()
+        {
+            AddStep("select both objects", () => EditorBeatmap.SelectedHitObjects.AddRange(EditorBeatmap.HitObjects));
+            clickSamplePiece(0);
+            samplePopoverHasIndeterminateBank();
+
+            dismissPopover();
+
+            clickSamplePiece(1);
+            samplePopoverHasIndeterminateBank();
+
+            setBankViaPopover(string.Empty);
+            hitObjectHasSampleBank(0, "normal");
+            hitObjectHasSampleBank(1, "soft");
+            samplePopoverHasIndeterminateBank();
+
+            setBankViaPopover("normal");
+            hitObjectHasSampleBank(0, "normal");
+            hitObjectHasSampleBank(1, "normal");
+            samplePopoverHasSingleBank("normal");
+        }
+
+        private void clickSamplePiece(int objectIndex) => AddStep($"click {objectIndex.ToOrdinalWords()} difficulty piece", () =>
+        {
+            var difficultyPiece = this.ChildrenOfType<SamplePointPiece>().Single(piece => piece.HitObject == EditorBeatmap.HitObjects.ElementAt(objectIndex));
+
+            InputManager.MoveMouseTo(difficultyPiece);
+            InputManager.Click(MouseButton.Left);
+        });
+
+        private void samplePopoverHasSingleVolume(int volume) => AddUntilStep($"sample popover has volume {volume}", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().SingleOrDefault();
+            var slider = popover?.ChildrenOfType<IndeterminateSliderWithTextBoxInput<int>>().Single();
+
+            return slider?.Current.Value == volume;
+        });
+
+        private void samplePopoverHasIndeterminateVolume() => AddUntilStep("sample popover has indeterminate volume", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().SingleOrDefault();
+            var slider = popover?.ChildrenOfType<IndeterminateSliderWithTextBoxInput<int>>().Single();
+
+            return slider != null && slider.Current.Value == null;
+        });
+
+        private void samplePopoverHasSingleBank(string bank) => AddUntilStep($"sample popover has bank {bank}", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().SingleOrDefault();
+            var textBox = popover?.ChildrenOfType<OsuTextBox>().First();
+
+            return textBox?.Current.Value == bank && string.IsNullOrEmpty(textBox?.PlaceholderText.ToString());
+        });
+
+        private void samplePopoverHasIndeterminateBank() => AddUntilStep("sample popover has indeterminate bank", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().SingleOrDefault();
+            var textBox = popover?.ChildrenOfType<OsuTextBox>().First();
+
+            return textBox != null && string.IsNullOrEmpty(textBox.Current.Value) && !string.IsNullOrEmpty(textBox.PlaceholderText.ToString());
+        });
+
+        private void dismissPopover()
+        {
+            AddStep("dismiss popover", () => InputManager.Key(Key.Escape));
+            AddUntilStep("wait for dismiss", () => !this.ChildrenOfType<DifficultyPointPiece.DifficultyEditPopover>().Any(popover => popover.IsPresent));
+        }
+
+        private void setVolumeViaPopover(int volume) => AddStep($"set volume {volume} via popover", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().Single();
+            var slider = popover.ChildrenOfType<IndeterminateSliderWithTextBoxInput<int>>().Single();
+            slider.Current.Value = volume;
+        });
+
+        private void hitObjectHasSampleVolume(int objectIndex, int volume) => AddAssert($"{objectIndex.ToOrdinalWords()} has volume {volume}", () =>
+        {
+            var h = EditorBeatmap.HitObjects.ElementAt(objectIndex);
+            return h.SampleControlPoint.SampleVolume == volume;
+        });
+
+        private void setBankViaPopover(string bank) => AddStep($"set bank {bank} via popover", () =>
+        {
+            var popover = this.ChildrenOfType<SamplePointPiece.SampleEditPopover>().Single();
+            var textBox = popover.ChildrenOfType<LabelledTextBox>().First();
+            textBox.Current.Value = bank;
+            // force a commit via keyboard.
+            // this is needed when testing attempting to set empty bank - which should revert to the previous value, but only on commit.
+            InputManager.ChangeFocus(textBox);
+            InputManager.Key(Key.Enter);
+        });
+
+        private void hitObjectHasSampleBank(int objectIndex, string bank) => AddAssert($"{objectIndex.ToOrdinalWords()} has bank {bank}", () =>
+        {
+            var h = EditorBeatmap.HitObjects.ElementAt(objectIndex);
+            return h.SampleControlPoint.SampleBank == bank;
+        });
+    }
+}

osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs

@@ -1,9 +1,14 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+#nullable enable
+
+using System.Collections.Generic;
+using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Extensions;
+using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Cursor;
@@ -14,12 +19,13 @@ using osu.Game.Graphics;
 using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Screens.Edit.Timing;
+using osuTK;
 
 namespace osu.Game.Screens.Edit.Compose.Components.Timeline
 {
     public class SamplePointPiece : HitObjectPointPiece, IHasPopover
     {
-        private readonly HitObject hitObject;
+        public readonly HitObject HitObject;
 
         private readonly Bindable<string> bank;
         private readonly BindableNumber<int> volume;
@@ -27,7 +33,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
         public SamplePointPiece(HitObject hitObject)
             : base(hitObject.SampleControlPoint)
         {
-            this.hitObject = hitObject;
+            HitObject = hitObject;
             volume = hitObject.SampleControlPoint.SampleVolumeBindable.GetBoundCopy();
             bank = hitObject.SampleControlPoint.SampleBankBindable.GetBoundCopy();
         }
@@ -50,23 +56,21 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
             Label.Text = $"{bank.Value} {volume.Value}";
         }
 
-        public Popover GetPopover() => new SampleEditPopover(hitObject);
+        public Popover GetPopover() => new SampleEditPopover(HitObject);
 
         public class SampleEditPopover : OsuPopover
         {
             private readonly HitObject hitObject;
-            private readonly SampleControlPoint point;
 
-            private LabelledTextBox bank;
-            private SliderWithTextBoxInput<int> volume;
+            private LabelledTextBox bank = null!;
+            private IndeterminateSliderWithTextBoxInput<int> volume = null!;
 
             [Resolved(canBeNull: true)]
-            private EditorBeatmap beatmap { get; set; }
+            private EditorBeatmap beatmap { get; set; } = null!;
 
             public SampleEditPopover(HitObject hitObject)
             {
                 this.hitObject = hitObject;
-                point = hitObject.SampleControlPoint;
             }
 
             [BackgroundDependencyLoader]
@@ -79,25 +83,84 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline
                         Width = 200,
                         Direction = FillDirection.Vertical,
                         AutoSizeAxes = Axes.Y,
+                        Spacing = new Vector2(0, 10),
                         Children = new Drawable[]
                         {
                             bank = new LabelledTextBox
                             {
                                 Label = "Bank Name",
                             },
-                            volume = new SliderWithTextBoxInput<int>("Volume")
-                            {
-                                Current = new SampleControlPoint().SampleVolumeBindable,
-                            }
+                            volume = new IndeterminateSliderWithTextBoxInput<int>("Volume", new SampleControlPoint().SampleVolumeBindable)
                         }
                     }
                 };
 
-                bank.Current = point.SampleBankBindable;
-                bank.Current.BindValueChanged(_ => beatmap.Update(hitObject));
+                // if the piece belongs to a currently selected object, assume that the user wants to change all selected objects.
+                // if the piece belongs to an unselected object, operate on that object alone, independently of the selection.
+                var relevantObjects = (beatmap.SelectedHitObjects.Contains(hitObject) ? beatmap.SelectedHitObjects : hitObject.Yield()).ToArray();
+                var relevantControlPoints = relevantObjects.Select(h => h.SampleControlPoint).ToArray();
 
-                volume.Current = point.SampleVolumeBindable;
-                volume.Current.BindValueChanged(_ => beatmap.Update(hitObject));
+                // even if there are multiple objects selected, we can still display sample volume or bank if they all have the same value.
+                string? commonBank = getCommonBank(relevantControlPoints);
+                if (!string.IsNullOrEmpty(commonBank))
+                    bank.Current.Value = commonBank;
+
+                int? commonVolume = getCommonVolume(relevantControlPoints);
+                if (commonVolume != null)
+                    volume.Current.Value = commonVolume.Value;
+
+                updateBankPlaceholderText(relevantObjects);
+                bank.Current.BindValueChanged(val =>
+                {
+                    updateBankFor(relevantObjects, val.NewValue);
+                    updateBankPlaceholderText(relevantObjects);
+                });
+                // on commit, ensure that the value is correct by sourcing it from the objects' control points again.
+                // this ensures that committing empty text causes a revert to the previous value.
+                bank.OnCommit += (_, __) => bank.Current.Value = getCommonBank(relevantControlPoints);
+
+                volume.Current.BindValueChanged(val => updateVolumeFor(relevantObjects, val.NewValue));
+            }
+
+            private static string? getCommonBank(SampleControlPoint[] relevantControlPoints) => relevantControlPoints.Select(point => point.SampleBank).Distinct().Count() == 1 ? relevantControlPoints.First().SampleBank : null;
+            private static int? getCommonVolume(SampleControlPoint[] relevantControlPoints) => relevantControlPoints.Select(point => point.SampleVolume).Distinct().Count() == 1 ? (int?)relevantControlPoints.First().SampleVolume : null;
+
+            private void updateBankFor(IEnumerable<HitObject> objects, string? newBank)
+            {
+                if (string.IsNullOrEmpty(newBank))
+                    return;
+
+                beatmap.BeginChange();
+
+                foreach (var h in objects)
+                {
+                    h.SampleControlPoint.SampleBank = newBank;
+                    beatmap.Update(h);
+                }
+
+                beatmap.EndChange();
+            }
+
+            private void updateBankPlaceholderText(IEnumerable<HitObject> objects)
+            {
+                string? commonBank = getCommonBank(objects.Select(h => h.SampleControlPoint).ToArray());
+                bank.PlaceholderText = string.IsNullOrEmpty(commonBank) ? "(multiple)" : null;
+            }
+
+            private void updateVolumeFor(IEnumerable<HitObject> objects, int? newVolume)
+            {
+                if (newVolume == null)
+                    return;
+
+                beatmap.BeginChange();
+
+                foreach (var h in objects)
+                {
+                    h.SampleControlPoint.SampleVolume = newVolume.Value;
+                    beatmap.Update(h);
+                }
+
+                beatmap.EndChange();
             }
         }
     }

osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs

@@ -11,6 +11,7 @@ using osu.Framework.Localisation;
 using osu.Game.Graphics.UserInterfaceV2;
 using osu.Game.Overlays.Settings;
 using osu.Game.Utils;
+using osuTK;
 
 namespace osu.Game.Screens.Edit.Timing
 {
@@ -62,6 +63,7 @@ namespace osu.Game.Screens.Edit.Timing
                     RelativeSizeAxes = Axes.X,
                     AutoSizeAxes = Axes.Y,
                     Direction = FillDirection.Vertical,
+                    Spacing = new Vector2(0, 5),
                     Children = new Drawable[]
                     {
                         textbox = new LabelledTextBox