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] 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)