From 290d18ad690accc2a54c3290e3403f6c2534fb45 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Aug 2023 17:31:19 +0900 Subject: [PATCH 01/23] Split out difficulties in beatmap carousel in a bit of a hacky way Seems like the simplest path forward for now, without a full rewrite. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 82 ++++++++++++++++------ osu.Game/Screens/Select/FilterCriteria.cs | 5 ++ 2 files changed, 65 insertions(+), 22 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 9af9a0ce72..d1a9b4176b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -78,6 +78,8 @@ namespace osu.Game.Screens.Select private CarouselBeatmapSet? selectedBeatmapSet; + private IEnumerable originalBeatmapSetsDetached = Enumerable.Empty(); + /// /// Raised when the is changed. /// @@ -127,13 +129,29 @@ namespace osu.Game.Screens.Select private void loadBeatmapSets(IEnumerable beatmapSets) { + originalBeatmapSetsDetached = beatmapSets.Detach(); + CarouselRoot newRoot = new CarouselRoot(this); - newRoot.AddItems(beatmapSets.Select(s => createCarouselSet(s.Detach())).OfType()); + if (beatmapsSplitOut) + { + var carouselBeatmapSets = originalBeatmapSetsDetached.SelectMany(s => s.Beatmaps).Select(b => + { + var set = new BeatmapSetInfo(new[] { b }); + return createCarouselSet(set); + }).OfType(); + + newRoot.AddItems(carouselBeatmapSets); + } + else + { + var carouselBeatmapSets = originalBeatmapSetsDetached.Select(createCarouselSet).OfType(); + newRoot.AddItems(carouselBeatmapSets); + } root = newRoot; - if (selectedBeatmapSet != null && !beatmapSets.Contains(selectedBeatmapSet.BeatmapSet)) + if (selectedBeatmapSet != null && !originalBeatmapSetsDetached.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; Scroll.Clear(false); @@ -330,8 +348,8 @@ namespace osu.Game.Screens.Select // Only require to action here if the beatmap is missing. // This avoids processing these events unnecessarily when new beatmaps are imported, for example. - if (root.BeatmapSetsByID.TryGetValue(beatmapSet.ID, out var existingSet) - && existingSet.BeatmapSet.Beatmaps.All(b => b.ID != beatmapInfo.ID)) + if (root.BeatmapSetsByID.TryGetValue(beatmapSet.ID, out var existingSets) + && existingSets.SelectMany(s => s.Beatmaps).All(b => b.BeatmapInfo.ID != beatmapInfo.ID)) { UpdateBeatmapSet(beatmapSet.Detach()); } @@ -345,15 +363,18 @@ namespace osu.Game.Screens.Select private void removeBeatmapSet(Guid beatmapSetID) => Schedule(() => { - if (!root.BeatmapSetsByID.TryGetValue(beatmapSetID, out var existingSet)) + if (!root.BeatmapSetsByID.TryGetValue(beatmapSetID, out var existingSets)) return; - foreach (var beatmap in existingSet.Beatmaps) - randomSelectedBeatmaps.Remove(beatmap); + foreach (var set in existingSets) + { + foreach (var beatmap in set.Beatmaps) + randomSelectedBeatmaps.Remove(beatmap); + previouslyVisitedRandomSets.Remove(set); - previouslyVisitedRandomSets.Remove(existingSet); + root.RemoveItem(set); + } - root.RemoveItem(existingSet); itemsCache.Invalidate(); if (!Scroll.UserScrolling) @@ -371,13 +392,16 @@ namespace osu.Game.Screens.Select previouslySelectedID = selectedBeatmap?.BeatmapInfo.ID; var newSet = createCarouselSet(beatmapSet); - var removedSet = root.RemoveChild(beatmapSet.ID); + var removedSets = root.RemoveChild(beatmapSet.ID); - // If we don't remove this here, it may remain in a hidden state until scrolled off screen. - // Doesn't really affect anything during actual user interaction, but makes testing annoying. - var removedDrawable = Scroll.FirstOrDefault(c => c.Item == removedSet); - if (removedDrawable != null) - expirePanelImmediately(removedDrawable); + foreach (var removedSet in removedSets) + { + // If we don't remove this here, it may remain in a hidden state until scrolled off screen. + // Doesn't really affect anything during actual user interaction, but makes testing annoying. + var removedDrawable = Scroll.FirstOrDefault(c => c.Item == removedSet); + if (removedDrawable != null) + expirePanelImmediately(removedDrawable); + } if (newSet != null) { @@ -632,6 +656,8 @@ namespace osu.Game.Screens.Select applyActiveCriteria(debounce); } + private bool beatmapsSplitOut; + private void applyActiveCriteria(bool debounce, bool alwaysResetScrollPosition = true) { PendingFilter?.Cancel(); @@ -652,6 +678,13 @@ namespace osu.Game.Screens.Select { PendingFilter = null; + if (activeCriteria.SplitOutDifficulties != beatmapsSplitOut) + { + beatmapsSplitOut = activeCriteria.SplitOutDifficulties; + loadBeatmapSets(originalBeatmapSetsDetached); + return; + } + root.Filter(activeCriteria); itemsCache.Invalidate(); @@ -1055,7 +1088,7 @@ namespace osu.Game.Screens.Select // May only be null during construction (State.Value set causes PerformSelection to be triggered). private readonly BeatmapCarousel? carousel; - public readonly Dictionary BeatmapSetsByID = new Dictionary(); + public readonly Dictionary> BeatmapSetsByID = new Dictionary>(); public CarouselRoot(BeatmapCarousel carousel) { @@ -1069,20 +1102,25 @@ namespace osu.Game.Screens.Select public override void AddItem(CarouselItem i) { CarouselBeatmapSet set = (CarouselBeatmapSet)i; - BeatmapSetsByID.Add(set.BeatmapSet.ID, set); + if (BeatmapSetsByID.TryGetValue(set.BeatmapSet.ID, out var sets)) + sets.Add(set); + else + BeatmapSetsByID.Add(set.BeatmapSet.ID, new List { set }); base.AddItem(i); } - public CarouselBeatmapSet? RemoveChild(Guid beatmapSetID) + public IEnumerable RemoveChild(Guid beatmapSetID) { - if (BeatmapSetsByID.TryGetValue(beatmapSetID, out var carouselBeatmapSet)) + if (BeatmapSetsByID.TryGetValue(beatmapSetID, out var carouselBeatmapSets)) { - RemoveItem(carouselBeatmapSet); - return carouselBeatmapSet; + foreach (var set in carouselBeatmapSets) + RemoveItem(set); + + return carouselBeatmapSets; } - return null; + return Enumerable.Empty(); } public override void RemoveItem(CarouselItem i) diff --git a/osu.Game/Screens/Select/FilterCriteria.cs b/osu.Game/Screens/Select/FilterCriteria.cs index ab4f85fc92..a2ae114126 100644 --- a/osu.Game/Screens/Select/FilterCriteria.cs +++ b/osu.Game/Screens/Select/FilterCriteria.cs @@ -19,6 +19,11 @@ namespace osu.Game.Screens.Select public GroupMode Group; public SortMode Sort; + /// + /// Whether the display of beatmap sets should be split apart per-difficulty for the current criteria. + /// + public bool SplitOutDifficulties => Sort == SortMode.Difficulty; + public BeatmapSetInfo? SelectedBeatmapSet; public OptionalRange StarDifficulty; From 2b1c6ae612cb4e6cecbd6736748da942fa724e39 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Aug 2023 18:38:23 +0900 Subject: [PATCH 02/23] Ensure ID is maintained in temporary `BeatmapSetInfo`s --- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index d1a9b4176b..5157e37a31 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -137,8 +137,10 @@ namespace osu.Game.Screens.Select { var carouselBeatmapSets = originalBeatmapSetsDetached.SelectMany(s => s.Beatmaps).Select(b => { - var set = new BeatmapSetInfo(new[] { b }); - return createCarouselSet(set); + return createCarouselSet(new BeatmapSetInfo(new[] { b }) + { + ID = b.BeatmapSet!.ID, + }); }).OfType(); newRoot.AddItems(carouselBeatmapSets); From ecbf0f138e2c7b3cf5716020a1aad4f8a834dfd7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Aug 2023 18:38:43 +0900 Subject: [PATCH 03/23] Fix incorrect handling when new beatmaps arrive --- osu.Game/Screens/Select/BeatmapCarousel.cs | 35 ++++++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 5157e37a31..2227eb801a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -393,7 +393,6 @@ namespace osu.Game.Screens.Select if (selectedBeatmapSet?.BeatmapSet.ID == beatmapSet.ID) previouslySelectedID = selectedBeatmap?.BeatmapInfo.ID; - var newSet = createCarouselSet(beatmapSet); var removedSets = root.RemoveChild(beatmapSet.ID); foreach (var removedSet in removedSets) @@ -405,13 +404,37 @@ namespace osu.Game.Screens.Select expirePanelImmediately(removedDrawable); } - if (newSet != null) + if (beatmapsSplitOut) { - root.AddItem(newSet); + foreach (var beatmap in beatmapSet.Beatmaps) + { + var newSet = createCarouselSet(new BeatmapSetInfo(new[] { beatmap }) + { + ID = beatmapSet.ID + }); - // check if we can/need to maintain our current selection. - if (previouslySelectedID != null) - select((CarouselItem?)newSet.Beatmaps.FirstOrDefault(b => b.BeatmapInfo.ID == previouslySelectedID) ?? newSet); + if (newSet != null) + { + root.AddItem(newSet); + + // check if we can/need to maintain our current selection. + if (previouslySelectedID != null) + select((CarouselItem?)newSet.Beatmaps.FirstOrDefault(b => b.BeatmapInfo.ID == previouslySelectedID) ?? newSet); + } + } + } + else + { + var newSet = createCarouselSet(beatmapSet); + + if (newSet != null) + { + root.AddItem(newSet); + + // check if we can/need to maintain our current selection. + if (previouslySelectedID != null) + select((CarouselItem?)newSet.Beatmaps.FirstOrDefault(b => b.BeatmapInfo.ID == previouslySelectedID) ?? newSet); + } } itemsCache.Invalidate(); From 018be4c20f408ee98bb00a3897e2658b4f016596 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 22 Aug 2023 18:40:34 +0900 Subject: [PATCH 04/23] Fix selection not being retained when switching between split mode --- osu.Game/Screens/Select/BeatmapCarousel.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 2227eb801a..c5e46a00b6 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -131,6 +131,12 @@ namespace osu.Game.Screens.Select { originalBeatmapSetsDetached = beatmapSets.Detach(); + if (selectedBeatmapSet != null && !originalBeatmapSetsDetached.Contains(selectedBeatmapSet.BeatmapSet)) + selectedBeatmapSet = null; + + var selectedSetBefore = selectedBeatmapSet; + var selectedBetmapBefore = selectedBeatmap; + CarouselRoot newRoot = new CarouselRoot(this); if (beatmapsSplitOut) @@ -148,14 +154,12 @@ namespace osu.Game.Screens.Select else { var carouselBeatmapSets = originalBeatmapSetsDetached.Select(createCarouselSet).OfType(); + newRoot.AddItems(carouselBeatmapSets); } root = newRoot; - if (selectedBeatmapSet != null && !originalBeatmapSetsDetached.Contains(selectedBeatmapSet.BeatmapSet)) - selectedBeatmapSet = null; - Scroll.Clear(false); itemsCache.Invalidate(); ScrollToSelected(); @@ -164,6 +168,15 @@ namespace osu.Game.Screens.Select if (loadedTestBeatmaps) signalBeatmapsLoaded(); + + // Restore selection + if (selectedBetmapBefore != null && selectedSetBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedSetBefore.BeatmapSet.ID, out var newSelectionCandidates)) + { + CarouselBeatmap? found = newSelectionCandidates.SelectMany(s => s.Beatmaps).SingleOrDefault(b => b.BeatmapInfo.ID == selectedBetmapBefore.BeatmapInfo.ID); + + if (found != null) + found.State.Value = CarouselItemState.Selected; + } } private readonly List visibleItems = new List(); From 5555f73e97f22f9c6f0425bc4c9459bef88f3be5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Aug 2023 19:38:18 +0900 Subject: [PATCH 05/23] Update test to match new behaviour --- .../SongSelect/TestSceneBeatmapCarousel.cs | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 61f95dc628..b0aff8b4db 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -758,7 +758,7 @@ namespace osu.Game.Tests.Visual.SongSelect } [Test] - public void TestSortingWithFiltered() + public void TestSortingWithDifficultyFiltered() { List sets = new List(); @@ -777,13 +777,32 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); + AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + + checkVisibleItemCount(false, 9); + checkVisibleItemCount(true, 1); + AddStep("Filter to normal", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Normal" }, false)); - AddAssert("Check first set at end", () => carousel.BeatmapSets.First().Equals(sets.Last())); - AddAssert("Check last set at start", () => carousel.BeatmapSets.Last().Equals(sets.First())); + checkVisibleItemCount(false, 3); + checkVisibleItemCount(true, 1); + + AddUntilStep("Check all visible sets have one normal", () => + { + return carousel.Items.OfType() + .Where(p => p.IsPresent) + .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Normal", StringComparison.Ordinal)) == 3; + }); AddStep("Filter to insane", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Insane" }, false)); - AddAssert("Check first set at start", () => carousel.BeatmapSets.First().Equals(sets.First())); - AddAssert("Check last set at end", () => carousel.BeatmapSets.Last().Equals(sets.Last())); + checkVisibleItemCount(false, 3); + checkVisibleItemCount(true, 1); + + AddUntilStep("Check all visible sets have one insane", () => + { + return carousel.Items.OfType() + .Where(p => p.IsPresent) + .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Insane", StringComparison.Ordinal)) == 3; + }); } [Test] From a64381f8553dd49b9c5c5142aa2934c333b101a4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Aug 2023 19:43:08 +0900 Subject: [PATCH 06/23] Add test coverage of add/remove when difficulties are split out --- .../SongSelect/TestSceneBeatmapCarousel.cs | 47 +++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index b0aff8b4db..5af6d862b2 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -39,6 +39,7 @@ namespace osu.Game.Tests.Visual.SongSelect private BeatmapInfo currentSelection => carousel.SelectedBeatmapInfo; private const int set_count = 5; + private const int diff_count = 3; [BackgroundDependencyLoader] private void load(RulesetStore rulesets) @@ -501,6 +502,36 @@ namespace osu.Game.Tests.Visual.SongSelect waitForSelection(set_count); } + [Test] + public void TestAddRemoveDifficultySort() + { + loadBeatmaps(); + + AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + + checkVisibleItemCount(false, set_count * diff_count); + + var firstAdded = TestResources.CreateTestBeatmapSetInfo(diff_count); + var secondAdded = TestResources.CreateTestBeatmapSetInfo(diff_count); + + AddStep("Add new set", () => carousel.UpdateBeatmapSet(firstAdded)); + AddStep("Add new set", () => carousel.UpdateBeatmapSet(secondAdded)); + + checkVisibleItemCount(false, (set_count + 2) * diff_count); + + AddStep("Remove set", () => carousel.RemoveBeatmapSet(firstAdded)); + + checkVisibleItemCount(false, (set_count + 1) * diff_count); + + setSelected(set_count + 1, 1); + + AddStep("Remove set", () => carousel.RemoveBeatmapSet(secondAdded)); + + checkVisibleItemCount(false, (set_count) * diff_count); + + waitForSelection(set_count); + } + [Test] public void TestSelectionEnteringFromEmptyRuleset() { @@ -662,7 +693,7 @@ namespace osu.Game.Tests.Visual.SongSelect for (int i = 0; i < 3; i++) { - var set = TestResources.CreateTestBeatmapSetInfo(3); + var set = TestResources.CreateTestBeatmapSetInfo(diff_count); // only need to set the first as they are a shared reference. var beatmap = set.Beatmaps.First(); @@ -709,7 +740,7 @@ namespace osu.Game.Tests.Visual.SongSelect for (int i = 0; i < 3; i++) { - var set = TestResources.CreateTestBeatmapSetInfo(3); + var set = TestResources.CreateTestBeatmapSetInfo(diff_count); // only need to set the first as they are a shared reference. var beatmap = set.Beatmaps.First(); @@ -768,7 +799,7 @@ namespace osu.Game.Tests.Visual.SongSelect for (int i = 0; i < 3; i++) { - var set = TestResources.CreateTestBeatmapSetInfo(3); + var set = TestResources.CreateTestBeatmapSetInfo(diff_count); set.Beatmaps[0].StarRating = 3 - i; set.Beatmaps[2].StarRating = 6 + i; sets.Add(set); @@ -857,7 +888,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("create hidden set", () => { - hidingSet = TestResources.CreateTestBeatmapSetInfo(3); + hidingSet = TestResources.CreateTestBeatmapSetInfo(diff_count); hidingSet.Beatmaps[1].Hidden = true; hiddenList.Clear(); @@ -904,7 +935,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("add mixed ruleset beatmapset", () => { - testMixed = TestResources.CreateTestBeatmapSetInfo(3); + testMixed = TestResources.CreateTestBeatmapSetInfo(diff_count); for (int i = 0; i <= 2; i++) { @@ -926,7 +957,7 @@ namespace osu.Game.Tests.Visual.SongSelect BeatmapSetInfo testSingle = null; AddStep("add single ruleset beatmapset", () => { - testSingle = TestResources.CreateTestBeatmapSetInfo(3); + testSingle = TestResources.CreateTestBeatmapSetInfo(diff_count); testSingle.Beatmaps.ForEach(b => { b.Ruleset = rulesets.AvailableRulesets.ElementAt(1); @@ -949,7 +980,7 @@ namespace osu.Game.Tests.Visual.SongSelect manySets.Clear(); for (int i = 1; i <= 50; i++) - manySets.Add(TestResources.CreateTestBeatmapSetInfo(3)); + manySets.Add(TestResources.CreateTestBeatmapSetInfo(diff_count)); }); loadBeatmaps(manySets); @@ -1113,7 +1144,7 @@ namespace osu.Game.Tests.Visual.SongSelect { beatmapSets.Add(randomDifficulties ? TestResources.CreateTestBeatmapSetInfo() - : TestResources.CreateTestBeatmapSetInfo(3)); + : TestResources.CreateTestBeatmapSetInfo(diff_count)); } } From 5eac604f8b2cf59a5346956d99a7fea0fdda0cd9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 23 Aug 2023 19:44:39 +0900 Subject: [PATCH 07/23] Add coverage of selection retention when difficulties are split out --- .../SongSelect/TestSceneBeatmapCarousel.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 5af6d862b2..daa8c9c4c2 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -1005,6 +1005,43 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("Selection was remembered", () => eagerSelectedIDs.Count == 1); } + [Test] + public void TestCarouselRemembersSelectionDifficultySort() + { + List manySets = new List(); + + AddStep("Populuate beatmap sets", () => + { + manySets.Clear(); + + for (int i = 1; i <= 50; i++) + manySets.Add(TestResources.CreateTestBeatmapSetInfo(diff_count)); + }); + + loadBeatmaps(manySets); + + AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + + advanceSelection(direction: 1, diff: false); + + for (int i = 0; i < 5; i++) + { + AddStep("Toggle non-matching filter", () => + { + carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false); + }); + + AddStep("Restore no filter", () => + { + carousel.Filter(new FilterCriteria(), false); + eagerSelectedIDs.Add(carousel.SelectedBeatmapSet!.ID); + }); + } + + // always returns to same selection as long as it's available. + AddAssert("Selection was remembered", () => eagerSelectedIDs.Count == 1); + } + [Test] public void TestFilteringByUserStarDifficulty() { From 4881130caeccc728dbb789665fd161da1cfb0b53 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Aug 2023 03:32:12 +0900 Subject: [PATCH 08/23] Limit set/diff count in test to better fit on screen --- .../SongSelect/TestSceneBeatmapCarousel.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index daa8c9c4c2..d68fa6100c 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -791,17 +791,20 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestSortingWithDifficultyFiltered() { + const int local_diff_count = 3; + const int local_set_count = 2; + List sets = new List(); AddStep("Populuate beatmap sets", () => { sets.Clear(); - for (int i = 0; i < 3; i++) + for (int i = 0; i < local_set_count; i++) { - var set = TestResources.CreateTestBeatmapSetInfo(diff_count); + var set = TestResources.CreateTestBeatmapSetInfo(local_diff_count); set.Beatmaps[0].StarRating = 3 - i; - set.Beatmaps[2].StarRating = 6 + i; + set.Beatmaps[1].StarRating = 6 + i; sets.Add(set); } }); @@ -810,29 +813,29 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); - checkVisibleItemCount(false, 9); + checkVisibleItemCount(false, local_set_count * local_diff_count); checkVisibleItemCount(true, 1); AddStep("Filter to normal", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Normal" }, false)); - checkVisibleItemCount(false, 3); + checkVisibleItemCount(false, local_set_count); checkVisibleItemCount(true, 1); AddUntilStep("Check all visible sets have one normal", () => { return carousel.Items.OfType() .Where(p => p.IsPresent) - .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Normal", StringComparison.Ordinal)) == 3; + .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Normal", StringComparison.Ordinal)) == local_set_count; }); AddStep("Filter to insane", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Insane" }, false)); - checkVisibleItemCount(false, 3); + checkVisibleItemCount(false, local_set_count); checkVisibleItemCount(true, 1); AddUntilStep("Check all visible sets have one insane", () => { return carousel.Items.OfType() .Where(p => p.IsPresent) - .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Insane", StringComparison.Ordinal)) == 3; + .Count(p => ((CarouselBeatmapSet)p.Item)!.Beatmaps.Single().BeatmapInfo.DifficultyName.StartsWith("Insane", StringComparison.Ordinal)) == local_set_count; }); } From ba70d48d2cc0013fbc42338e0867cbe1e6ea8c82 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Aug 2023 13:31:33 +0900 Subject: [PATCH 09/23] Fix one more test probably going off-screen --- .../SongSelect/TestSceneBeatmapCarousel.cs | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index d68fa6100c..8a38e2a84b 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -112,7 +112,7 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestScrollPositionMaintainedOnAdd() { - loadBeatmaps(count: 1, randomDifficulties: false); + loadBeatmaps(setCount: 1); for (int i = 0; i < 10; i++) { @@ -125,7 +125,7 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestDeletion() { - loadBeatmaps(count: 5, randomDifficulties: true); + loadBeatmaps(setCount: 5, randomDifficulties: true); AddStep("remove first set", () => carousel.RemoveBeatmapSet(carousel.Items.Select(item => item.Item).OfType().First().BeatmapSet)); AddUntilStep("4 beatmap sets visible", () => this.ChildrenOfType().Count(set => set.Alpha > 0) == 4); @@ -134,7 +134,7 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestScrollPositionMaintainedOnDelete() { - loadBeatmaps(count: 50, randomDifficulties: false); + loadBeatmaps(setCount: 50); for (int i = 0; i < 10; i++) { @@ -151,7 +151,7 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestManyPanels() { - loadBeatmaps(count: 5000, randomDifficulties: true); + loadBeatmaps(setCount: 5000, randomDifficulties: true); } [Test] @@ -505,31 +505,34 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestAddRemoveDifficultySort() { - loadBeatmaps(); + const int local_set_count = 1; + const int local_diff_count = 1; + + loadBeatmaps(setCount: local_set_count, diffCount: local_diff_count); AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); - checkVisibleItemCount(false, set_count * diff_count); + checkVisibleItemCount(false, local_set_count * local_diff_count); - var firstAdded = TestResources.CreateTestBeatmapSetInfo(diff_count); - var secondAdded = TestResources.CreateTestBeatmapSetInfo(diff_count); + var firstAdded = TestResources.CreateTestBeatmapSetInfo(local_diff_count); + var secondAdded = TestResources.CreateTestBeatmapSetInfo(local_diff_count); AddStep("Add new set", () => carousel.UpdateBeatmapSet(firstAdded)); AddStep("Add new set", () => carousel.UpdateBeatmapSet(secondAdded)); - checkVisibleItemCount(false, (set_count + 2) * diff_count); + checkVisibleItemCount(false, (local_set_count + 2) * local_diff_count); AddStep("Remove set", () => carousel.RemoveBeatmapSet(firstAdded)); - checkVisibleItemCount(false, (set_count + 1) * diff_count); + checkVisibleItemCount(false, (local_set_count + 1) * local_diff_count); - setSelected(set_count + 1, 1); + setSelected(local_set_count + 1, 1); AddStep("Remove set", () => carousel.RemoveBeatmapSet(secondAdded)); - checkVisibleItemCount(false, (set_count) * diff_count); + checkVisibleItemCount(false, (local_set_count) * local_diff_count); - waitForSelection(set_count); + waitForSelection(local_set_count); } [Test] @@ -1171,8 +1174,8 @@ namespace osu.Game.Tests.Visual.SongSelect } } - private void loadBeatmaps(List beatmapSets = null, Func initialCriteria = null, Action carouselAdjust = null, int? count = null, - bool randomDifficulties = false) + private void loadBeatmaps(List beatmapSets = null, Func initialCriteria = null, Action carouselAdjust = null, + int? setCount = null, int? diffCount = null, bool randomDifficulties = false) { bool changed = false; @@ -1180,11 +1183,11 @@ namespace osu.Game.Tests.Visual.SongSelect { beatmapSets = new List(); - for (int i = 1; i <= (count ?? set_count); i++) + for (int i = 1; i <= (setCount ?? set_count); i++) { beatmapSets.Add(randomDifficulties ? TestResources.CreateTestBeatmapSetInfo() - : TestResources.CreateTestBeatmapSetInfo(diff_count)); + : TestResources.CreateTestBeatmapSetInfo(diffCount ?? diff_count)); } } From b471ab07a62dc225275154efdfd15c46646f0104 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Aug 2023 18:30:59 +0900 Subject: [PATCH 10/23] Fix typo in test step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 8a38e2a84b..110466f6d5 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -1016,7 +1016,7 @@ namespace osu.Game.Tests.Visual.SongSelect { List manySets = new List(); - AddStep("Populuate beatmap sets", () => + AddStep("Populate beatmap sets", () => { manySets.Clear(); From 9e94f3809103b6079a158e19050f9bc15829235a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 24 Aug 2023 18:33:15 +0900 Subject: [PATCH 11/23] Fix typo in local variable --- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index c5e46a00b6..91d1ced9d9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -135,7 +135,7 @@ namespace osu.Game.Screens.Select selectedBeatmapSet = null; var selectedSetBefore = selectedBeatmapSet; - var selectedBetmapBefore = selectedBeatmap; + var selectedBeatmapBefore = selectedBeatmap; CarouselRoot newRoot = new CarouselRoot(this); @@ -170,9 +170,9 @@ namespace osu.Game.Screens.Select signalBeatmapsLoaded(); // Restore selection - if (selectedBetmapBefore != null && selectedSetBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedSetBefore.BeatmapSet.ID, out var newSelectionCandidates)) + if (selectedBeatmapBefore != null && selectedSetBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedSetBefore.BeatmapSet.ID, out var newSelectionCandidates)) { - CarouselBeatmap? found = newSelectionCandidates.SelectMany(s => s.Beatmaps).SingleOrDefault(b => b.BeatmapInfo.ID == selectedBetmapBefore.BeatmapInfo.ID); + CarouselBeatmap? found = newSelectionCandidates.SelectMany(s => s.Beatmaps).SingleOrDefault(b => b.BeatmapInfo.ID == selectedBeatmapBefore.BeatmapInfo.ID); if (found != null) found.State.Value = CarouselItemState.Selected; From 89eeff515b42a9724ffd2e552f9e7d462f848d5a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Aug 2023 00:52:54 +0900 Subject: [PATCH 12/23] Reduce complexity of selection restore --- osu.Game/Screens/Select/BeatmapCarousel.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 91d1ced9d9..d52b5592f8 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -134,8 +134,7 @@ namespace osu.Game.Screens.Select if (selectedBeatmapSet != null && !originalBeatmapSetsDetached.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; - var selectedSetBefore = selectedBeatmapSet; - var selectedBeatmapBefore = selectedBeatmap; + var selectedBeatmapBefore = selectedBeatmap?.BeatmapInfo; CarouselRoot newRoot = new CarouselRoot(this); @@ -170,9 +169,9 @@ namespace osu.Game.Screens.Select signalBeatmapsLoaded(); // Restore selection - if (selectedBeatmapBefore != null && selectedSetBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedSetBefore.BeatmapSet.ID, out var newSelectionCandidates)) + if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) { - CarouselBeatmap? found = newSelectionCandidates.SelectMany(s => s.Beatmaps).SingleOrDefault(b => b.BeatmapInfo.ID == selectedBeatmapBefore.BeatmapInfo.ID); + CarouselBeatmap? found = newSelectionCandidates.SelectMany(s => s.Beatmaps).SingleOrDefault(b => b.BeatmapInfo.ID == selectedBeatmapBefore.ID); if (found != null) found.State.Value = CarouselItemState.Selected; From 84f4fab9cfad4a122a271977ed7902f9e504904b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Aug 2023 18:09:51 +0900 Subject: [PATCH 13/23] Adjust test to actually test diff splitting --- .../SongSelect/TestSceneBeatmapCarousel.cs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 110466f6d5..040b341584 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -505,8 +505,8 @@ namespace osu.Game.Tests.Visual.SongSelect [Test] public void TestAddRemoveDifficultySort() { - const int local_set_count = 1; - const int local_diff_count = 1; + const int local_set_count = 2; + const int local_diff_count = 2; loadBeatmaps(setCount: local_set_count, diffCount: local_diff_count); @@ -515,23 +515,17 @@ namespace osu.Game.Tests.Visual.SongSelect checkVisibleItemCount(false, local_set_count * local_diff_count); var firstAdded = TestResources.CreateTestBeatmapSetInfo(local_diff_count); - var secondAdded = TestResources.CreateTestBeatmapSetInfo(local_diff_count); AddStep("Add new set", () => carousel.UpdateBeatmapSet(firstAdded)); - AddStep("Add new set", () => carousel.UpdateBeatmapSet(secondAdded)); - - checkVisibleItemCount(false, (local_set_count + 2) * local_diff_count); - - AddStep("Remove set", () => carousel.RemoveBeatmapSet(firstAdded)); checkVisibleItemCount(false, (local_set_count + 1) * local_diff_count); - setSelected(local_set_count + 1, 1); - - AddStep("Remove set", () => carousel.RemoveBeatmapSet(secondAdded)); + AddStep("Remove set", () => carousel.RemoveBeatmapSet(firstAdded)); checkVisibleItemCount(false, (local_set_count) * local_diff_count); + setSelected(local_set_count, 1); + waitForSelection(local_set_count); } From 10b14501380320939f43fe715b033970b8af4a2e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 25 Aug 2023 18:10:54 +0900 Subject: [PATCH 14/23] Rename remove method to better explain return type being `IEnumerable` --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index d52b5592f8..a8da5dc742 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -405,7 +405,7 @@ namespace osu.Game.Screens.Select if (selectedBeatmapSet?.BeatmapSet.ID == beatmapSet.ID) previouslySelectedID = selectedBeatmap?.BeatmapInfo.ID; - var removedSets = root.RemoveChild(beatmapSet.ID); + var removedSets = root.RemoveItemsByID(beatmapSet.ID); foreach (var removedSet in removedSets) { @@ -1147,7 +1147,7 @@ namespace osu.Game.Screens.Select base.AddItem(i); } - public IEnumerable RemoveChild(Guid beatmapSetID) + public IEnumerable RemoveItemsByID(Guid beatmapSetID) { if (BeatmapSetsByID.TryGetValue(beatmapSetID, out var carouselBeatmapSets)) { From 3a6920c3067f90c53578d021f3ecf7e89d368c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 09:51:37 +0200 Subject: [PATCH 15/23] Add failing test coverage of beatmap update flow w/ split diffs --- .../TestSceneUpdateBeatmapSetButton.cs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs index 11d55bc0bd..585d93d3af 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs @@ -15,6 +15,7 @@ using osu.Game.Overlays; using osu.Game.Overlays.Dialog; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; +using osu.Game.Screens.Select.Filter; using osu.Game.Tests.Online; using osu.Game.Tests.Resources; using osuTK.Input; @@ -192,6 +193,57 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("release mouse button", () => InputManager.ReleaseButton(MouseButton.Left)); } + [Test] + public void TestSplitDisplay() + { + ArchiveDownloadRequest? downloadRequest = null; + + AddStep("set difficulty sort mode", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty })); + AddStep("update online hash", () => + { + testBeatmapSetInfo.Beatmaps.First().OnlineMD5Hash = "different hash"; + testBeatmapSetInfo.Beatmaps.First().LastOnlineUpdate = DateTimeOffset.Now; + + carousel.UpdateBeatmapSet(testBeatmapSetInfo); + }); + + AddUntilStep("multiple \"sets\" visible", () => carousel.ChildrenOfType().Count(), () => Is.GreaterThan(1)); + AddUntilStep("update button visible", getUpdateButton, () => Is.Not.Null); + + AddStep("click button", () => getUpdateButton()?.TriggerClick()); + + AddUntilStep("wait for download started", () => + { + downloadRequest = beatmapDownloader.GetExistingDownload(testBeatmapSetInfo); + return downloadRequest != null; + }); + + AddUntilStep("wait for button disabled", () => getUpdateButton()?.Enabled.Value == false); + + AddUntilStep("progress download to completion", () => + { + if (downloadRequest is TestSceneOnlinePlayBeatmapAvailabilityTracker.TestDownloadRequest testRequest) + { + testRequest.SetProgress(testRequest.Progress + 0.1f); + + if (testRequest.Progress >= 1) + { + testRequest.TriggerSuccess(); + + // usually this would be done by the import process. + testBeatmapSetInfo.Beatmaps.First().MD5Hash = "different hash"; + testBeatmapSetInfo.Beatmaps.First().LastOnlineUpdate = DateTimeOffset.Now; + + // usually this would be done by a realm subscription. + carousel.UpdateBeatmapSet(testBeatmapSetInfo); + return true; + } + } + + return false; + }); + } + private BeatmapCarousel createCarousel() { return carousel = new BeatmapCarousel From 6251803868588f188ca7b78f74ad4c9e18d4b10b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 10:02:05 +0200 Subject: [PATCH 16/23] Add failing test coverage of selection not being retained on song select --- .../TestSceneBeatmapEditorNavigation.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs index 54ee1659e1..d0fa5fc737 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs @@ -9,6 +9,7 @@ using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; +using osu.Game.Configuration; using osu.Game.Database; using osu.Game.Rulesets.Mania; using osu.Game.Rulesets.Osu; @@ -16,6 +17,7 @@ using osu.Game.Screens.Edit; using osu.Game.Screens.Edit.GameplayTest; using osu.Game.Screens.Menu; using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Filter; using osu.Game.Tests.Resources; using osuTK.Input; @@ -203,6 +205,33 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for music stopped", () => !Game.MusicController.IsPlaying); } + [TestCase(SortMode.Title)] + [TestCase(SortMode.Difficulty)] + public void TestSelectionRetainedOnExit(SortMode sortMode) + { + BeatmapSetInfo beatmapSet = null!; + + AddStep("import test beatmap", () => Game.BeatmapManager.Import(TestResources.GetTestBeatmapForImport()).WaitSafely()); + AddStep("retrieve beatmap", () => beatmapSet = Game.BeatmapManager.QueryBeatmapSet(set => !set.Protected).AsNonNull().Value.Detach()); + + AddStep($"set sort mode to {sortMode}", () => Game.LocalConfig.SetValue(OsuSetting.SongSelectSortingMode, sortMode)); + AddStep("present beatmap", () => Game.PresentBeatmap(beatmapSet)); + AddUntilStep("wait for song select", + () => Game.Beatmap.Value.BeatmapSetInfo.Equals(beatmapSet) + && Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect + && songSelect.IsLoaded); + + AddStep("open editor", () => ((PlaySongSelect)Game.ScreenStack.CurrentScreen).Edit(beatmapSet.Beatmaps.First(beatmap => beatmap.Ruleset.OnlineID == 0))); + AddUntilStep("wait for editor open", () => Game.ScreenStack.CurrentScreen is Editor editor && editor.ReadyForUse); + + AddStep("exit editor", () => InputManager.Key(Key.Escape)); + AddUntilStep("wait for editor exit", () => Game.ScreenStack.CurrentScreen is not Editor); + + AddUntilStep("selection retained on song select", + () => Game.Beatmap.Value.BeatmapInfo.ID, + () => Is.EqualTo(beatmapSet.Beatmaps.First(b => b.Ruleset.OnlineID == 0).ID)); + } + private EditorBeatmap getEditorBeatmap() => getEditor().ChildrenOfType().Single(); private Editor getEditor() => (Editor)Game.ScreenStack.CurrentScreen; From 0af6cc1394f7fe521bdf94773b423b5ff61c2bb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 10:02:30 +0200 Subject: [PATCH 17/23] Fix online ID not being propagated in split difficulty mode Would result in failures to re-download the beatmap in update flows, for instance. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index a8da5dc742..0354e72803 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -145,6 +145,7 @@ namespace osu.Game.Screens.Select return createCarouselSet(new BeatmapSetInfo(new[] { b }) { ID = b.BeatmapSet!.ID, + OnlineID = b.BeatmapSet!.OnlineID }); }).OfType(); @@ -422,7 +423,8 @@ namespace osu.Game.Screens.Select { var newSet = createCarouselSet(new BeatmapSetInfo(new[] { beatmap }) { - ID = beatmapSet.ID + ID = beatmapSet.ID, + OnlineID = beatmapSet.OnlineID }); if (newSet != null) From 80ec18d117fc7ae252731bcde4e65a71be838ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 10:06:26 +0200 Subject: [PATCH 18/23] Fix incorrect selection restore code in split case The fallback to "any of the added sets" needs to be applied after they've all been added, rather than with every added one. Otherwise, in flows that expect a particular difficulty to be selected in the end (such as exiting from editor) would end up switching away from the edited beatmap. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 0354e72803..861385f453 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -419,6 +419,8 @@ namespace osu.Game.Screens.Select if (beatmapsSplitOut) { + var newSets = new List(); + foreach (var beatmap in beatmapSet.Beatmaps) { var newSet = createCarouselSet(new BeatmapSetInfo(new[] { beatmap }) @@ -429,13 +431,18 @@ namespace osu.Game.Screens.Select if (newSet != null) { + newSets.Add(newSet); root.AddItem(newSet); - - // check if we can/need to maintain our current selection. - if (previouslySelectedID != null) - select((CarouselItem?)newSet.Beatmaps.FirstOrDefault(b => b.BeatmapInfo.ID == previouslySelectedID) ?? newSet); } } + + // check if we can/need to maintain our current selection. + if (previouslySelectedID != null) + { + var toSelect = newSets.FirstOrDefault(s => s.Beatmaps.Any(b => b.BeatmapInfo.ID == previouslySelectedID)) + ?? newSets.FirstOrDefault(); + select(toSelect); + } } else { From b9795eb3d4e09d66c9e2950da6e8742637dd8dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 11:02:22 +0200 Subject: [PATCH 19/23] Fix changes to beatmap sets being undone on switching sort mode --- osu.Game/Screens/Select/BeatmapCarousel.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 861385f453..9ea94b2a52 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -78,7 +78,7 @@ namespace osu.Game.Screens.Select private CarouselBeatmapSet? selectedBeatmapSet; - private IEnumerable originalBeatmapSetsDetached = Enumerable.Empty(); + private List originalBeatmapSetsDetached = new List(); /// /// Raised when the is changed. @@ -381,6 +381,8 @@ namespace osu.Game.Screens.Select if (!root.BeatmapSetsByID.TryGetValue(beatmapSetID, out var existingSets)) return; + originalBeatmapSetsDetached.RemoveAll(set => set.ID == beatmapSetID); + foreach (var set in existingSets) { foreach (var beatmap in set.Beatmaps) @@ -402,6 +404,9 @@ namespace osu.Game.Screens.Select { Guid? previouslySelectedID = null; + originalBeatmapSetsDetached.RemoveAll(set => set.ID == beatmapSet.ID); + originalBeatmapSetsDetached.Add(beatmapSet.Detach()); + // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required if (selectedBeatmapSet?.BeatmapSet.ID == beatmapSet.ID) previouslySelectedID = selectedBeatmap?.BeatmapInfo.ID; From 118c86df342340da729c3931a4c58a1ed72e0f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 28 Aug 2023 14:57:00 +0200 Subject: [PATCH 20/23] Fix `TestSceneUpdateBeatmapSetButton` using random difficulty count --- .../Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs index 585d93d3af..6d97be730b 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs @@ -251,7 +251,7 @@ namespace osu.Game.Tests.Visual.SongSelect RelativeSizeAxes = Axes.Both, BeatmapSets = new List { - (testBeatmapSetInfo = TestResources.CreateTestBeatmapSetInfo()), + (testBeatmapSetInfo = TestResources.CreateTestBeatmapSetInfo(5)), } }; } From b658b0e3462170adcb3a57901de17ee514a8d9c1 Mon Sep 17 00:00:00 2001 From: Nabile Rahmani Date: Tue, 29 Aug 2023 23:28:50 +0200 Subject: [PATCH 21/23] Fix and use score user's IsBot property in results screen animation While a mod-created replay did flag itself as performed by a bot, the extension method converting it into a Score did not copy all the generated properties. As noted, it might be preferable for ModCreatedUser to inherit APIUser and forward it as-is to the Score instance. Related to PR #24675 --- osu.Game/Rulesets/Mods/ModExtensions.cs | 4 +++- osu.Game/Screens/Ranking/ResultsScreen.cs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/Mods/ModExtensions.cs b/osu.Game/Rulesets/Mods/ModExtensions.cs index b22030414b..49a3c9178a 100644 --- a/osu.Game/Rulesets/Mods/ModExtensions.cs +++ b/osu.Game/Rulesets/Mods/ModExtensions.cs @@ -21,8 +21,10 @@ namespace osu.Game.Rulesets.Mods { User = new APIUser { - Id = APIUser.SYSTEM_USER_ID, + // TODO: Some fields weren't copied from replayData.User (namely IsBot and Id). Should ModCreatedUser inherit from APIUser so we could pass it verbatim to avoid future mistakes ? + Id = replayData.User.OnlineID, Username = replayData.User.Username, + IsBot = replayData.User.IsBot, } } }; diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index 1d0499c909..03231f9329 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -156,7 +156,7 @@ namespace osu.Game.Screens.Ranking if (Score != null) { // only show flair / animation when arriving after watching a play that isn't autoplay. - bool shouldFlair = player != null && Score.Mods.All(m => m.UserPlayable); + bool shouldFlair = player != null && !Score.User.IsBot; ScorePanelList.AddScore(Score, shouldFlair); } From 270e2a66009b7b551837ad39ed45be9d3cdbf3b9 Mon Sep 17 00:00:00 2001 From: Nabile Rahmani Date: Tue, 29 Aug 2023 23:59:08 +0200 Subject: [PATCH 22/23] Update osu.Game/Rulesets/Mods/ModExtensions.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game/Rulesets/Mods/ModExtensions.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/Mods/ModExtensions.cs b/osu.Game/Rulesets/Mods/ModExtensions.cs index 49a3c9178a..aa106f1203 100644 --- a/osu.Game/Rulesets/Mods/ModExtensions.cs +++ b/osu.Game/Rulesets/Mods/ModExtensions.cs @@ -21,7 +21,6 @@ namespace osu.Game.Rulesets.Mods { User = new APIUser { - // TODO: Some fields weren't copied from replayData.User (namely IsBot and Id). Should ModCreatedUser inherit from APIUser so we could pass it verbatim to avoid future mistakes ? Id = replayData.User.OnlineID, Username = replayData.User.Username, IsBot = replayData.User.IsBot, From a85f0d57911fd880bb7a406cd5bd82d3934f0b50 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 30 Aug 2023 13:29:08 +0900 Subject: [PATCH 23/23] Allow saving changes in tournament system using `Ctrl`+`S` --- osu.Game.Tournament/SaveChangesOverlay.cs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tournament/SaveChangesOverlay.cs b/osu.Game.Tournament/SaveChangesOverlay.cs index 7838d4ba48..0b5194a51f 100644 --- a/osu.Game.Tournament/SaveChangesOverlay.cs +++ b/osu.Game.Tournament/SaveChangesOverlay.cs @@ -7,13 +7,16 @@ using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; +using osu.Framework.Input; +using osu.Framework.Input.Bindings; +using osu.Framework.Input.Events; using osu.Game.Graphics; using osu.Game.Online.Multiplayer; using osuTK; namespace osu.Game.Tournament { - internal partial class SaveChangesOverlay : CompositeDrawable + internal partial class SaveChangesOverlay : CompositeDrawable, IKeyBindingHandler { [Resolved] private TournamentGame tournamentGame { get; set; } = null!; @@ -78,6 +81,21 @@ namespace osu.Game.Tournament scheduleNextCheck(); } + public bool OnPressed(KeyBindingPressEvent e) + { + if (e.Action == PlatformAction.Save && !e.Repeat) + { + saveChangesButton.TriggerClick(); + return true; + } + + return false; + } + + public void OnReleased(KeyBindingReleaseEvent e) + { + } + private void scheduleNextCheck() => Scheduler.AddDelayed(() => checkForChanges().FireAndForget(), 1000); private void saveChanges()