From 32cc3f9ef74594875f9049bbe7f5bdea901cb1e5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:00:57 +0900 Subject: [PATCH 01/23] Combine multiple similar invalidation logic into single event --- osu.Game/Screens/Select/BeatmapCarousel.cs | 33 ++++++++-------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index eb47a7201a99..19ade780e196 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -168,7 +168,7 @@ private void loadBeatmapSets(IEnumerable beatmapSets) applyActiveCriteria(false); if (loadedTestBeatmaps) - signalBeatmapsLoaded(); + invalidateAfterChange(true); // Restore selection if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) @@ -298,7 +298,8 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS removeBeatmapSet(id); } - signalBeatmapsLoaded(); + invalidateAfterChange(!BeatmapSetsLoaded); + BeatmapSetsLoaded = true; return; } @@ -393,12 +394,7 @@ private void removeBeatmapSet(Guid beatmapSetID) => Schedule(() => root.RemoveItem(set); } - itemsCache.Invalidate(); - - if (!Scroll.UserScrolling) - ScrollToSelected(true); - - BeatmapSetsChanged?.Invoke(); + invalidateAfterChange(true); }); public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => @@ -465,12 +461,7 @@ public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => } } - itemsCache.Invalidate(); - - if (!Scroll.UserScrolling) - ScrollToSelected(true); - - BeatmapSetsChanged?.Invoke(); + invalidateAfterChange(true); }); /// @@ -748,15 +739,15 @@ void perform() } } - private void signalBeatmapsLoaded() + private void invalidateAfterChange(bool invokeSetsChangedEvent) { - if (!BeatmapSetsLoaded) - { - BeatmapSetsChanged?.Invoke(); - BeatmapSetsLoaded = true; - } - itemsCache.Invalidate(); + + if (!Scroll.UserScrolling) + ScrollToSelected(true); + + if (invokeSetsChangedEvent) + BeatmapSetsChanged?.Invoke(); } private float? scrollTarget; From 87b7699fcc29ab7e70ec1f632a0341bbe08a23b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:09:09 +0900 Subject: [PATCH 02/23] Avoid calling invalidation logic per beatmap set updated Realm will batch the updates. We don't want to do expensive operations per set when we don't need to. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 33 +++++++++++++++------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 19ade780e196..63654fd36a94 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -289,7 +289,7 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS foreach (var id in realmSets) { if (!root.BeatmapSetsByID.ContainsKey(id)) - UpdateBeatmapSet(realm.Realm.Find(id)!.Detach()); + updateBeatmapSet(realm.Realm.Find(id)!.Detach()); } foreach (var id in root.BeatmapSetsByID.Keys) @@ -304,10 +304,10 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS } foreach (int i in changes.NewModifiedIndices) - UpdateBeatmapSet(sender[i].Detach()); + updateBeatmapSet(sender[i].Detach()); foreach (int i in changes.InsertedIndices) - UpdateBeatmapSet(sender[i].Detach()); + updateBeatmapSet(sender[i].Detach()); if (changes.DeletedIndices.Length > 0 && SelectedBeatmapInfo != null) { @@ -348,6 +348,8 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS SelectBeatmap(sender[modifiedAndInserted.First()].Beatmaps.First()); } } + + invalidateAfterChange(!BeatmapSetsLoaded); } private void beatmapsChanged(IRealmCollection sender, ChangeSet? changes) @@ -356,6 +358,8 @@ private void beatmapsChanged(IRealmCollection sender, ChangeSet? ch if (changes == null) return; + bool changed = false; + foreach (int i in changes.InsertedIndices) { var beatmapInfo = sender[i]; @@ -368,17 +372,24 @@ private void beatmapsChanged(IRealmCollection sender, ChangeSet? ch if (root.BeatmapSetsByID.TryGetValue(beatmapSet.ID, out var existingSets) && existingSets.SelectMany(s => s.Beatmaps).All(b => b.BeatmapInfo.ID != beatmapInfo.ID)) { - UpdateBeatmapSet(beatmapSet.Detach()); + updateBeatmapSet(beatmapSet.Detach()); + changed = true; } } + + if (changed) + invalidateAfterChange(true); } private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); - public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => + public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => + { removeBeatmapSet(beatmapSet.ID); + invalidateAfterChange(true); + }); - private void removeBeatmapSet(Guid beatmapSetID) => Schedule(() => + private void removeBeatmapSet(Guid beatmapSetID) { if (!root.BeatmapSetsByID.TryGetValue(beatmapSetID, out var existingSets)) return; @@ -393,11 +404,15 @@ private void removeBeatmapSet(Guid beatmapSetID) => Schedule(() => root.RemoveItem(set); } + } + public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => + { + updateBeatmapSet(beatmapSet); invalidateAfterChange(true); }); - public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => + private void updateBeatmapSet(BeatmapSetInfo beatmapSet) { Guid? previouslySelectedID = null; @@ -460,9 +475,7 @@ public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => select((CarouselItem?)newSet.Beatmaps.FirstOrDefault(b => b.BeatmapInfo.ID == previouslySelectedID) ?? newSet); } } - - invalidateAfterChange(true); - }); + } /// /// Selects a given beatmap on the carousel. From 6fa1f5ef9bfaac577cad7b29d4840e9302c6a889 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:10:31 +0900 Subject: [PATCH 03/23] Simplify invalidation logic The only case where this was checking is guaranteed by realm to only be called once. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63654fd36a94..5178413ad693 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -168,7 +168,7 @@ private void loadBeatmapSets(IEnumerable beatmapSets) applyActiveCriteria(false); if (loadedTestBeatmaps) - invalidateAfterChange(true); + invalidateAfterChange(); // Restore selection if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) @@ -298,7 +298,7 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS removeBeatmapSet(id); } - invalidateAfterChange(!BeatmapSetsLoaded); + invalidateAfterChange(); BeatmapSetsLoaded = true; return; } @@ -349,7 +349,7 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS } } - invalidateAfterChange(!BeatmapSetsLoaded); + invalidateAfterChange(); } private void beatmapsChanged(IRealmCollection sender, ChangeSet? changes) @@ -378,7 +378,7 @@ private void beatmapsChanged(IRealmCollection sender, ChangeSet? ch } if (changed) - invalidateAfterChange(true); + invalidateAfterChange(); } private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); @@ -386,7 +386,7 @@ private void beatmapsChanged(IRealmCollection sender, ChangeSet? ch public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => { removeBeatmapSet(beatmapSet.ID); - invalidateAfterChange(true); + invalidateAfterChange(); }); private void removeBeatmapSet(Guid beatmapSetID) @@ -409,7 +409,7 @@ private void removeBeatmapSet(Guid beatmapSetID) public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => { updateBeatmapSet(beatmapSet); - invalidateAfterChange(true); + invalidateAfterChange(); }); private void updateBeatmapSet(BeatmapSetInfo beatmapSet) @@ -752,15 +752,14 @@ void perform() } } - private void invalidateAfterChange(bool invokeSetsChangedEvent) + private void invalidateAfterChange() { itemsCache.Invalidate(); if (!Scroll.UserScrolling) ScrollToSelected(true); - if (invokeSetsChangedEvent) - BeatmapSetsChanged?.Invoke(); + BeatmapSetsChanged?.Invoke(); } private float? scrollTarget; From 034c5cd6541d622fe42ed1c301b647a3ac06afd8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:30:56 +0900 Subject: [PATCH 04/23] Debounce count updates for good measure --- osu.Game/Screens/Select/SongSelect.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index dfea4e37942f..7c30fd5baa3d 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -162,7 +162,7 @@ private void load(AudioManager audio, OsuColour colours, ManageCollectionsDialog BleedBottom = Footer.HEIGHT, SelectionChanged = updateSelectedBeatmap, BeatmapSetsChanged = carouselBeatmapsLoaded, - FilterApplied = updateVisibleBeatmapCount, + FilterApplied = () => Scheduler.Add(updateVisibleBeatmapCount), GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s), }, c => carouselContainer.Child = c); @@ -843,7 +843,7 @@ private void ensurePlayingSelected() private void carouselBeatmapsLoaded() { bindBindables(); - updateVisibleBeatmapCount(); + Scheduler.AddOnce(updateVisibleBeatmapCount); Carousel.AllowSelection = true; @@ -877,7 +877,8 @@ private void updateVisibleBeatmapCount() { // Intentionally not localised until we have proper support for this (see https://github.com/ppy/osu-framework/pull/4918 // but also in this case we want support for formatting a number within a string). - FilterControl.InformationalText = Carousel.CountDisplayed != 1 ? $"{Carousel.CountDisplayed:#,0} matches" : $"{Carousel.CountDisplayed:#,0} match"; + int carouselCountDisplayed = Carousel.CountDisplayed; + FilterControl.InformationalText = carouselCountDisplayed != 1 ? $"{carouselCountDisplayed:#,0} matches" : $"{carouselCountDisplayed:#,0} match"; } private bool boundLocalBindables; From 5755fa214a450d1663babcad4fd1eb5bcafb12e0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:44:08 +0900 Subject: [PATCH 05/23] Cache non-filtered beatmap counts to massively improve count performance --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- .../Screens/Select/Carousel/CarouselGroup.cs | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 5178413ad693..919a320bca54 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -64,7 +64,7 @@ public partial class BeatmapCarousel : CompositeDrawable, IKeyBindingHandler /// The total count of non-filtered beatmaps displayed. /// - public int CountDisplayed => beatmapSets.Where(s => !s.Filtered.Value).Sum(s => s.Beatmaps.Count(b => !b.Filtered.Value)); + public int CountDisplayed => beatmapSets.Where(s => !s.Filtered.Value).Sum(s => s.TotalItemsNotFiltered); /// /// The currently selected beatmap set. diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index c353ee98ae3f..5aefdf5e28ff 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -14,6 +14,8 @@ public class CarouselGroup : CarouselItem public IReadOnlyList Items => items; + public int TotalItemsNotFiltered { get; private set; } + private readonly List items = new List(); /// @@ -31,6 +33,9 @@ public virtual void RemoveItem(CarouselItem i) { items.Remove(i); + if (!i.Filtered.Value) + TotalItemsNotFiltered--; + // it's important we do the deselection after removing, so any further actions based on // State.ValueChanged make decisions post-removal. i.State.Value = CarouselItemState.Collapsed; @@ -55,6 +60,9 @@ public virtual void AddItem(CarouselItem i) // criteria may be null for initial population. the filtering will be applied post-add. items.Add(i); } + + if (!i.Filtered.Value) + TotalItemsNotFiltered--; } public CarouselGroup(List? items = null) @@ -84,7 +92,14 @@ public override void Filter(FilterCriteria criteria) { base.Filter(criteria); - items.ForEach(c => c.Filter(criteria)); + TotalItemsNotFiltered = 0; + + foreach (var c in items) + { + c.Filter(criteria); + if (!c.Filtered.Value) + TotalItemsNotFiltered++; + } // Sorting is expensive, so only perform if it's actually changed. if (lastCriteria?.Sort != criteria.Sort) From 25df42630ecba979457a9dcd2a7678d154a85a5e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 20:47:08 +0900 Subject: [PATCH 06/23] Improve performance of `attemptSelection` using new cached count and `LastSelected` --- osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 7f90e05744cb..8d2ddc58122a 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Linq; namespace osu.Game.Screens.Select.Carousel { @@ -101,7 +100,7 @@ private void attemptSelection() if (State.Value != CarouselItemState.Selected) return; // we only perform eager selection if none of our items are in a selected state already. - if (Items.Any(i => i.State.Value == CarouselItemState.Selected)) return; + if (LastSelected?.State.Value == CarouselItemState.Selected || TotalItemsNotFiltered == 0) return; PerformSelection(); } From 25e3a8e82e712d18c904ccfcc29fbcf821e94163 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 22:24:57 +0900 Subject: [PATCH 07/23] Fix a few of silly issues --- osu.Game/Screens/Select/BeatmapCarousel.cs | 3 +++ osu.Game/Screens/Select/Carousel/CarouselGroup.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 919a320bca54..607b891beb0a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -168,7 +168,10 @@ private void loadBeatmapSets(IEnumerable beatmapSets) applyActiveCriteria(false); if (loadedTestBeatmaps) + { invalidateAfterChange(); + BeatmapSetsLoaded = true; + } // Restore selection if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index 5aefdf5e28ff..f5ea32a22abd 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -62,7 +62,7 @@ public virtual void AddItem(CarouselItem i) } if (!i.Filtered.Value) - TotalItemsNotFiltered--; + TotalItemsNotFiltered++; } public CarouselGroup(List? items = null) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 7c30fd5baa3d..d23a660ff682 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -162,7 +162,7 @@ private void load(AudioManager audio, OsuColour colours, ManageCollectionsDialog BleedBottom = Footer.HEIGHT, SelectionChanged = updateSelectedBeatmap, BeatmapSetsChanged = carouselBeatmapsLoaded, - FilterApplied = () => Scheduler.Add(updateVisibleBeatmapCount), + FilterApplied = () => Scheduler.AddOnce(updateVisibleBeatmapCount), GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s), }, c => carouselContainer.Child = c); From d81cabc06361ab9cfb20f7294c38166d4bdb03a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 22:35:36 +0900 Subject: [PATCH 08/23] Revert "Improve performance of `attemptSelection` using new cached count and `LastSelected`" This reverts commit 25df42630ecba979457a9dcd2a7678d154a85a5e. --- osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs index 8d2ddc58122a..7f90e05744cb 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroupEagerSelect.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; namespace osu.Game.Screens.Select.Carousel { @@ -100,7 +101,7 @@ private void attemptSelection() if (State.Value != CarouselItemState.Selected) return; // we only perform eager selection if none of our items are in a selected state already. - if (LastSelected?.State.Value == CarouselItemState.Selected || TotalItemsNotFiltered == 0) return; + if (Items.Any(i => i.State.Value == CarouselItemState.Selected)) return; PerformSelection(); } From 51f4c7254c80bca74ea0ae9e772c630feb7f6a4e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 01:32:30 +0900 Subject: [PATCH 09/23] Fix mod search textbox having focus while settings are visible Stopped arrow key adjust on slider bars from working. Also just felt wrong that you could type into an off-screen textbox. --- osu.Game/Overlays/Mods/ModSelectOverlay.cs | 13 ++++++++----- osu.Game/Overlays/Mods/ModSettingsArea.cs | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModSelectOverlay.cs b/osu.Game/Overlays/Mods/ModSelectOverlay.cs index ce798ae75208..30d7b6191e6d 100644 --- a/osu.Game/Overlays/Mods/ModSelectOverlay.cs +++ b/osu.Game/Overlays/Mods/ModSelectOverlay.cs @@ -508,6 +508,11 @@ private void updateCustomisationVisualState() modSettingsArea.ResizeHeightTo(modAreaHeight, transition_duration, Easing.InOutCubic); TopLevelContent.MoveToY(-modAreaHeight, transition_duration, Easing.InOutCubic); + + if (customisationVisible.Value) + GetContainingInputManager().ChangeFocus(modSettingsArea); + else + Scheduler.Add(() => GetContainingInputManager().ChangeFocus(null)); } /// @@ -622,7 +627,7 @@ protected override void PopIn() } if (textSearchStartsActive.Value) - SearchTextBox.TakeFocus(); + SearchTextBox.HoldFocus = true; } protected override void PopOut() @@ -761,11 +766,9 @@ protected override bool OnKeyDown(KeyDownEvent e) return false; // TODO: should probably eventually support typical platform search shortcuts (`Ctrl-F`, `/`) - if (SearchTextBox.HasFocus) - SearchTextBox.KillFocus(); - else + SearchTextBox.HoldFocus = !SearchTextBox.HoldFocus; + if (SearchTextBox.HoldFocus) SearchTextBox.TakeFocus(); - return true; } diff --git a/osu.Game/Overlays/Mods/ModSettingsArea.cs b/osu.Game/Overlays/Mods/ModSettingsArea.cs index 6158c2c70f16..54bfcc719974 100644 --- a/osu.Game/Overlays/Mods/ModSettingsArea.cs +++ b/osu.Game/Overlays/Mods/ModSettingsArea.cs @@ -32,6 +32,8 @@ public partial class ModSettingsArea : CompositeDrawable [Resolved] private OverlayColourProvider colourProvider { get; set; } = null!; + public override bool AcceptsFocus => true; + public ModSettingsArea() { RelativeSizeAxes = Axes.X; From 41485c19cfe597a5144911fdaff34fe218f1d963 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 02:08:25 +0900 Subject: [PATCH 10/23] Add realm refresh steps in an attempt to stabilise failing test I think this is required because there is a higher chance of batched updates with the new structure (and less calls to `BeatmapSetsChanged` which causes re-selection). --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 6b53277964e3..28b4dae56d24 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -464,6 +464,8 @@ public void TestSelectionRetainedOnBeatmapUpdate() manager.Import(testBeatmapSetInfo); }, 10); + AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); + AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID); Task?> updateTask = null!; @@ -476,6 +478,8 @@ public void TestSelectionRetainedOnBeatmapUpdate() }); AddUntilStep("wait for update completion", () => updateTask.IsCompleted); + AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); + AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID); } From bf668174ecc743f5941035a57fbbbcfa8d2278fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 18 Dec 2023 19:02:23 +0100 Subject: [PATCH 11/23] Use nunit constraints in test for transparency --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 28b4dae56d24..6af53df725d3 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -466,7 +466,7 @@ public void TestSelectionRetainedOnBeatmapUpdate() AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID); + AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); Task?> updateTask = null!; @@ -480,7 +480,7 @@ public void TestSelectionRetainedOnBeatmapUpdate() AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID == originalOnlineSetID); + AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); } [Test] From cc800a18b203e474fc9fe6d43f50c2bc4e63ea03 Mon Sep 17 00:00:00 2001 From: Susko3 Date: Mon, 18 Dec 2023 21:11:00 +0100 Subject: [PATCH 12/23] Fix opening log files from notification not presenting the correct file --- osu.Game/OsuGame.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 8d9e029c9d1b..e7ff99ef01ea 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1190,7 +1190,7 @@ private void forwardGeneralLogsToNotifications() } else if (recentLogCount == short_term_display_limit) { - string logFile = $@"{entry.Target.Value.ToString().ToLowerInvariant()}.log"; + string logFile = Logger.GetLogger(entry.Target.Value).Filename; Schedule(() => Notifications.Post(new SimpleNotification { @@ -1198,7 +1198,7 @@ private void forwardGeneralLogsToNotifications() Text = NotificationsStrings.SubsequentMessagesLogged, Activated = () => { - Storage.GetStorageForDirectory(@"logs").PresentFileExternally(logFile); + Logger.Storage.PresentFileExternally(logFile); return true; } })); From ee8a5d5a3077c5a1045ef74fc8b037296a26a005 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 13:33:45 +0900 Subject: [PATCH 13/23] Update bug-issue.yml with new workflow for exporting logs --- .github/ISSUE_TEMPLATE/bug-issue.yml | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug-issue.yml b/.github/ISSUE_TEMPLATE/bug-issue.yml index ff6d869e7200..17a3e1df4137 100644 --- a/.github/ISSUE_TEMPLATE/bug-issue.yml +++ b/.github/ISSUE_TEMPLATE/bug-issue.yml @@ -46,22 +46,16 @@ body: value: | ## Logs - Attaching log files is required for every reported bug. See instructions below on how to find them. - - **Logs are reset when you reopen the game.** If the game crashed or has been closed since you found the bug, retrieve the logs using the file explorer instead. + Attaching log files is required for **every** issue, regardless of whether you deem them required or not. See instructions below on how to find them. ### Desktop platforms If the game has not yet been closed since you found the bug: - 1. Head on to game settings and click on "Open osu! folder" - 2. Then open the `logs` folder located there - - The default places to find the logs on desktop platforms are as follows: - - `%AppData%/osu/logs` *on Windows* - - `~/.local/share/osu/logs` *on Linux* - - `~/Library/Application Support/osu/logs` *on macOS* + 1. Head on to game settings and click on "Export logs" + 2. Click the notification to locate the file + 3. Drag the generated `.zip` files into the github issue window - If you have selected a custom location for the game files, you can find the `logs` folder there. + ![export logs button](https://github.com/ppy/osu/assets/191335/0866443f-0728-47bc-9dbd-f2b79ac802d5) ### Mobile platforms @@ -69,10 +63,6 @@ body: - *On Android*, navigate to `Android/data/sh.ppy.osulazer/files/logs` using a file browser app. - *On iOS*, connect your device to a PC and copy the `logs` directory from the app's document storage using iTunes. (https://support.apple.com/en-us/HT201301#copy-to-computer) - --- - - After locating the `logs` folder, select all log files inside and drag them into the "Logs" box below. - - type: textarea attributes: label: Logs From 469a659938dfb3c696c9a9fb51c7252214014a31 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 13:35:02 +0900 Subject: [PATCH 14/23] Fix arrow pointing to wrong place --- .github/ISSUE_TEMPLATE/bug-issue.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug-issue.yml b/.github/ISSUE_TEMPLATE/bug-issue.yml index 17a3e1df4137..00a873f9c8b6 100644 --- a/.github/ISSUE_TEMPLATE/bug-issue.yml +++ b/.github/ISSUE_TEMPLATE/bug-issue.yml @@ -53,9 +53,9 @@ body: If the game has not yet been closed since you found the bug: 1. Head on to game settings and click on "Export logs" 2. Click the notification to locate the file - 3. Drag the generated `.zip` files into the github issue window + 3. Drag the generated `.zip` files into the github issue window - ![export logs button](https://github.com/ppy/osu/assets/191335/0866443f-0728-47bc-9dbd-f2b79ac802d5) + ![export logs button](https://github.com/ppy/osu/assets/191335/cbfa5550-b7ed-4c5c-8dd0-8b87cc90ad9b) ### Mobile platforms From 44efa2c540c392c998af7cd052a88d35d053aa5b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 15:09:03 +0900 Subject: [PATCH 15/23] Fix incorrect ordering of items at song select when certain sort modes are used --- .../Screens/Select/Carousel/CarouselGroup.cs | 2 +- osu.Game/Screens/Select/FilterCriteria.cs | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs index c353ee98ae3f..be841465bfdb 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselGroup.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselGroup.cs @@ -87,7 +87,7 @@ public override void Filter(FilterCriteria criteria) items.ForEach(c => c.Filter(criteria)); // Sorting is expensive, so only perform if it's actually changed. - if (lastCriteria?.Sort != criteria.Sort) + if (lastCriteria?.RequiresSorting(criteria) != false) { criteriaComparer = Comparer.Create((x, y) => { diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 811f623ee54c..0bea2247cead 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -219,6 +219,44 @@ public string SearchTerm public bool Equals(OptionalTextFilter other) => SearchTerm == other.SearchTerm; } + /// + /// Given a new filter criteria, decide whether a full sort needs to be performed. + /// + /// + /// + public bool RequiresSorting(FilterCriteria newCriteria) + { + if (Sort != newCriteria.Sort) + return true; + + switch (Sort) + { + // Some sorts are stable across all other changes. + // Running these sorts will sort all items, including currently hidden items. + case SortMode.Artist: + case SortMode.Author: + case SortMode.DateSubmitted: + case SortMode.DateAdded: + case SortMode.DateRanked: + case SortMode.Source: + case SortMode.Title: + return false; + + // Some sorts use aggregate max comparisons, which will change based on filtered items. + // These sorts generally ignore items hidden by filtered state, so we must force a sort under all circumstances here. + // + // This makes things very slow when typing a text search, and we probably want to consider a way to optimise things going forward. + case SortMode.LastPlayed: + case SortMode.BPM: + case SortMode.Length: + case SortMode.Difficulty: + return true; + + default: + throw new ArgumentOutOfRangeException(); + } + } + public enum MatchMode { /// From 011bd61e7d3f7301fd00bd5acd0d89120a92b61a Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 19 Dec 2023 16:47:49 +0900 Subject: [PATCH 16/23] Give sliders in test scene a sample --- .../TestSceneSliderEarlyHitJudgement.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderEarlyHitJudgement.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderEarlyHitJudgement.cs index 9caee86a3250..4ea21e51f66a 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderEarlyHitJudgement.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderEarlyHitJudgement.cs @@ -6,6 +6,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Screens; +using osu.Game.Audio; using osu.Game.Beatmaps; using osu.Game.Replays; using osu.Game.Rulesets.Judgements; @@ -160,6 +161,10 @@ private void performTest(List frames, Action? adjustSliderF Position = new Vector2(256 - slider_path_length / 2, 192), TickDistanceMultiplier = 3, ClassicSliderBehaviour = classic, + Samples = new[] + { + new HitSampleInfo(HitSampleInfo.HIT_NORMAL) + }, Path = new SliderPath(PathType.LINEAR, new[] { Vector2.Zero, From 7c05d66bd7913c3f7aa1a569af5fa1dd901c58dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Dec 2023 08:57:18 +0100 Subject: [PATCH 17/23] Add more detail to exception --- osu.Game/Screens/Select/FilterCriteria.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index 0bea2247cead..a7c8e7d09372 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -253,7 +253,7 @@ public bool RequiresSorting(FilterCriteria newCriteria) return true; default: - throw new ArgumentOutOfRangeException(); + throw new ArgumentOutOfRangeException(nameof(Sort), Sort, "Unknown sort mode"); } } From fe5e071e70cbd0a24f555fb9255dfa957246eb36 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 19 Dec 2023 17:01:52 +0900 Subject: [PATCH 18/23] Fix sliding sample playing before Slider's start time --- .../Objects/Drawables/DrawableSlider.cs | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 1f9a02804598..b306fd38c19e 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -128,8 +128,6 @@ private void load() foreach (var drawableHitObject in NestedHitObjects) drawableHitObject.AccentColour.Value = colour.NewValue; }, true); - - Tracking.BindValueChanged(updateSlidingSample); } protected override void OnApply() @@ -166,14 +164,6 @@ public override void StopAllSamples() slidingSample?.Stop(); } - private void updateSlidingSample(ValueChangedEvent tracking) - { - if (tracking.NewValue) - slidingSample?.Play(); - else - slidingSample?.Stop(); - } - protected override void AddNestedHitObject(DrawableHitObject hitObject) { base.AddNestedHitObject(hitObject); @@ -238,9 +228,18 @@ protected override void Update() Tracking.Value = SliderInputManager.Tracking; - if (Tracking.Value && slidingSample != null) - // keep the sliding sample playing at the current tracking position - slidingSample.Balance.Value = CalculateSamplePlaybackBalance(CalculateDrawableRelativePosition(Ball)); + if (slidingSample != null) + { + if (Tracking.Value && Time.Current >= HitObject.StartTime) + { + // keep the sliding sample playing at the current tracking position + if (!slidingSample.IsPlaying) + slidingSample.Play(); + slidingSample.Balance.Value = CalculateSamplePlaybackBalance(CalculateDrawableRelativePosition(Ball)); + } + else if (slidingSample.IsPlaying) + slidingSample.Stop(); + } double completionProgress = Math.Clamp((Time.Current - HitObject.StartTime) / HitObject.Duration, 0, 1); From 7e9c1b2acbaabb19564821d748deb2c97a964554 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 17:27:52 +0900 Subject: [PATCH 19/23] Use `sender`'s realm (because we can) --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 607b891beb0a..4c6c67c34847 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -320,7 +320,7 @@ private void beatmapSetsChanged(IRealmCollection sender, ChangeS // To handle the beatmap update flow, attempt to track selection changes across delete-insert transactions. // When an update occurs, the previous beatmap set is either soft or hard deleted. // Check if the current selection was potentially deleted by re-querying its validity. - bool selectedSetMarkedDeleted = realm.Run(r => r.Find(SelectedBeatmapSet.ID))?.DeletePending != false; + bool selectedSetMarkedDeleted = sender.Realm.Find(SelectedBeatmapSet.ID)?.DeletePending != false; int[] modifiedAndInserted = changes.NewModifiedIndices.Concat(changes.InsertedIndices).ToArray(); From 8f5d21dc703e3c1f3930e4b4e1c58c90acaba87a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 18:10:55 +0900 Subject: [PATCH 20/23] Actually fix realm selection retention regression --- .../SongSelect/TestScenePlaySongSelect.cs | 4 ---- osu.Game/Screens/Select/BeatmapCarousel.cs | 24 +++++++++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 6af53df725d3..518035fb822a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -464,8 +464,6 @@ public void TestSelectionRetainedOnBeatmapUpdate() manager.Import(testBeatmapSetInfo); }, 10); - AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("has selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); Task?> updateTask = null!; @@ -478,8 +476,6 @@ public void TestSelectionRetainedOnBeatmapUpdate() }); AddUntilStep("wait for update completion", () => updateTask.IsCompleted); - AddStep("Force realm refresh", () => Realm.Run(r => r.Refresh())); - AddUntilStep("retained selection", () => songSelect!.Carousel.SelectedBeatmapInfo?.BeatmapSet?.OnlineID, () => Is.EqualTo(originalOnlineSetID)); } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 4c6c67c34847..47cdbe34f499 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -269,8 +269,28 @@ private void deletedBeatmapSetsChanged(IRealmCollection sender, if (changes == null) return; - foreach (int i in changes.InsertedIndices) - removeBeatmapSet(sender[i].ID); + var removeableSets = changes.InsertedIndices.Select(i => sender[i].ID).ToHashSet(); + + // This schedule is required to retain selection of beatmaps over an ImportAsUpdate operation. + // This is covered by TestPlaySongSelect.TestSelectionRetainedOnBeatmapUpdate. + // + // In short, we have specialised logic in `beatmapSetsChanged` (directly below) to infer that an + // update operation has occurred. For this to work, we need to confirm the `DeletePending` flag + // of the current selection. + // + // If we don't schedule the following code, it is possible for the `deleteBeatmapSetsChanged` handler + // to be invoked before the `beatmapSetsChanged` handler (realm call order seems non-deterministic) + // which will lead to the currently selected beatmap changing via `CarouselGroupEagerSelect`. + // + // We need a better path forward here. A few ideas: + // - Avoid the necessity of having realm subscriptions on deleted/hidden items, maybe by storing all guids in realm + // to a local list so we can better look them up on receiving `DeletedIndices`. + // - Add a new property on `BeatmapSetInfo` to link to the pre-update set, and use that to handle the update case. + Schedule(() => + { + foreach (var set in removeableSets) + removeBeatmapSet(set); + }); } private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) From 502e3edac3ea2c5082718ca49d87064fdea4e92e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 19:39:48 +0900 Subject: [PATCH 21/23] Add missing invalidation call --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 47cdbe34f499..fe7eee701f5f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -290,6 +290,8 @@ private void deletedBeatmapSetsChanged(IRealmCollection sender, { foreach (var set in removeableSets) removeBeatmapSet(set); + + invalidateAfterChange(); }); } From c556475c2c3522729cc1cf17b02b1497730d6e66 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 19:46:30 +0900 Subject: [PATCH 22/23] Revert to using a more manual approach to holding focus --- osu.Game/Overlays/Mods/ModSelectOverlay.cs | 23 +++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/osu.Game/Overlays/Mods/ModSelectOverlay.cs b/osu.Game/Overlays/Mods/ModSelectOverlay.cs index 30d7b6191e6d..baa7e594c18e 100644 --- a/osu.Game/Overlays/Mods/ModSelectOverlay.cs +++ b/osu.Game/Overlays/Mods/ModSelectOverlay.cs @@ -132,6 +132,8 @@ protected virtual IEnumerable CreateFooterButtons() protected ShearedToggleButton? CustomisationButton { get; private set; } protected SelectAllModsButton? SelectAllModsButton { get; set; } + private bool textBoxShouldFocus; + private Sample? columnAppearSample; private WorkingBeatmap? beatmap; @@ -510,9 +512,9 @@ private void updateCustomisationVisualState() TopLevelContent.MoveToY(-modAreaHeight, transition_duration, Easing.InOutCubic); if (customisationVisible.Value) - GetContainingInputManager().ChangeFocus(modSettingsArea); + SearchTextBox.KillFocus(); else - Scheduler.Add(() => GetContainingInputManager().ChangeFocus(null)); + setTextBoxFocus(textBoxShouldFocus); } /// @@ -626,8 +628,7 @@ protected override void PopIn() nonFilteredColumnCount += 1; } - if (textSearchStartsActive.Value) - SearchTextBox.HoldFocus = true; + setTextBoxFocus(textSearchStartsActive.Value); } protected override void PopOut() @@ -766,12 +767,20 @@ protected override bool OnKeyDown(KeyDownEvent e) return false; // TODO: should probably eventually support typical platform search shortcuts (`Ctrl-F`, `/`) - SearchTextBox.HoldFocus = !SearchTextBox.HoldFocus; - if (SearchTextBox.HoldFocus) - SearchTextBox.TakeFocus(); + setTextBoxFocus(!textBoxShouldFocus); return true; } + private void setTextBoxFocus(bool keepFocus) + { + textBoxShouldFocus = keepFocus; + + if (textBoxShouldFocus) + SearchTextBox.TakeFocus(); + else + SearchTextBox.KillFocus(); + } + #endregion #region Sample playback control From d793d1cea16de4098c58fb5c50a3e20142a7a996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Dec 2023 14:52:16 +0100 Subject: [PATCH 23/23] Add test coverage of desired input handling behaviour --- .../TestSceneModSelectOverlay.cs | 29 +++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs index 80be4412b39e..4b101a52f42a 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneModSelectOverlay.cs @@ -572,7 +572,7 @@ void navigateAndClick() where T : Drawable [Test] public void TestTextSearchActiveByDefault() { - configManager.SetValue(OsuSetting.ModSelectTextSearchStartsActive, true); + AddStep("text search starts active", () => configManager.SetValue(OsuSetting.ModSelectTextSearchStartsActive, true)); createScreen(); AddUntilStep("search text box focused", () => modSelectOverlay.SearchTextBox.HasFocus); @@ -587,7 +587,7 @@ public void TestTextSearchActiveByDefault() [Test] public void TestTextSearchNotActiveByDefault() { - configManager.SetValue(OsuSetting.ModSelectTextSearchStartsActive, false); + AddStep("text search does not start active", () => configManager.SetValue(OsuSetting.ModSelectTextSearchStartsActive, false)); createScreen(); AddUntilStep("search text box not focused", () => !modSelectOverlay.SearchTextBox.HasFocus); @@ -599,6 +599,31 @@ public void TestTextSearchNotActiveByDefault() AddAssert("search text box unfocused", () => !modSelectOverlay.SearchTextBox.HasFocus); } + [Test] + public void TestTextSearchDoesNotBlockCustomisationPanelKeyboardInteractions() + { + AddStep("text search starts active", () => configManager.SetValue(OsuSetting.ModSelectTextSearchStartsActive, true)); + createScreen(); + + AddUntilStep("search text box focused", () => modSelectOverlay.SearchTextBox.HasFocus); + + AddStep("select DT", () => SelectedMods.Value = new Mod[] { new OsuModDoubleTime() }); + AddAssert("DT selected", () => modSelectOverlay.ChildrenOfType().Count(panel => panel.Active.Value), () => Is.EqualTo(1)); + + AddStep("open customisation area", () => modSelectOverlay.CustomisationButton!.TriggerClick()); + assertCustomisationToggleState(false, true); + AddStep("hover over mod settings slider", () => + { + var slider = modSelectOverlay.ChildrenOfType().Single().ChildrenOfType>().First(); + InputManager.MoveMouseTo(slider); + }); + AddStep("press right arrow", () => InputManager.PressKey(Key.Right)); + AddAssert("DT speed changed", () => !SelectedMods.Value.OfType().Single().SpeedChange.IsDefault); + + AddStep("close customisation area", () => InputManager.PressKey(Key.Escape)); + AddUntilStep("search text box reacquired focus", () => modSelectOverlay.SearchTextBox.HasFocus); + } + [Test] public void TestDeselectAllViaKey() {