From 9800cd490384bf41bebb1fe0d105e3a7c7ae4f16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 16:58:24 +0100 Subject: [PATCH 1/7] Add test coverage for sample control point piece operation --- ...estSceneHitObjectSamplePointAdjustments.cs | 139 ++++++++++++++++++ .../Components/Timeline/SamplePointPiece.cs | 6 +- 2 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs new file mode 100644 index 0000000000..24048e6052 --- /dev/null +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -0,0 +1,139 @@ +// 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 Humanizer; +using NUnit.Framework; +using osu.Framework.Testing; +using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; +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"); + } + + private void clickSamplePiece(int objectIndex) => AddStep($"click {objectIndex.ToOrdinalWords()} difficulty piece", () => + { + var difficultyPiece = this.ChildrenOfType().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().SingleOrDefault(); + var slider = popover?.ChildrenOfType>().Single(); + + return slider?.Current.Value == volume; + }); + + private void samplePopoverHasSingleBank(string bank) => AddUntilStep($"sample popover has bank {bank}", () => + { + var popover = this.ChildrenOfType().SingleOrDefault(); + var textBox = popover?.ChildrenOfType().First(); + + return textBox?.Current.Value == bank; + }); + + private void dismissPopover() + { + AddStep("dismiss popover", () => InputManager.Key(Key.Escape)); + AddUntilStep("wait for dismiss", () => !this.ChildrenOfType().Any(popover => popover.IsPresent)); + } + + private void setVolumeViaPopover(int volume) => AddStep($"set volume {volume} via popover", () => + { + var popover = this.ChildrenOfType().Single(); + var slider = popover.ChildrenOfType>().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().Single(); + var textBox = popover.ChildrenOfType().First(); + textBox.Current.Value = bank; + }); + + private void hitObjectHasSampleBank(int objectIndex, string bank) => AddAssert($"{objectIndex.ToOrdinalWords()} has bank {bank}", () => + { + var h = EditorBeatmap.HitObjects.ElementAt(objectIndex); + return h.SampleControlPoint.SampleBank == bank; + }); + } +} diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs index 6a26f69e41..6250a9ccb8 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs @@ -19,7 +19,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline { public class SamplePointPiece : HitObjectPointPiece, IHasPopover { - private readonly HitObject hitObject; + public readonly HitObject HitObject; private readonly Bindable bank; private readonly BindableNumber volume; @@ -27,7 +27,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,7 +50,7 @@ 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 { From 6ba154129bdae1037e4a3ae305020b0193e949b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 17:02:55 +0100 Subject: [PATCH 2/7] Add failing test coverage for setting sample volume on multiple objects --- ...estSceneHitObjectSamplePointAdjustments.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs index 24048e6052..0c4056e90c 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -80,6 +80,46 @@ namespace osu.Game.Tests.Visual.Editing 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); + } + private void clickSamplePiece(int objectIndex) => AddStep($"click {objectIndex.ToOrdinalWords()} difficulty piece", () => { var difficultyPiece = this.ChildrenOfType().Single(piece => piece.HitObject == EditorBeatmap.HitObjects.ElementAt(objectIndex)); @@ -96,6 +136,14 @@ namespace osu.Game.Tests.Visual.Editing return slider?.Current.Value == volume; }); + private void samplePopoverHasIndeterminateVolume() => AddUntilStep($"sample popover has indeterminate volume", () => + { + var popover = this.ChildrenOfType().SingleOrDefault(); + var slider = popover?.ChildrenOfType>().Single(); + + return slider != null && slider.Current.Value == null; + }); + private void samplePopoverHasSingleBank(string bank) => AddUntilStep($"sample popover has bank {bank}", () => { var popover = this.ChildrenOfType().SingleOrDefault(); From 9cf45e418ab78da6e12dd40665777a285cd88269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 17:07:19 +0100 Subject: [PATCH 3/7] Add failing test coverage for setting sample bank on multiple objects --- ...estSceneHitObjectSamplePointAdjustments.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs index 0c4056e90c..5b2b6fc780 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -120,6 +120,46 @@ namespace osu.Game.Tests.Visual.Editing 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("drum"); + hitObjectHasSampleBank(0, "drum"); + hitObjectHasSampleBank(1, "drum"); + } + + [Test] + public void TestMultipleSelectionWithDifferentSampleBank() + { + AddStep("select both objects", () => EditorBeatmap.SelectedHitObjects.AddRange(EditorBeatmap.HitObjects)); + clickSamplePiece(0); + samplePopoverHasIndeterminateBank(); + + dismissPopover(); + + clickSamplePiece(1); + samplePopoverHasIndeterminateBank(); + + setBankViaPopover("normal"); + hitObjectHasSampleBank(0, "normal"); + hitObjectHasSampleBank(1, "normal"); + } + private void clickSamplePiece(int objectIndex) => AddStep($"click {objectIndex.ToOrdinalWords()} difficulty piece", () => { var difficultyPiece = this.ChildrenOfType().Single(piece => piece.HitObject == EditorBeatmap.HitObjects.ElementAt(objectIndex)); @@ -152,6 +192,14 @@ namespace osu.Game.Tests.Visual.Editing return textBox?.Current.Value == bank; }); + private void samplePopoverHasIndeterminateBank() => AddUntilStep($"sample popover has indeterminate bank", () => + { + var popover = this.ChildrenOfType().SingleOrDefault(); + var textBox = popover?.ChildrenOfType().First(); + + return textBox != null && textBox.Current.Value == null; + }); + private void dismissPopover() { AddStep("dismiss popover", () => InputManager.Key(Key.Escape)); From 3fee6b0938261e62654a1b1e8ab677d68653e524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 17:21:48 +0100 Subject: [PATCH 4/7] Add support for setting sample bank & volume for multiple objects at once --- ...estSceneHitObjectSamplePointAdjustments.cs | 10 +-- .../Components/Timeline/SamplePointPiece.cs | 71 +++++++++++++++---- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs index 5b2b6fc780..f30c56d53c 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -171,15 +171,15 @@ namespace osu.Game.Tests.Visual.Editing private void samplePopoverHasSingleVolume(int volume) => AddUntilStep($"sample popover has volume {volume}", () => { var popover = this.ChildrenOfType().SingleOrDefault(); - var slider = popover?.ChildrenOfType>().Single(); + var slider = popover?.ChildrenOfType>().Single(); return slider?.Current.Value == volume; }); - private void samplePopoverHasIndeterminateVolume() => AddUntilStep($"sample popover has indeterminate volume", () => + private void samplePopoverHasIndeterminateVolume() => AddUntilStep("sample popover has indeterminate volume", () => { var popover = this.ChildrenOfType().SingleOrDefault(); - var slider = popover?.ChildrenOfType>().Single(); + var slider = popover?.ChildrenOfType>().Single(); return slider != null && slider.Current.Value == null; }); @@ -197,7 +197,7 @@ namespace osu.Game.Tests.Visual.Editing var popover = this.ChildrenOfType().SingleOrDefault(); var textBox = popover?.ChildrenOfType().First(); - return textBox != null && textBox.Current.Value == null; + return textBox != null && string.IsNullOrEmpty(textBox.Current.Value); }); private void dismissPopover() @@ -209,7 +209,7 @@ namespace osu.Game.Tests.Visual.Editing private void setVolumeViaPopover(int volume) => AddStep($"set volume {volume} via popover", () => { var popover = this.ChildrenOfType().Single(); - var slider = popover.ChildrenOfType>().Single(); + var slider = popover.ChildrenOfType>().Single(); slider.Current.Value = volume; }); diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs index 6250a9ccb8..f0b11ce96e 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs @@ -1,9 +1,14 @@ // Copyright (c) ppy Pty Ltd . 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; @@ -55,18 +60,16 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline public class SampleEditPopover : OsuPopover { private readonly HitObject hitObject; - private readonly SampleControlPoint point; - private LabelledTextBox bank; - private SliderWithTextBoxInput volume; + private LabelledTextBox bank = null!; + private IndeterminateSliderWithTextBoxInput 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] @@ -85,19 +88,61 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline { Label = "Bank Name", }, - volume = new SliderWithTextBoxInput("Volume") - { - Current = new SampleControlPoint().SampleVolumeBindable, - } + volume = new IndeterminateSliderWithTextBoxInput("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 = relevantControlPoints.Select(point => point.SampleBank).Distinct().Count() == 1 ? relevantControlPoints.First().SampleBank : null; + + if (!string.IsNullOrEmpty(commonBank)) + bank.Current.Value = commonBank; + + int? commonVolume = relevantControlPoints.Select(point => point.SampleVolume).Distinct().Count() == 1 ? (int?)relevantControlPoints.First().SampleVolume : null; + + if (commonVolume != null) + volume.Current.Value = commonVolume.Value; + + bank.Current.BindValueChanged(val => updateBankFor(relevantObjects, val.NewValue)); + volume.Current.BindValueChanged(val => updateVolumeFor(relevantObjects, val.NewValue)); + } + + private void updateBankFor(IEnumerable 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 updateVolumeFor(IEnumerable objects, int? newVolume) + { + if (newVolume == null) + return; + + beatmap.BeginChange(); + + foreach (var h in objects) + { + h.SampleControlPoint.SampleVolume = newVolume.Value; + beatmap.Update(h); + } + + beatmap.EndChange(); } } } From 76baf08140b4ae9971a44c9443f5b780a3d7ecef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 17:43:33 +0100 Subject: [PATCH 5/7] Expand test coverage with desired bank text box placeholder behaviour --- ...estSceneHitObjectSamplePointAdjustments.cs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs index f30c56d53c..460b608166 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -7,6 +7,7 @@ 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; @@ -43,7 +44,7 @@ namespace osu.Game.Tests.Visual.Editing } }); - EditorBeatmap.Add(new HitCircle() + EditorBeatmap.Add(new HitCircle { StartTime = 500, Position = (OsuPlayfield.BASE_SIZE + new Vector2(100, 0)) / 2, @@ -138,9 +139,15 @@ namespace osu.Game.Tests.Visual.Editing 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] @@ -155,9 +162,15 @@ namespace osu.Game.Tests.Visual.Editing 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", () => @@ -187,17 +200,17 @@ namespace osu.Game.Tests.Visual.Editing private void samplePopoverHasSingleBank(string bank) => AddUntilStep($"sample popover has bank {bank}", () => { var popover = this.ChildrenOfType().SingleOrDefault(); - var textBox = popover?.ChildrenOfType().First(); + var textBox = popover?.ChildrenOfType().First(); - return textBox?.Current.Value == bank; + return textBox?.Current.Value == bank && string.IsNullOrEmpty(textBox?.PlaceholderText.ToString()); }); - private void samplePopoverHasIndeterminateBank() => AddUntilStep($"sample popover has indeterminate bank", () => + private void samplePopoverHasIndeterminateBank() => AddUntilStep("sample popover has indeterminate bank", () => { var popover = this.ChildrenOfType().SingleOrDefault(); - var textBox = popover?.ChildrenOfType().First(); + var textBox = popover?.ChildrenOfType().First(); - return textBox != null && string.IsNullOrEmpty(textBox.Current.Value); + return textBox != null && string.IsNullOrEmpty(textBox.Current.Value) && !string.IsNullOrEmpty(textBox.PlaceholderText.ToString()); }); private void dismissPopover() From 73ca1d39a2fbaeef88ad13ebc7904027eece8d5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 18:06:32 +0100 Subject: [PATCH 6/7] Improve sample bank text box UX in case of multiple selection --- ...estSceneHitObjectSamplePointAdjustments.cs | 4 +++ .../Components/Timeline/SamplePointPiece.cs | 26 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs index 460b608166..dca30a6fc0 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneHitObjectSamplePointAdjustments.cs @@ -237,6 +237,10 @@ namespace osu.Game.Tests.Visual.Editing var popover = this.ChildrenOfType().Single(); var textBox = popover.ChildrenOfType().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}", () => diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs index f0b11ce96e..fbbbb153b9 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs @@ -99,20 +99,30 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline var relevantControlPoints = relevantObjects.Select(h => h.SampleControlPoint).ToArray(); // even if there are multiple objects selected, we can still display sample volume or bank if they all have the same value. - string? commonBank = relevantControlPoints.Select(point => point.SampleBank).Distinct().Count() == 1 ? relevantControlPoints.First().SampleBank : null; - + string? commonBank = getCommonBank(relevantControlPoints); if (!string.IsNullOrEmpty(commonBank)) bank.Current.Value = commonBank; - int? commonVolume = relevantControlPoints.Select(point => point.SampleVolume).Distinct().Count() == 1 ? (int?)relevantControlPoints.First().SampleVolume : null; - + int? commonVolume = getCommonVolume(relevantControlPoints); if (commonVolume != null) volume.Current.Value = commonVolume.Value; - bank.Current.BindValueChanged(val => updateBankFor(relevantObjects, val.NewValue)); + 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 objects, string? newBank) { if (string.IsNullOrEmpty(newBank)) @@ -129,6 +139,12 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline beatmap.EndChange(); } + private void updateBankPlaceholderText(IEnumerable objects) + { + string? commonBank = getCommonBank(objects.Select(h => h.SampleControlPoint).ToArray()); + bank.PlaceholderText = string.IsNullOrEmpty(commonBank) ? "(multiple)" : null; + } + private void updateVolumeFor(IEnumerable objects, int? newVolume) { if (newVolume == null) From 9a19a516f99cd62b4b2dc5d5baa76e982ca96bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 13 Nov 2021 18:09:40 +0100 Subject: [PATCH 7/7] Adjust spacings on sample point piece popover --- .../Edit/Compose/Components/Timeline/SamplePointPiece.cs | 2 ++ .../Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs index fbbbb153b9..2cbfe88519 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/SamplePointPiece.cs @@ -19,6 +19,7 @@ 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 { @@ -82,6 +83,7 @@ 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 diff --git a/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs b/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs index 17f82f4978..14b8c4c9de 100644 --- a/osu.Game/Screens/Edit/Timing/IndeterminateSliderWithTextBoxInput.cs +++ b/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