From e59a00ac6eb1f30cb368392aedb299177fd8a168 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 14:04:33 +0900 Subject: [PATCH 01/11] Remove excessive selection updating --- .../SongSelect/TestScenePlaySongSelect.cs | 28 ++++++++++++++++++- osu.Game/Screens/Select/SongSelect.cs | 8 +++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 7e962dbc06..85811e3a0a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -209,7 +209,33 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("start not requested", () => !startRequested); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + [Test] + public void TestAddNewBeatmap() + { + const int test_count = 10; + int beatmapChangedCount = 0; + createSongSelect(); + AddStep("Setup counter", () => + { + beatmapChangedCount = 0; + songSelect.Carousel.BeatmapSetsChanged += () => beatmapChangedCount++; + }); + AddRepeatStep($"Create beatmaps {test_count} times", () => + { + manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == 0).ToArray())); + + Scheduler.AddDelayed(() => + { + // Wait for debounce + songSelect.Carousel.SelectNextRandom(); + }, 400); + }, test_count); + + AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); + } + + private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", + () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); private static int importId; private int getImportId() => ++importId; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index fed1f7a944..f30618ce3f 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -597,11 +597,17 @@ namespace osu.Game.Screens.Select { bindBindables(); + // As a selection was already obtained, do not attempt to update the selected beatmap. + if (Carousel.SelectedBeatmapSet != null) + return; + + // Attempt to select the current beatmap on the carousel, if it is valid to be selected. if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false && Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false)) return; - if (Carousel.SelectedBeatmapSet == null && !Carousel.SelectNextRandom()) + // If the current active beatmap could not be selected, select a new random beatmap. + if (!Carousel.SelectNextRandom()) { // in the case random selection failed, we want to trigger selectionChanged // to show the dummy beatmap (we have nothing else to display). From 436760de967e59400822e25356b502294bd487e3 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 14:34:52 +0900 Subject: [PATCH 02/11] Change test name --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 85811e3a0a..8e3fe57d92 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -210,7 +210,7 @@ namespace osu.Game.Tests.Visual.SongSelect } [Test] - public void TestAddNewBeatmap() + public void TestAddNewBeatmapWhileSelectingRandom() { const int test_count = 10; int beatmapChangedCount = 0; From 1a871af5520113372c38d70740337dc794fe2b30 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 19:15:29 +0900 Subject: [PATCH 03/11] Fix hide selection, add test --- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 12 ++++++++++++ osu.Game/Screens/Select/BeatmapCarousel.cs | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 8e3fe57d92..c33528c3bf 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -234,6 +234,18 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); } + [Test] + public void TestHideSetSelectsCorrectBeatmap() + { + int? previousID = null; + createSongSelect(); + importForRuleset(0); + AddStep("Move to last difficulty", () => songSelect.Carousel.SelectBeatmap(songSelect.Carousel.BeatmapSets.First().Beatmaps.Last())); + AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID); + AddStep("Hide first beatmap", () => manager.Hide(songSelect.Carousel.SelectedBeatmapSet.Beatmaps.First())); + AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID); + } + private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63ad3b6ab2..0b3d0c448b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -158,6 +158,9 @@ namespace osu.Game.Screens.Select var newSet = createCarouselSet(beatmapSet); + // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + var previouslySelectedID = selectedBeatmap?.Beatmap.ID; + if (existingSet != null) root.RemoveChild(existingSet); @@ -173,7 +176,7 @@ namespace osu.Game.Screens.Select //check if we can/need to maintain our current selection. if (hadSelection) - select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); itemsCache.Invalidate(); Schedule(() => BeatmapSetsChanged?.Invoke()); From 4f091417189baf51bf504cf401a914fe8d568234 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Wed, 29 May 2019 12:22:34 +0900 Subject: [PATCH 04/11] remove extra bool --- osu.Game/Screens/Select/BeatmapCarousel.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 0b3d0c448b..6e3bec106f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -152,15 +152,15 @@ namespace osu.Game.Screens.Select { Schedule(() => { + int? previouslySelectedID = null; CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; + // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + if (existingSet?.State?.Value == CarouselItemState.Selected) + previouslySelectedID = selectedBeatmap?.Beatmap.ID; var newSet = createCarouselSet(beatmapSet); - // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. - var previouslySelectedID = selectedBeatmap?.Beatmap.ID; - if (existingSet != null) root.RemoveChild(existingSet); @@ -175,7 +175,7 @@ namespace osu.Game.Screens.Select applyActiveCriteria(false, false); //check if we can/need to maintain our current selection. - if (hadSelection) + if (previouslySelectedID != null) select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); itemsCache.Invalidate(); From 1aa865c3fb154d0ce88321f38cbef309c7799c22 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:34:24 +0900 Subject: [PATCH 05/11] split out method for reuse --- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 8636890081..d6b5e0175f 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -138,7 +138,7 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(0); + addRulesetImportStep(0); AddUntilStep("no selection", () => songSelect.Carousel.SelectedBeatmap == null); } @@ -147,8 +147,8 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(2); - importForRuleset(1); + addRulesetImportStep(2); + addRulesetImportStep(1); AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmap.RulesetID == 2); changeRuleset(1); @@ -223,7 +223,7 @@ namespace osu.Game.Tests.Visual.SongSelect }); AddRepeatStep($"Create beatmaps {test_count} times", () => { - manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == 0).ToArray())); + importForRuleset(0); Scheduler.AddDelayed(() => { @@ -240,15 +240,16 @@ namespace osu.Game.Tests.Visual.SongSelect { int? previousID = null; createSongSelect(); - importForRuleset(0); + addRulesetImportStep(0); AddStep("Move to last difficulty", () => songSelect.Carousel.SelectBeatmap(songSelect.Carousel.BeatmapSets.First().Beatmaps.Last())); AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID); AddStep("Hide first beatmap", () => manager.Hide(songSelect.Carousel.SelectedBeatmapSet.Beatmaps.First())); AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", - () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + private void addRulesetImportStep(int id) => AddStep($"import test map for ruleset {id}", () => importForRuleset(id)); + + private void importForRuleset(int id) => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray())); private static int importId; private int getImportId() => ++importId; From 41e3e2222bc7d9c75e44a1340144c2d52e1f7acb Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:40:49 +0900 Subject: [PATCH 06/11] fix test --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index d6b5e0175f..bc15bc6b6d 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -219,7 +219,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("Setup counter", () => { beatmapChangedCount = 0; - songSelect.Carousel.BeatmapSetsChanged += () => beatmapChangedCount++; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { From d4ba67747b37f4db9a0a482a5fef54470a6bcbfc Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:58:03 +0900 Subject: [PATCH 07/11] fix test count --- .idea/.idea.osu/.idea/.name | 1 + .../Visual/SongSelect/TestScenePlaySongSelect.cs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .idea/.idea.osu/.idea/.name diff --git a/.idea/.idea.osu/.idea/.name b/.idea/.idea.osu/.idea/.name new file mode 100644 index 0000000000..21cb4db60e --- /dev/null +++ b/.idea/.idea.osu/.idea/.name @@ -0,0 +1 @@ +osu \ No newline at end of file diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index bc15bc6b6d..fa73a14252 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -11,6 +11,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Extensions; +using osu.Framework.Logging; using osu.Framework.MathUtils; using osu.Framework.Platform; using osu.Framework.Screens; @@ -215,11 +216,13 @@ namespace osu.Game.Tests.Visual.SongSelect { const int test_count = 10; int beatmapChangedCount = 0; + int debounceCount = 0; createSongSelect(); - AddStep("Setup counter", () => + AddStep("Setup counters", () => { beatmapChangedCount = 0; - songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; + debounceCount = 0; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount += 1; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { @@ -229,10 +232,14 @@ namespace osu.Game.Tests.Visual.SongSelect { // Wait for debounce songSelect.Carousel.SelectNextRandom(); + ++debounceCount; }, 400); }, test_count); - AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); + AddUntilStep("Debounce limit reached", () => debounceCount == test_count); + + // The selected beatmap should have changed an additional 2 times since both initially loading songselect and the first import also triggers selectionChanged + AddAssert($"Beatmap changed {test_count + 2} times", () => beatmapChangedCount == test_count + 2); } [Test] From 975bb3db8a2aba107858543eb12b419f363cad44 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 11 Jun 2019 15:51:14 +0900 Subject: [PATCH 08/11] cleanup --- .idea/.idea.osu/.idea/.name | 1 - osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 .idea/.idea.osu/.idea/.name diff --git a/.idea/.idea.osu/.idea/.name b/.idea/.idea.osu/.idea/.name deleted file mode 100644 index 21cb4db60e..0000000000 --- a/.idea/.idea.osu/.idea/.name +++ /dev/null @@ -1 +0,0 @@ -osu \ No newline at end of file diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index fa73a14252..198da2c5b1 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -222,7 +222,7 @@ namespace osu.Game.Tests.Visual.SongSelect { beatmapChangedCount = 0; debounceCount = 0; - songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount += 1; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { From a53ade07a5ea39046b5b90d240435664010021a3 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 11 Jun 2019 15:51:57 +0900 Subject: [PATCH 09/11] remove unused using --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 198da2c5b1..2664c7a42c 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -11,7 +11,6 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Extensions; -using osu.Framework.Logging; using osu.Framework.MathUtils; using osu.Framework.Platform; using osu.Framework.Screens; From 94e65a324456172971e1b387f039ec1fd7343c0d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jun 2019 15:16:59 +0900 Subject: [PATCH 10/11] Fix settings checkboxes not being searchable --- osu.Game/Overlays/Settings/SettingsCheckbox.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/SettingsCheckbox.cs b/osu.Game/Overlays/Settings/SettingsCheckbox.cs index a1501d8015..a554159fd7 100644 --- a/osu.Game/Overlays/Settings/SettingsCheckbox.cs +++ b/osu.Game/Overlays/Settings/SettingsCheckbox.cs @@ -10,11 +10,14 @@ namespace osu.Game.Overlays.Settings { private OsuCheckbox checkbox; + private string labelText; + protected override Drawable CreateControl() => checkbox = new OsuCheckbox(); public override string LabelText { - set => checkbox.LabelText = value; + get => labelText; + set => checkbox.LabelText = labelText = value; } } } From 13234fb4a4b5e9a26a3480c3ac3c960fd7f56c60 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 12 Jun 2019 16:07:35 +0900 Subject: [PATCH 11/11] Adjust comments a bit --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 6e3bec106f..cf21c78c7f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -155,7 +155,7 @@ namespace osu.Game.Screens.Select int? previouslySelectedID = null; CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required if (existingSet?.State?.Value == CarouselItemState.Selected) previouslySelectedID = selectedBeatmap?.Beatmap.ID; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index dcfe1de8f2..d0645dbab6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -594,7 +594,7 @@ namespace osu.Game.Screens.Select { bindBindables(); - // As a selection was already obtained, do not attempt to update the selected beatmap. + // If a selection was already obtained, do not attempt to update the selected beatmap. if (Carousel.SelectedBeatmapSet != null) return;