From 0c85fd496f9d555e1ec1c0b07fbebd2f9e8fd998 Mon Sep 17 00:00:00 2001 From: wooster0 Date: Sun, 31 Dec 2023 22:10:49 +0900 Subject: [PATCH 1/9] Don't leave scores screen empty if no scores are present yet Addresses https://github.com/ppy/osu/discussions/23787 I originally wanted to set `allowShowingResults` to false if there are no results but because this involves an API request to fetch the scores that would mean all the scores would have to be fetched all at once so that it knows whether to hide or show the "View results" button on a beatmap. Because that would slow things down and be very inefficient, this still allows the user to view the scores screen but if there aren't any scores, it shows a text, which I think is at least for now better than nothing. As for the testing of this, I wasn't sure how to not generate scores only for one specific test so I opted into not using `SetUpSteps` and doing it that way. --- .../TestScenePlaylistsResultsScreen.cs | 47 ++++++++++++++----- osu.Game/Screens/Ranking/ResultsScreen.cs | 35 ++++++++++---- osu.Game/Screens/Ranking/ScorePanelList.cs | 2 + 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index cb422d8c06..e36e7f54ba 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -42,15 +42,15 @@ namespace osu.Game.Tests.Visual.Playlists private int totalCount; private ScoreInfo userScore; - [SetUpSteps] - public override void SetUpSteps() + // We don't use SetUpSteps for this because one of the tests shouldn't receive any scores. + public void InitialiseUserScoresAndBeatmap(bool noScores = false) { base.SetUpSteps(); // Previous test instances of the results screen may still exist at this point so wait for // those screens to be cleaned up by the base SetUpSteps before re-initialising test state. - // The the screen also holds a leased Beatmap bindable so reassigning it must happen after - // the screen as been exited. + // The screen also holds a leased Beatmap bindable so reassigning it must happen after + // the screen has been exited. AddStep("initialise user scores and beatmap", () => { lowestScoreId = 1; @@ -63,7 +63,7 @@ namespace osu.Game.Tests.Visual.Playlists userScore.Statistics = new Dictionary(); userScore.MaximumStatistics = new Dictionary(); - bindHandler(); + bindHandler(noScores: noScores); // Beatmap is required to be an actual beatmap so the scores can get their scores correctly // calculated for standardised scoring, else the tests that rely on ordering will fall over. @@ -74,6 +74,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowWithUserScore() { + InitialiseUserScoresAndBeatmap(); + AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); @@ -86,6 +88,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScore() { + InitialiseUserScoresAndBeatmap(); + createResults(); AddAssert("top score selected", () => this.ChildrenOfType().OrderByDescending(p => p.Score.TotalScore).First().State == PanelState.Expanded); @@ -94,6 +98,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowUserScoreWithDelay() { + InitialiseUserScoresAndBeatmap(); + AddStep("bind user score info handler", () => bindHandler(true, userScore)); createResults(() => userScore); @@ -105,6 +111,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScoreWithDelay() { + InitialiseUserScoresAndBeatmap(); + AddStep("bind delayed handler", () => bindHandler(true)); createResults(); @@ -115,6 +123,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheRight() { + InitialiseUserScoresAndBeatmap(); + createResults(); AddStep("bind delayed handler", () => bindHandler(true)); @@ -137,6 +147,8 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheLeft() { + InitialiseUserScoresAndBeatmap(); + AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); @@ -158,7 +170,16 @@ namespace osu.Game.Tests.Visual.Playlists } } - private void createResults(Func getScore = null) + [Test] + public void TestShowWithNoScores() + { + InitialiseUserScoresAndBeatmap(noScores: true); + + createResults(noScores: true); + AddAssert("no scores visible", () => resultsScreen.ScorePanelList.GetScorePanels().Count() == 0); + } + + private void createResults(Func getScore = null, bool noScores = false) { AddStep("load results", () => { @@ -169,11 +190,13 @@ namespace osu.Game.Tests.Visual.Playlists }); AddUntilStep("wait for screen to load", () => resultsScreen.IsLoaded); - waitForDisplay(); + waitForDisplay(noScores); } - private void waitForDisplay() + private void waitForDisplay(bool noScores = false) { + if (noScores) return; + AddUntilStep("wait for scores loaded", () => requestComplete // request handler may need to fire more than once to get scores. @@ -183,7 +206,7 @@ namespace osu.Game.Tests.Visual.Playlists AddWaitStep("wait for display", 5); } - private void bindHandler(bool delayed = false, ScoreInfo userScore = null, bool failRequests = false) => ((DummyAPIAccess)API).HandleRequest = request => + private void bindHandler(bool delayed = false, ScoreInfo userScore = null, bool failRequests = false, bool noScores = false) => ((DummyAPIAccess)API).HandleRequest = request => { // pre-check for requests we should be handling (as they are scheduled below). switch (request) @@ -219,7 +242,7 @@ namespace osu.Game.Tests.Visual.Playlists break; case IndexPlaylistScoresRequest i: - triggerSuccess(i, createIndexResponse(i)); + triggerSuccess(i, createIndexResponse(i, noScores)); break; } }, delay); @@ -301,10 +324,12 @@ namespace osu.Game.Tests.Visual.Playlists return multiplayerUserScore; } - private IndexedMultiplayerScores createIndexResponse(IndexPlaylistScoresRequest req) + private IndexedMultiplayerScores createIndexResponse(IndexPlaylistScoresRequest req, bool noScores = false) { var result = new IndexedMultiplayerScores(); + if (noScores) return result; + string sort = req.IndexParams?.Properties["sort"].ToObject() ?? "score_desc"; for (int i = 1; i <= scores_per_result; i++) diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index e3d19725da..e18ab63706 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -19,6 +19,7 @@ using osu.Framework.Input.Events; using osu.Framework.Screens; using osu.Game.Graphics; using osu.Game.Graphics.Containers; +using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Input.Bindings; using osu.Game.Online.API; @@ -204,17 +205,31 @@ namespace osu.Game.Screens.Ranking if (lastFetchCompleted) { - APIRequest nextPageRequest = null; - - if (ScorePanelList.IsScrolledToStart) - nextPageRequest = FetchNextPage(-1, fetchScoresCallback); - else if (ScorePanelList.IsScrolledToEnd) - nextPageRequest = FetchNextPage(1, fetchScoresCallback); - - if (nextPageRequest != null) + if (ScorePanelList.IsEmpty) { - lastFetchCompleted = false; - api.Queue(nextPageRequest); + // This can happen if a beatmap part of a playlist hasn't been played yet. + VerticalScrollContent.Add(new OsuSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Font = OsuFont.GetFont(size: 32, weight: FontWeight.Regular), + Text = "no scores yet!", + }); + } + else + { + APIRequest nextPageRequest = null; + + if (ScorePanelList.IsScrolledToStart) + nextPageRequest = FetchNextPage(-1, fetchScoresCallback); + else if (ScorePanelList.IsScrolledToEnd) + nextPageRequest = FetchNextPage(1, fetchScoresCallback); + + if (nextPageRequest != null) + { + lastFetchCompleted = false; + api.Queue(nextPageRequest); + } } } } diff --git a/osu.Game/Screens/Ranking/ScorePanelList.cs b/osu.Game/Screens/Ranking/ScorePanelList.cs index b75f3d86ff..95c90e35a0 100644 --- a/osu.Game/Screens/Ranking/ScorePanelList.cs +++ b/osu.Game/Screens/Ranking/ScorePanelList.cs @@ -49,6 +49,8 @@ namespace osu.Game.Screens.Ranking public bool AllPanelsVisible => flow.All(p => p.IsPresent); + public bool IsEmpty => flow.Count == 0; + /// /// The current scroll position. /// From 2c64db06287b40811e742f785664eafac09093c5 Mon Sep 17 00:00:00 2001 From: wooster0 Date: Thu, 4 Jan 2024 12:15:48 +0900 Subject: [PATCH 2/9] Use already existing message placeholder + localized string --- osu.Game/Screens/Ranking/ResultsScreen.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index e18ab63706..8823e4d320 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -19,10 +19,11 @@ using osu.Framework.Input.Events; using osu.Framework.Screens; using osu.Game.Graphics; using osu.Game.Graphics.Containers; -using osu.Game.Graphics.Sprites; using osu.Game.Graphics.UserInterface; using osu.Game.Input.Bindings; +using osu.Game.Localisation; using osu.Game.Online.API; +using osu.Game.Online.Placeholders; using osu.Game.Scoring; using osu.Game.Screens.Play; using osu.Game.Screens.Ranking.Statistics; @@ -208,13 +209,7 @@ namespace osu.Game.Screens.Ranking if (ScorePanelList.IsEmpty) { // This can happen if a beatmap part of a playlist hasn't been played yet. - VerticalScrollContent.Add(new OsuSpriteText - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Font = OsuFont.GetFont(size: 32, weight: FontWeight.Regular), - Text = "no scores yet!", - }); + VerticalScrollContent.Add(new MessagePlaceholder(LeaderboardStrings.NoRecordsYet)); } else { From 8ad697ff4cea857bec7f18a94723b098c6e036ef Mon Sep 17 00:00:00 2001 From: wooster0 Date: Tue, 9 Jan 2024 21:28:46 +0900 Subject: [PATCH 3/9] apply some suggestions/corrections --- .../TestScenePlaylistsResultsScreen.cs | 16 ++++++---- osu.Game/Screens/Ranking/ResultsScreen.cs | 32 +++++++++---------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index e36e7f54ba..98aeb66428 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -79,6 +79,7 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); + waitForDisplay(); AddAssert("user score selected", () => this.ChildrenOfType().Single(p => p.Score.OnlineID == userScore.OnlineID).State == PanelState.Expanded); AddAssert($"score panel position is {real_user_position}", @@ -91,6 +92,7 @@ namespace osu.Game.Tests.Visual.Playlists InitialiseUserScoresAndBeatmap(); createResults(); + waitForDisplay(); AddAssert("top score selected", () => this.ChildrenOfType().OrderByDescending(p => p.Score.TotalScore).First().State == PanelState.Expanded); } @@ -103,6 +105,7 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("bind user score info handler", () => bindHandler(true, userScore)); createResults(() => userScore); + waitForDisplay(); AddAssert("more than 1 panel displayed", () => this.ChildrenOfType().Count() > 1); AddAssert("user score selected", () => this.ChildrenOfType().Single(p => p.Score.OnlineID == userScore.OnlineID).State == PanelState.Expanded); @@ -116,6 +119,7 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("bind delayed handler", () => bindHandler(true)); createResults(); + waitForDisplay(); AddAssert("top score selected", () => this.ChildrenOfType().OrderByDescending(p => p.Score.TotalScore).First().State == PanelState.Expanded); } @@ -126,6 +130,7 @@ namespace osu.Game.Tests.Visual.Playlists InitialiseUserScoresAndBeatmap(); createResults(); + waitForDisplay(); AddStep("bind delayed handler", () => bindHandler(true)); @@ -152,6 +157,7 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); + waitForDisplay(); AddStep("bind delayed handler", () => bindHandler(true)); @@ -174,12 +180,11 @@ namespace osu.Game.Tests.Visual.Playlists public void TestShowWithNoScores() { InitialiseUserScoresAndBeatmap(noScores: true); - - createResults(noScores: true); + createResults(); AddAssert("no scores visible", () => resultsScreen.ScorePanelList.GetScorePanels().Count() == 0); } - private void createResults(Func getScore = null, bool noScores = false) + private void createResults(Func getScore = null) { AddStep("load results", () => { @@ -190,13 +195,10 @@ namespace osu.Game.Tests.Visual.Playlists }); AddUntilStep("wait for screen to load", () => resultsScreen.IsLoaded); - waitForDisplay(noScores); } - private void waitForDisplay(bool noScores = false) + private void waitForDisplay() { - if (noScores) return; - AddUntilStep("wait for scores loaded", () => requestComplete // request handler may need to fire more than once to get scores. diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index 8823e4d320..697d62ad6e 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -206,25 +206,17 @@ namespace osu.Game.Screens.Ranking if (lastFetchCompleted) { - if (ScorePanelList.IsEmpty) - { - // This can happen if a beatmap part of a playlist hasn't been played yet. - VerticalScrollContent.Add(new MessagePlaceholder(LeaderboardStrings.NoRecordsYet)); - } - else - { - APIRequest nextPageRequest = null; + APIRequest nextPageRequest = null; - if (ScorePanelList.IsScrolledToStart) - nextPageRequest = FetchNextPage(-1, fetchScoresCallback); - else if (ScorePanelList.IsScrolledToEnd) - nextPageRequest = FetchNextPage(1, fetchScoresCallback); + if (ScorePanelList.IsScrolledToStart) + nextPageRequest = FetchNextPage(-1, fetchScoresCallback); + else if (ScorePanelList.IsScrolledToEnd) + nextPageRequest = FetchNextPage(1, fetchScoresCallback); - if (nextPageRequest != null) - { - lastFetchCompleted = false; - api.Queue(nextPageRequest); - } + if (nextPageRequest != null) + { + lastFetchCompleted = false; + api.Queue(nextPageRequest); } } } @@ -255,6 +247,12 @@ namespace osu.Game.Screens.Ranking addScore(s); lastFetchCompleted = true; + + if (ScorePanelList.IsEmpty) + { + // This can happen if for example a beatmap that is part of a playlist hasn't been played yet. + VerticalScrollContent.Add(new MessagePlaceholder(LeaderboardStrings.NoRecordsYet)); + } }); public override void OnEntering(ScreenTransitionEvent e) From ea37c70e0b0e032e84a9fa1ee7c902c1a7cb2c2e Mon Sep 17 00:00:00 2001 From: wooster0 Date: Tue, 9 Jan 2024 21:34:40 +0900 Subject: [PATCH 4/9] apply suggestion --- .../TestScenePlaylistsResultsScreen.cs | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index 98aeb66428..02bd7f950f 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -42,8 +42,8 @@ namespace osu.Game.Tests.Visual.Playlists private int totalCount; private ScoreInfo userScore; - // We don't use SetUpSteps for this because one of the tests shouldn't receive any scores. - public void InitialiseUserScoresAndBeatmap(bool noScores = false) + [SetUpSteps] + public override void SetUpSteps() { base.SetUpSteps(); @@ -63,18 +63,24 @@ namespace osu.Game.Tests.Visual.Playlists userScore.Statistics = new Dictionary(); userScore.MaximumStatistics = new Dictionary(); - bindHandler(noScores: noScores); - // Beatmap is required to be an actual beatmap so the scores can get their scores correctly // calculated for standardised scoring, else the tests that rely on ordering will fall over. Beatmap.Value = CreateWorkingBeatmap(Ruleset.Value); }); } + public void SetUpRequestHandler(bool noScores = false) + { + AddStep("set up request handler", () => + { + bindHandler(noScores: noScores); + }); + } + [Test] public void TestShowWithUserScore() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); @@ -89,7 +95,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScore() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); createResults(); waitForDisplay(); @@ -100,7 +106,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowUserScoreWithDelay() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); AddStep("bind user score info handler", () => bindHandler(true, userScore)); @@ -114,7 +120,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScoreWithDelay() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); AddStep("bind delayed handler", () => bindHandler(true)); @@ -127,7 +133,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheRight() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); createResults(); waitForDisplay(); @@ -152,7 +158,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheLeft() { - InitialiseUserScoresAndBeatmap(); + SetUpRequestHandler(); AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); @@ -179,7 +185,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowWithNoScores() { - InitialiseUserScoresAndBeatmap(noScores: true); + SetUpRequestHandler(true); createResults(); AddAssert("no scores visible", () => resultsScreen.ScorePanelList.GetScorePanels().Count() == 0); } From d5fdd0c0f9bb4a52630e9fead6b27ab44e472ca0 Mon Sep 17 00:00:00 2001 From: wooster0 Date: Tue, 9 Jan 2024 21:44:05 +0900 Subject: [PATCH 5/9] make each test bind the handler only once, depending on its need --- .../TestScenePlaylistsResultsScreen.cs | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index 02bd7f950f..727d9da50b 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -69,19 +69,9 @@ namespace osu.Game.Tests.Visual.Playlists }); } - public void SetUpRequestHandler(bool noScores = false) - { - AddStep("set up request handler", () => - { - bindHandler(noScores: noScores); - }); - } - [Test] public void TestShowWithUserScore() { - SetUpRequestHandler(); - AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); @@ -95,7 +85,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScore() { - SetUpRequestHandler(); + AddStep("bind user score info handler", () => bindHandler()); createResults(); waitForDisplay(); @@ -106,8 +96,6 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowUserScoreWithDelay() { - SetUpRequestHandler(); - AddStep("bind user score info handler", () => bindHandler(true, userScore)); createResults(() => userScore); @@ -120,8 +108,6 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowNullUserScoreWithDelay() { - SetUpRequestHandler(); - AddStep("bind delayed handler", () => bindHandler(true)); createResults(); @@ -133,13 +119,11 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheRight() { - SetUpRequestHandler(); + AddStep("bind delayed handler", () => bindHandler(true)); createResults(); waitForDisplay(); - AddStep("bind delayed handler", () => bindHandler(true)); - for (int i = 0; i < 2; i++) { int beforePanelCount = 0; @@ -158,8 +142,6 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestFetchWhenScrolledToTheLeft() { - SetUpRequestHandler(); - AddStep("bind user score info handler", () => bindHandler(userScore: userScore)); createResults(() => userScore); @@ -185,7 +167,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestShowWithNoScores() { - SetUpRequestHandler(true); + AddStep("bind user score info handler", () => bindHandler(noScores: true)); createResults(); AddAssert("no scores visible", () => resultsScreen.ScorePanelList.GetScorePanels().Count() == 0); } From 9bc9db9138ee6d8e103c1f2a8dd87b671cb6a5b8 Mon Sep 17 00:00:00 2001 From: wooster0 Date: Mon, 15 Jan 2024 20:58:43 +0900 Subject: [PATCH 6/9] simplify the code even more --- osu.Game/Screens/Ranking/ResultsScreen.cs | 2 +- osu.Game/Screens/Ranking/ScorePanelList.cs | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index 697d62ad6e..da1609c2c4 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -248,7 +248,7 @@ namespace osu.Game.Screens.Ranking lastFetchCompleted = true; - if (ScorePanelList.IsEmpty) + if (!scores.Any()) { // This can happen if for example a beatmap that is part of a playlist hasn't been played yet. VerticalScrollContent.Add(new MessagePlaceholder(LeaderboardStrings.NoRecordsYet)); diff --git a/osu.Game/Screens/Ranking/ScorePanelList.cs b/osu.Game/Screens/Ranking/ScorePanelList.cs index 95c90e35a0..b75f3d86ff 100644 --- a/osu.Game/Screens/Ranking/ScorePanelList.cs +++ b/osu.Game/Screens/Ranking/ScorePanelList.cs @@ -49,8 +49,6 @@ namespace osu.Game.Screens.Ranking public bool AllPanelsVisible => flow.All(p => p.IsPresent); - public bool IsEmpty => flow.Count == 0; - /// /// The current scroll position. /// From 8a839f64ed18cfabd6ca8ae98e7ba80441641803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 15 Jan 2024 18:47:46 +0100 Subject: [PATCH 7/9] Add failing test coverage for placeholder shown when it shouldn't be --- .../TestScenePlaylistsResultsScreen.cs | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index 727d9da50b..ff1b351b9f 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -17,6 +17,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; +using osu.Game.Online.Placeholders; using osu.Game.Online.Rooms; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Scoring; @@ -139,6 +140,37 @@ namespace osu.Game.Tests.Visual.Playlists } } + [Test] + public void TestNoMoreScoresToTheRight() + { + AddStep("bind delayed handler with scores", () => bindHandler(delayed: true)); + + createResults(); + waitForDisplay(); + + int beforePanelCount = 0; + + AddStep("get panel count", () => beforePanelCount = this.ChildrenOfType().Count()); + AddStep("scroll to right", () => resultsScreen.ScorePanelList.ChildrenOfType().Single().ScrollToEnd(false)); + + AddAssert("right loading spinner shown", () => resultsScreen.RightSpinner.State.Value == Visibility.Visible); + waitForDisplay(); + + AddAssert($"count increased by {scores_per_result}", () => this.ChildrenOfType().Count() >= beforePanelCount + scores_per_result); + AddAssert("right loading spinner hidden", () => resultsScreen.RightSpinner.State.Value == Visibility.Hidden); + + AddStep("get panel count", () => beforePanelCount = this.ChildrenOfType().Count()); + AddStep("bind delayed handler with no scores", () => bindHandler(delayed: true, noScores: true)); + AddStep("scroll to right", () => resultsScreen.ScorePanelList.ChildrenOfType().Single().ScrollToEnd(false)); + + AddAssert("right loading spinner shown", () => resultsScreen.RightSpinner.State.Value == Visibility.Visible); + waitForDisplay(); + + AddAssert("count not increased", () => this.ChildrenOfType().Count() == beforePanelCount); + AddAssert("right loading spinner hidden", () => resultsScreen.RightSpinner.State.Value == Visibility.Hidden); + AddAssert("no placeholders shown", () => this.ChildrenOfType().Count(), () => Is.Zero); + } + [Test] public void TestFetchWhenScrolledToTheLeft() { From 9d2c82452cfee1cfd7b23dc81c78d902938b5945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 15 Jan 2024 18:47:49 +0100 Subject: [PATCH 8/9] Revert "simplify the code even more" This reverts commit 9bc9db9138ee6d8e103c1f2a8dd87b671cb6a5b8. --- osu.Game/Screens/Ranking/ResultsScreen.cs | 2 +- osu.Game/Screens/Ranking/ScorePanelList.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Ranking/ResultsScreen.cs b/osu.Game/Screens/Ranking/ResultsScreen.cs index da1609c2c4..697d62ad6e 100644 --- a/osu.Game/Screens/Ranking/ResultsScreen.cs +++ b/osu.Game/Screens/Ranking/ResultsScreen.cs @@ -248,7 +248,7 @@ namespace osu.Game.Screens.Ranking lastFetchCompleted = true; - if (!scores.Any()) + if (ScorePanelList.IsEmpty) { // This can happen if for example a beatmap that is part of a playlist hasn't been played yet. VerticalScrollContent.Add(new MessagePlaceholder(LeaderboardStrings.NoRecordsYet)); diff --git a/osu.Game/Screens/Ranking/ScorePanelList.cs b/osu.Game/Screens/Ranking/ScorePanelList.cs index b75f3d86ff..95c90e35a0 100644 --- a/osu.Game/Screens/Ranking/ScorePanelList.cs +++ b/osu.Game/Screens/Ranking/ScorePanelList.cs @@ -49,6 +49,8 @@ namespace osu.Game.Screens.Ranking public bool AllPanelsVisible => flow.All(p => p.IsPresent); + public bool IsEmpty => flow.Count == 0; + /// /// The current scroll position. /// From 988794cf90d8bbe803c6a24a1e8c0c8a3ffaf2c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 15 Jan 2024 18:49:41 +0100 Subject: [PATCH 9/9] Improve test coverage for empty results --- .../Visual/Playlists/TestScenePlaylistsResultsScreen.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs index ff1b351b9f..e805b03542 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs @@ -201,7 +201,8 @@ namespace osu.Game.Tests.Visual.Playlists { AddStep("bind user score info handler", () => bindHandler(noScores: true)); createResults(); - AddAssert("no scores visible", () => resultsScreen.ScorePanelList.GetScorePanels().Count() == 0); + AddAssert("no scores visible", () => !resultsScreen.ScorePanelList.GetScorePanels().Any()); + AddAssert("placeholder shown", () => this.ChildrenOfType().Count(), () => Is.EqualTo(1)); } private void createResults(Func getScore = null)