From 4cc38cea63a97611d6f5fbc902700c2e374d4dae Mon Sep 17 00:00:00 2001 From: OliBomby Date: Fri, 16 Aug 2024 14:24:03 +0200 Subject: [PATCH 01/46] fix last anchor converting to implicit segment --- osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index 8a8964ccd4..b0173b3ae3 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -450,7 +450,7 @@ namespace osu.Game.Beatmaps.Formats // Explicit segments have a new format in which the type is injected into the middle of the control point string. // To preserve compatibility with osu-stable as much as possible, explicit segments with the same type are converted to use implicit segments by duplicating the control point. // One exception are consecutive perfect curves, which aren't supported in osu!stable and can lead to decoding issues if encoded as implicit segments - bool needsExplicitSegment = point.Type != lastType || point.Type == PathType.PERFECT_CURVE; + bool needsExplicitSegment = point.Type != lastType || point.Type == PathType.PERFECT_CURVE || i == pathData.Path.ControlPoints.Count - 1; // Another exception to this is when the last two control points of the last segment were duplicated. This is not a scenario supported by osu!stable. // Lazer does not add implicit segments for the last two control points of _any_ explicit segment, so an explicit segment is forced in order to maintain consistency with the decoder. From a2e26ba9ffb93308e73263a7243833d429b873cb Mon Sep 17 00:00:00 2001 From: OliBomby Date: Fri, 16 Aug 2024 14:24:55 +0200 Subject: [PATCH 02/46] Fix perfect curve anchors losing type between reloads --- osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 8e6ffa20cc..d4e3053856 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -347,7 +347,7 @@ namespace osu.Game.Rulesets.Objects.Legacy vertices[i] = new PathControlPoint { Position = points[i] }; // Edge-case rules (to match stable). - if (type == PathType.PERFECT_CURVE) + if (type == PathType.PERFECT_CURVE && FormatVersion < LegacyBeatmapEncoder.FIRST_LAZER_VERSION) { int endPointLength = endPoint == null ? 0 : 1; From 637c9aeef0c4879c3ad8e5adbf8b7fcfe9c21472 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 21 Aug 2024 03:35:26 +0900 Subject: [PATCH 03/46] Add `DailyChallengeIntroPlayed` session static --- osu.Game/Configuration/SessionStatics.cs | 6 ++++++ osu.Game/Screens/Menu/DailyChallengeButton.cs | 7 +++++++ osu.Game/Screens/Menu/MainMenu.cs | 5 ++++- .../OnlinePlay/DailyChallenge/DailyChallengeIntro.cs | 5 +++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/osu.Game/Configuration/SessionStatics.cs b/osu.Game/Configuration/SessionStatics.cs index 1548b781a7..225f209380 100644 --- a/osu.Game/Configuration/SessionStatics.cs +++ b/osu.Game/Configuration/SessionStatics.cs @@ -80,5 +80,11 @@ namespace osu.Game.Configuration /// Stores the local user's last score (can be completed or aborted). /// LastLocalUserScore, + + /// + /// Whether the intro animation for the daily challenge screen has been played once. + /// This is reset when a new challenge is up. + /// + DailyChallengeIntroPlayed, } } diff --git a/osu.Game/Screens/Menu/DailyChallengeButton.cs b/osu.Game/Screens/Menu/DailyChallengeButton.cs index e6593c9b0d..d47866ef73 100644 --- a/osu.Game/Screens/Menu/DailyChallengeButton.cs +++ b/osu.Game/Screens/Menu/DailyChallengeButton.cs @@ -13,6 +13,7 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Threading; using osu.Framework.Utils; using osu.Game.Beatmaps.Drawables; +using osu.Game.Configuration; using osu.Game.Graphics; using osu.Game.Graphics.Sprites; using osu.Game.Localisation; @@ -46,6 +47,9 @@ namespace osu.Game.Screens.Menu [Resolved] private INotificationOverlay? notificationOverlay { get; set; } + [Resolved] + private SessionStatics statics { get; set; } = null!; + public DailyChallengeButton(string sampleName, Color4 colour, Action? clickAction = null, params Key[] triggerKeys) : base(ButtonSystemStrings.DailyChallenge, sampleName, OsuIcon.DailyChallenge, colour, clickAction, triggerKeys) { @@ -148,6 +152,9 @@ namespace osu.Game.Screens.Menu roomRequest.Success += room => { + // force showing intro on the first time when a new daily challenge is up. + statics.SetValue(Static.DailyChallengeIntroPlayed, false); + Room = room; cover.OnlineInfo = TooltipContent = room.Playlist.FirstOrDefault()?.Beatmap.BeatmapSet as APIBeatmapSet; diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index dfe5460aee..64a173e088 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -150,7 +150,10 @@ namespace osu.Game.Screens.Menu OnPlaylists = () => this.Push(new Playlists()), OnDailyChallenge = room => { - this.Push(new DailyChallengeIntro(room)); + if (statics.Get(Static.DailyChallengeIntroPlayed)) + this.Push(new DailyChallenge(room)); + else + this.Push(new DailyChallengeIntro(room)); }, OnExit = () => { diff --git a/osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallengeIntro.cs b/osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallengeIntro.cs index e59031f663..619e7c1e42 100644 --- a/osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallengeIntro.cs +++ b/osu.Game/Screens/OnlinePlay/DailyChallenge/DailyChallengeIntro.cs @@ -70,6 +70,9 @@ namespace osu.Game.Screens.OnlinePlay.DailyChallenge [Resolved] private MusicController musicController { get; set; } = null!; + [Resolved] + private SessionStatics statics { get; set; } = null!; + private Sample? dateWindupSample; private Sample? dateImpactSample; private Sample? beatmapWindupSample; @@ -461,6 +464,8 @@ namespace osu.Game.Screens.OnlinePlay.DailyChallenge { Schedule(() => { + statics.SetValue(Static.DailyChallengeIntroPlayed, true); + if (this.IsCurrentScreen()) this.Push(new DailyChallenge(room)); }); From db568bfb79d5d037e8a0e9d1485917421483da9a Mon Sep 17 00:00:00 2001 From: OliBomby Date: Wed, 21 Aug 2024 01:01:17 +0200 Subject: [PATCH 04/46] add tests --- .../Editor/TestSceneSliderChangeStates.cs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderChangeStates.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderChangeStates.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderChangeStates.cs new file mode 100644 index 0000000000..0b8f2f7417 --- /dev/null +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneSliderChangeStates.cs @@ -0,0 +1,47 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Game.Beatmaps; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Tests.Beatmaps; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Tests.Editor +{ + public partial class TestSceneSliderChangeStates : TestSceneOsuEditor + { + protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new TestBeatmap(ruleset, false); + + [TestCase(SplineType.Catmull)] + [TestCase(SplineType.BSpline)] + [TestCase(SplineType.Linear)] + [TestCase(SplineType.PerfectCurve)] + public void TestSliderRetainsCurveTypes(SplineType splineType) + { + Slider? slider = null; + PathType pathType = new PathType(splineType); + + AddStep("add slider", () => EditorBeatmap.Add(slider = new Slider + { + StartTime = 500, + Path = new SliderPath(new[] + { + new PathControlPoint(Vector2.Zero, pathType), + new PathControlPoint(new Vector2(200, 0), pathType), + }) + })); + AddAssert("slider has correct spline type", () => ((Slider)EditorBeatmap.HitObjects[0]).Path.ControlPoints.All(p => p.Type == pathType)); + AddStep("remove object", () => EditorBeatmap.Remove(slider)); + AddAssert("slider removed", () => EditorBeatmap.HitObjects.Count == 0); + addUndoSteps(); + AddAssert("slider not removed", () => EditorBeatmap.HitObjects.Count == 1); + AddAssert("slider has correct spline type", () => ((Slider)EditorBeatmap.HitObjects[0]).Path.ControlPoints.All(p => p.Type == pathType)); + } + + private void addUndoSteps() => AddStep("undo", () => Editor.Undo()); + } +} From eefd7cf0833e3e18a40b698c5a090b7934ec1205 Mon Sep 17 00:00:00 2001 From: OliBomby Date: Wed, 21 Aug 2024 12:03:15 +0200 Subject: [PATCH 05/46] add back protection against perfect curve segments with > 3 points --- .../Objects/Legacy/ConvertHitObjectParser.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index d4e3053856..c518a3e8b2 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -347,17 +347,23 @@ namespace osu.Game.Rulesets.Objects.Legacy vertices[i] = new PathControlPoint { Position = points[i] }; // Edge-case rules (to match stable). - if (type == PathType.PERFECT_CURVE && FormatVersion < LegacyBeatmapEncoder.FIRST_LAZER_VERSION) + if (type == PathType.PERFECT_CURVE) { int endPointLength = endPoint == null ? 0 : 1; - if (vertices.Length + endPointLength != 3) - type = PathType.BEZIER; - else if (isLinear(points[0], points[1], endPoint ?? points[2])) + if (FormatVersion < LegacyBeatmapEncoder.FIRST_LAZER_VERSION) { - // osu-stable special-cased colinear perfect curves to a linear path - type = PathType.LINEAR; + if (vertices.Length + endPointLength != 3) + type = PathType.BEZIER; + else if (isLinear(points[0], points[1], endPoint ?? points[2])) + { + // osu-stable special-cased colinear perfect curves to a linear path + type = PathType.LINEAR; + } } + else if (vertices.Length + endPointLength > 3) + // Lazer supports perfect curves with less than 3 points and colinear points + type = PathType.BEZIER; } // The first control point must have a definite type. From a669c53df7ea119792c3c42007791ba5941c6635 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 22 Aug 2024 06:00:07 +0900 Subject: [PATCH 06/46] Add failing test cases --- .../UserInterface/TestSceneMainMenuButton.cs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs index 98f2b129ff..e534547c27 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs @@ -6,6 +6,7 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; +using osu.Game.Configuration; using osu.Game.Graphics; using osu.Game.Localisation; using osu.Game.Online.API; @@ -71,6 +72,7 @@ namespace osu.Game.Tests.Visual.UserInterface NotificationOverlay notificationOverlay = null!; DependencyProvidingContainer buttonContainer = null!; + AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); AddStep("beatmap of the day active", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = 1234, @@ -96,6 +98,7 @@ namespace osu.Game.Tests.Visual.UserInterface }, }; }); + AddAssert("intro played flag reset", () => !Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); AddAssert("notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1)); AddStep("clear notifications", () => @@ -103,15 +106,85 @@ namespace osu.Game.Tests.Visual.UserInterface foreach (var notification in notificationOverlay.AllNotifications) notification.Close(runFlingAnimation: false); }); + + AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); + AddStep("beatmap of the day not active", () => metadataClient.DailyChallengeUpdated(null)); AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); + AddAssert("intro played flag still set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); AddStep("hide button's parent", () => buttonContainer.Hide()); + AddStep("beatmap of the day active", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = 1234, })); AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); + AddAssert("intro played flag still set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); + } + + [Test] + public void TestDailyChallengeButtonOldChallenge() + { + AddStep("set up API", () => dummyAPI.HandleRequest = req => + { + switch (req) + { + case GetRoomRequest getRoomRequest: + if (getRoomRequest.RoomId != 1234) + return false; + + var beatmap = CreateAPIBeatmap(); + beatmap.OnlineID = 1001; + getRoomRequest.TriggerSuccess(new Room + { + RoomID = { Value = 1234 }, + Playlist = + { + new PlaylistItem(beatmap) + }, + StartDate = { Value = DateTimeOffset.Now.AddMinutes(-50) }, + EndDate = { Value = DateTimeOffset.Now.AddSeconds(30) } + }); + return true; + + default: + return false; + } + }); + + NotificationOverlay notificationOverlay = null!; + + AddStep("beatmap of the day not active", () => metadataClient.DailyChallengeUpdated(null)); + AddStep("add content", () => + { + notificationOverlay = new NotificationOverlay(); + Children = new Drawable[] + { + notificationOverlay, + new DependencyProvidingContainer + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + AutoSizeAxes = Axes.Both, + CachedDependencies = [(typeof(INotificationOverlay), notificationOverlay)], + Child = new DailyChallengeButton(@"button-default-select", new Color4(102, 68, 204, 255), _ => { }, 0, Key.D) + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + ButtonSystemState = ButtonSystemState.TopLevel, + }, + }, + }; + }); + + AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); + AddStep("beatmap of the day active", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo + { + RoomID = 1234 + })); + AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); + AddAssert("intro played flag reset", () => !Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); } } } From 922814fab37f7f915f80265740640f049acd2056 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Thu, 22 Aug 2024 06:00:37 +0900 Subject: [PATCH 07/46] Fix flag reset on connection dropouts --- osu.Game/Screens/Menu/DailyChallengeButton.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/osu.Game/Screens/Menu/DailyChallengeButton.cs b/osu.Game/Screens/Menu/DailyChallengeButton.cs index d47866ef73..4dbebf0ae9 100644 --- a/osu.Game/Screens/Menu/DailyChallengeButton.cs +++ b/osu.Game/Screens/Menu/DailyChallengeButton.cs @@ -132,7 +132,7 @@ namespace osu.Game.Screens.Menu } } - private long? lastNotifiedDailyChallengeRoomId; + private long? lastDailyChallengeRoomID; private void dailyChallengeChanged(ValueChangedEvent _) { @@ -152,19 +152,19 @@ namespace osu.Game.Screens.Menu roomRequest.Success += room => { - // force showing intro on the first time when a new daily challenge is up. - statics.SetValue(Static.DailyChallengeIntroPlayed, false); - Room = room; cover.OnlineInfo = TooltipContent = room.Playlist.FirstOrDefault()?.Beatmap.BeatmapSet as APIBeatmapSet; - // We only want to notify the user if a new challenge recently went live. - if (room.StartDate.Value != null - && Math.Abs((DateTimeOffset.Now - room.StartDate.Value!.Value).TotalSeconds) < 1800 - && room.RoomID.Value != lastNotifiedDailyChallengeRoomId) + if (room.StartDate.Value != null && room.RoomID.Value != lastDailyChallengeRoomID) { - lastNotifiedDailyChallengeRoomId = room.RoomID.Value; - notificationOverlay?.Post(new NewDailyChallengeNotification(room)); + lastDailyChallengeRoomID = room.RoomID.Value; + + // new challenge is live, reset intro played static. + statics.SetValue(Static.DailyChallengeIntroPlayed, false); + + // we only want to notify the user if the new challenge just went live. + if (Math.Abs((DateTimeOffset.Now - room.StartDate.Value!.Value).TotalSeconds) < 1800) + notificationOverlay?.Post(new NewDailyChallengeNotification(room)); } updateCountdown(); From 236a273e09d0e05fc17561a449cd9dc2d95b2c82 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 22 Aug 2024 15:11:23 +0900 Subject: [PATCH 08/46] Simplify `DailyChallengeIntro` test scene Seems like some bad copy-paste in the past. Most of this is already being done in `TestSceneDailyChallenge`. --- .../TestSceneDailyChallengeIntro.cs | 37 ++++--------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs index a3541d957e..cff2387aed 100644 --- a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs +++ b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs @@ -5,13 +5,12 @@ using System; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Screens; -using osu.Framework.Testing; using osu.Game.Online.API; using osu.Game.Online.Metadata; using osu.Game.Online.Rooms; using osu.Game.Overlays; using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Screens.OnlinePlay.DailyChallenge; using osu.Game.Tests.Resources; using osu.Game.Tests.Visual.Metadata; using osu.Game.Tests.Visual.OnlinePlay; @@ -27,6 +26,8 @@ namespace osu.Game.Tests.Visual.DailyChallenge [Cached(typeof(INotificationOverlay))] private NotificationOverlay notificationOverlay = new NotificationOverlay(); + private Room room = null!; + [BackgroundDependencyLoader] private void load() { @@ -35,33 +36,15 @@ namespace osu.Game.Tests.Visual.DailyChallenge } [Test] - [Solo] public void TestDailyChallenge() { - var room = new Room - { - RoomID = { Value = 1234 }, - Name = { Value = "Daily Challenge: June 4, 2024" }, - Playlist = - { - new PlaylistItem(CreateAPIBeatmapSet().Beatmaps.First()) - { - RequiredMods = [new APIMod(new OsuModTraceable())], - AllowedMods = [new APIMod(new OsuModDoubleTime())] - } - }, - EndDate = { Value = DateTimeOffset.Now.AddHours(12) }, - Category = { Value = RoomCategory.DailyChallenge } - }; - - AddStep("add room", () => API.Perform(new CreateRoomRequest(room))); - AddStep("push screen", () => LoadScreen(new Screens.OnlinePlay.DailyChallenge.DailyChallengeIntro(room))); + startChallenge(); + AddStep("push screen", () => LoadScreen(new DailyChallengeIntro(room))); } - [Test] - public void TestNotifications() + private void startChallenge() { - var room = new Room + room = new Room { RoomID = { Value = 1234 }, Name = { Value = "Daily Challenge: June 4, 2024" }, @@ -78,12 +61,6 @@ namespace osu.Game.Tests.Visual.DailyChallenge }; AddStep("add room", () => API.Perform(new CreateRoomRequest(room))); - AddStep("set daily challenge info", () => metadataClient.DailyChallengeInfo.Value = new DailyChallengeInfo { RoomID = 1234 }); - - Screens.OnlinePlay.DailyChallenge.DailyChallenge screen = null!; - AddStep("push screen", () => LoadScreen(screen = new Screens.OnlinePlay.DailyChallenge.DailyChallenge(room))); - AddUntilStep("wait for screen", () => screen.IsCurrentScreen()); - AddStep("daily challenge ended", () => metadataClient.DailyChallengeInfo.Value = null); } } } From 9b9986b6f2c508c37fb6674c7b47381fa6681148 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 22 Aug 2024 15:14:52 +0900 Subject: [PATCH 09/46] Add isolated test for daily challenge intro flag --- .../DailyChallenge/TestSceneDailyChallengeIntro.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs index cff2387aed..08d44d7405 100644 --- a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs +++ b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs @@ -5,6 +5,7 @@ using System; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; +using osu.Game.Configuration; using osu.Game.Online.API; using osu.Game.Online.Metadata; using osu.Game.Online.Rooms; @@ -42,6 +43,19 @@ namespace osu.Game.Tests.Visual.DailyChallenge AddStep("push screen", () => LoadScreen(new DailyChallengeIntro(room))); } + [Test] + public void TestPlayIntroOnceFlag() + { + AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); + + startChallenge(); + + AddAssert("intro played flag reset", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.False); + + AddStep("push screen", () => LoadScreen(new DailyChallengeIntro(room))); + AddUntilStep("intro played flag set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.True); + } + private void startChallenge() { room = new Room From f5e195a7ee2e6a4d2c72ca2ab982bf23adb160aa Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 24 Aug 2024 06:40:30 +0900 Subject: [PATCH 10/46] Remove existing test asserts --- .../Visual/UserInterface/TestSceneMainMenuButton.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs index e534547c27..41543669eb 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneMainMenuButton.cs @@ -6,7 +6,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Game.Configuration; using osu.Game.Graphics; using osu.Game.Localisation; using osu.Game.Online.API; @@ -72,7 +71,6 @@ namespace osu.Game.Tests.Visual.UserInterface NotificationOverlay notificationOverlay = null!; DependencyProvidingContainer buttonContainer = null!; - AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); AddStep("beatmap of the day active", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = 1234, @@ -98,7 +96,6 @@ namespace osu.Game.Tests.Visual.UserInterface }, }; }); - AddAssert("intro played flag reset", () => !Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); AddAssert("notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1)); AddStep("clear notifications", () => @@ -107,11 +104,8 @@ namespace osu.Game.Tests.Visual.UserInterface notification.Close(runFlingAnimation: false); }); - AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); - AddStep("beatmap of the day not active", () => metadataClient.DailyChallengeUpdated(null)); AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); - AddAssert("intro played flag still set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); AddStep("hide button's parent", () => buttonContainer.Hide()); @@ -120,7 +114,6 @@ namespace osu.Game.Tests.Visual.UserInterface RoomID = 1234, })); AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); - AddAssert("intro played flag still set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); } [Test] @@ -178,13 +171,11 @@ namespace osu.Game.Tests.Visual.UserInterface }; }); - AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); AddStep("beatmap of the day active", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = 1234 })); AddAssert("no notification posted", () => notificationOverlay.AllNotifications.Count(), () => Is.Zero); - AddAssert("intro played flag reset", () => !Dependencies.Get().Get(Static.DailyChallengeIntroPlayed)); } } } From b54487031ab01ffac4e6aa67c0fc33a7b3ac71c3 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 24 Aug 2024 06:40:51 +0900 Subject: [PATCH 11/46] Add `DailyChallengeButton` for intro played flag reset logic --- .../TestSceneDailyChallengeIntro.cs | 51 +++++++++++-------- .../OnlinePlay/TestRoomRequestsHandler.cs | 10 +++- osu.Game/Utils/Optional.cs | 2 +- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs index 08d44d7405..f1a2d6b5f2 100644 --- a/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs +++ b/osu.Game.Tests/Visual/DailyChallenge/TestSceneDailyChallengeIntro.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Game.Configuration; @@ -10,11 +9,14 @@ using osu.Game.Online.API; using osu.Game.Online.Metadata; using osu.Game.Online.Rooms; using osu.Game.Overlays; +using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Screens.Menu; using osu.Game.Screens.OnlinePlay.DailyChallenge; -using osu.Game.Tests.Resources; using osu.Game.Tests.Visual.Metadata; using osu.Game.Tests.Visual.OnlinePlay; +using osuTK.Graphics; +using osuTK.Input; using CreateRoomRequest = osu.Game.Online.Rooms.CreateRoomRequest; namespace osu.Game.Tests.Visual.DailyChallenge @@ -32,23 +34,27 @@ namespace osu.Game.Tests.Visual.DailyChallenge [BackgroundDependencyLoader] private void load() { - base.Content.Add(notificationOverlay); - base.Content.Add(metadataClient); + Add(notificationOverlay); + Add(metadataClient); + + // add button to observe for daily challenge changes and perform its logic. + Add(new DailyChallengeButton(@"button-default-select", new Color4(102, 68, 204, 255), _ => { }, 0, Key.D)); } [Test] public void TestDailyChallenge() { - startChallenge(); + startChallenge(1234); AddStep("push screen", () => LoadScreen(new DailyChallengeIntro(room))); } [Test] public void TestPlayIntroOnceFlag() { + startChallenge(1234); AddStep("set intro played flag", () => Dependencies.Get().SetValue(Static.DailyChallengeIntroPlayed, true)); - startChallenge(); + startChallenge(1235); AddAssert("intro played flag reset", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.False); @@ -56,25 +62,28 @@ namespace osu.Game.Tests.Visual.DailyChallenge AddUntilStep("intro played flag set", () => Dependencies.Get().Get(Static.DailyChallengeIntroPlayed), () => Is.True); } - private void startChallenge() + private void startChallenge(int roomId) { - room = new Room + AddStep("add room", () => { - RoomID = { Value = 1234 }, - Name = { Value = "Daily Challenge: June 4, 2024" }, - Playlist = + API.Perform(new CreateRoomRequest(room = new Room { - new PlaylistItem(TestResources.CreateTestBeatmapSetInfo().Beatmaps.First()) + RoomID = { Value = roomId }, + Name = { Value = "Daily Challenge: June 4, 2024" }, + Playlist = { - RequiredMods = [new APIMod(new OsuModTraceable())], - AllowedMods = [new APIMod(new OsuModDoubleTime())] - } - }, - EndDate = { Value = DateTimeOffset.Now.AddHours(12) }, - Category = { Value = RoomCategory.DailyChallenge } - }; - - AddStep("add room", () => API.Perform(new CreateRoomRequest(room))); + new PlaylistItem(CreateAPIBeatmap(new OsuRuleset().RulesetInfo)) + { + RequiredMods = [new APIMod(new OsuModTraceable())], + AllowedMods = [new APIMod(new OsuModDoubleTime())] + } + }, + StartDate = { Value = DateTimeOffset.Now }, + EndDate = { Value = DateTimeOffset.Now.AddHours(24) }, + Category = { Value = RoomCategory.DailyChallenge } + })); + }); + AddStep("signal client", () => metadataClient.DailyChallengeUpdated(new DailyChallengeInfo { RoomID = roomId })); } } } diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs index 91df38feb9..4ceb946b28 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRoomRequestsHandler.cs @@ -18,6 +18,7 @@ using osu.Game.Rulesets.Scoring; using osu.Game.Scoring; using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Tests.Beatmaps; +using osu.Game.Utils; namespace osu.Game.Tests.Visual.OnlinePlay { @@ -277,11 +278,18 @@ namespace osu.Game.Tests.Visual.OnlinePlay var result = JsonConvert.DeserializeObject(JsonConvert.SerializeObject(source)); Debug.Assert(result != null); - // Playlist item IDs aren't serialised. + // Playlist item IDs and beatmaps aren't serialised. if (source.CurrentPlaylistItem.Value != null) + { + result.CurrentPlaylistItem.Value = result.CurrentPlaylistItem.Value.With(new Optional(source.CurrentPlaylistItem.Value.Beatmap)); result.CurrentPlaylistItem.Value.ID = source.CurrentPlaylistItem.Value.ID; + } + for (int i = 0; i < source.Playlist.Count; i++) + { + result.Playlist[i] = result.Playlist[i].With(new Optional(source.Playlist[i].Beatmap)); result.Playlist[i].ID = source.Playlist[i].ID; + } return result; } diff --git a/osu.Game/Utils/Optional.cs b/osu.Game/Utils/Optional.cs index 301767ba08..f5749a513f 100644 --- a/osu.Game/Utils/Optional.cs +++ b/osu.Game/Utils/Optional.cs @@ -22,7 +22,7 @@ namespace osu.Game.Utils /// public readonly bool HasValue; - private Optional(T value) + public Optional(T value) { Value = value; HasValue = true; From 466ed5de785e2f2f70a5077bdb8f1d527aad788d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Aug 2024 17:37:15 +0900 Subject: [PATCH 12/46] Add basic detached beatmap store --- osu.Game/Database/DetachedBeatmapStore.cs | 117 +++++++++++++++++++++ osu.Game/OsuGame.cs | 1 + osu.Game/Screens/Select/BeatmapCarousel.cs | 92 ++-------------- 3 files changed, 129 insertions(+), 81 deletions(-) create mode 100644 osu.Game/Database/DetachedBeatmapStore.cs diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs new file mode 100644 index 0000000000..0acc38a5a1 --- /dev/null +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -0,0 +1,117 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Logging; +using osu.Game.Beatmaps; +using Realms; + +namespace osu.Game.Database +{ + // TODO: handle realm migration + public partial class DetachedBeatmapStore : Component + { + private readonly ManualResetEventSlim loaded = new ManualResetEventSlim(); + + private List originalBeatmapSetsDetached = new List(); + + private IDisposable? subscriptionSets; + + /// + /// Track GUIDs of all sets in realm to allow handling deletions. + /// + private readonly List realmBeatmapSets = new List(); + + [Resolved] + private RealmAccess realm { get; set; } = null!; + + public IReadOnlyList GetDetachedBeatmaps() + { + if (!loaded.Wait(60000)) + Logger.Error(new TimeoutException("Beatmaps did not load in an acceptable time"), $"{nameof(DetachedBeatmapStore)} fell over"); + + return originalBeatmapSetsDetached; + } + + [BackgroundDependencyLoader] + private void load() + { + subscriptionSets = realm.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); + } + + private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) + { + if (changes == null) + { + if (originalBeatmapSetsDetached.Count > 0 && sender.Count == 0) + { + // Usually we'd reset stuff here, but doing so triggers a silly flow which ends up deadlocking realm. + // Additionally, user should not be at song select when realm is blocking all operations in the first place. + // + // Note that due to the catch-up logic below, once operations are restored we will still be in a roughly + // correct state. The only things that this return will change is the carousel will not empty *during* the blocking + // operation. + return; + } + + originalBeatmapSetsDetached = sender.Detach(); + + realmBeatmapSets.Clear(); + realmBeatmapSets.AddRange(sender.Select(r => r.ID)); + + loaded.Set(); + return; + } + + HashSet setsRequiringUpdate = new HashSet(); + HashSet setsRequiringRemoval = new HashSet(); + + foreach (int i in changes.DeletedIndices.OrderDescending()) + { + Guid id = realmBeatmapSets[i]; + + setsRequiringRemoval.Add(id); + setsRequiringUpdate.Remove(id); + + realmBeatmapSets.RemoveAt(i); + } + + foreach (int i in changes.InsertedIndices) + { + Guid id = sender[i].ID; + + setsRequiringRemoval.Remove(id); + setsRequiringUpdate.Add(id); + + realmBeatmapSets.Insert(i, id); + } + + foreach (int i in changes.NewModifiedIndices) + setsRequiringUpdate.Add(sender[i].ID); + + // deletions + foreach (Guid g in setsRequiringRemoval) + originalBeatmapSetsDetached.RemoveAll(set => set.ID == g); + + // updates + foreach (Guid g in setsRequiringUpdate) + { + originalBeatmapSetsDetached.RemoveAll(set => set.ID == g); + originalBeatmapSetsDetached.Add(fetchFromID(g)!); + } + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + subscriptionSets?.Dispose(); + } + + private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); + } +} diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 089db3b698..0ef6a94679 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -1141,6 +1141,7 @@ namespace osu.Game loadComponentSingleFile(new MedalOverlay(), topMostOverlayContent.Add); loadComponentSingleFile(new BackgroundDataStoreProcessor(), Add); + loadComponentSingleFile(new DetachedBeatmapStore(), Add, true); Add(difficultyRecommender); Add(externalLinkOpener = new ExternalLinkOpener()); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index b0f198d486..d06023258a 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -76,8 +76,6 @@ namespace osu.Game.Screens.Select private CarouselBeatmapSet? selectedBeatmapSet; - private List originalBeatmapSetsDetached = new List(); - /// /// Raised when the is changed. /// @@ -109,6 +107,9 @@ namespace osu.Game.Screens.Select [Cached] protected readonly CarouselScrollContainer Scroll; + [Resolved] + private DetachedBeatmapStore detachedBeatmapStore { get; set; } = null!; + private readonly NoResultsPlaceholder noResultsPlaceholder; private IEnumerable beatmapSets => root.Items.OfType(); @@ -128,9 +129,7 @@ namespace osu.Game.Screens.Select private void loadBeatmapSets(IEnumerable beatmapSets) { - originalBeatmapSetsDetached = beatmapSets.Detach(); - - if (selectedBeatmapSet != null && !originalBeatmapSetsDetached.Contains(selectedBeatmapSet.BeatmapSet)) + if (selectedBeatmapSet != null && !beatmapSets.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; var selectedBeatmapBefore = selectedBeatmap?.BeatmapInfo; @@ -139,7 +138,7 @@ namespace osu.Game.Screens.Select if (beatmapsSplitOut) { - var carouselBeatmapSets = originalBeatmapSetsDetached.SelectMany(s => s.Beatmaps).Select(b => + var carouselBeatmapSets = beatmapSets.SelectMany(s => s.Beatmaps).Select(b => { return createCarouselSet(new BeatmapSetInfo(new[] { b }) { @@ -153,7 +152,7 @@ namespace osu.Game.Screens.Select } else { - var carouselBeatmapSets = originalBeatmapSetsDetached.Select(createCarouselSet).OfType(); + var carouselBeatmapSets = beatmapSets.Select(createCarouselSet).OfType(); newRoot.AddItems(carouselBeatmapSets); } @@ -230,7 +229,7 @@ namespace osu.Game.Screens.Select } [BackgroundDependencyLoader] - private void load(OsuConfigManager config, AudioManager audio) + private void load(OsuConfigManager config, AudioManager audio, DetachedBeatmapStore beatmapStore) { spinSample = audio.Samples.Get("SongSelect/random-spin"); randomSelectSample = audio.Samples.Get(@"SongSelect/select-random"); @@ -246,18 +245,13 @@ namespace osu.Game.Screens.Select // This is performing an unnecessary second lookup on realm (in addition to the subscription), but for performance reasons // we require it to be separate: the subscription's initial callback (with `ChangeSet` of `null`) will run on the update // thread. If we attempt to detach beatmaps in this callback the game will fall over (it takes time). - realm.Run(r => loadBeatmapSets(getBeatmapSets(r))); + loadBeatmapSets(detachedBeatmapStore.GetDetachedBeatmaps()); } } [Resolved] private RealmAccess realm { get; set; } = null!; - /// - /// Track GUIDs of all sets in realm to allow handling deletions. - /// - private readonly List realmBeatmapSets = new List(); - protected override void LoadComplete() { base.LoadComplete(); @@ -266,6 +260,8 @@ namespace osu.Game.Screens.Select subscriptionBeatmaps = realm.RegisterForNotifications(r => r.All().Where(b => !b.Hidden), beatmapsChanged); } + private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); + private readonly HashSet setsRequiringUpdate = new HashSet(); private readonly HashSet setsRequiringRemoval = new HashSet(); @@ -275,65 +271,6 @@ namespace osu.Game.Screens.Select if (loadedTestBeatmaps) return; - if (changes == null) - { - realmBeatmapSets.Clear(); - realmBeatmapSets.AddRange(sender.Select(r => r.ID)); - - if (originalBeatmapSetsDetached.Count > 0 && sender.Count == 0) - { - // Usually we'd reset stuff here, but doing so triggers a silly flow which ends up deadlocking realm. - // Additionally, user should not be at song select when realm is blocking all operations in the first place. - // - // Note that due to the catch-up logic below, once operations are restored we will still be in a roughly - // correct state. The only things that this return will change is the carousel will not empty *during* the blocking - // operation. - return; - } - - // Do a full two-way check for missing (or incorrectly present) beatmaps. - // Let's assume that the worst that can happen is deletions or additions. - setsRequiringRemoval.Clear(); - setsRequiringUpdate.Clear(); - - foreach (Guid id in realmBeatmapSets) - { - if (!root.BeatmapSetsByID.ContainsKey(id)) - setsRequiringUpdate.Add(id); - } - - foreach (Guid id in root.BeatmapSetsByID.Keys) - { - if (!realmBeatmapSets.Contains(id)) - setsRequiringRemoval.Add(id); - } - } - else - { - foreach (int i in changes.DeletedIndices.OrderDescending()) - { - Guid id = realmBeatmapSets[i]; - - setsRequiringRemoval.Add(id); - setsRequiringUpdate.Remove(id); - - realmBeatmapSets.RemoveAt(i); - } - - foreach (int i in changes.InsertedIndices) - { - Guid id = sender[i].ID; - - setsRequiringRemoval.Remove(id); - setsRequiringUpdate.Add(id); - - realmBeatmapSets.Insert(i, id); - } - - foreach (int i in changes.NewModifiedIndices) - setsRequiringUpdate.Add(sender[i].ID); - } - Scheduler.AddOnce(processBeatmapChanges); } @@ -425,8 +362,6 @@ namespace osu.Game.Screens.Select invalidateAfterChange(); } - private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); - public void RemoveBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => { removeBeatmapSet(beatmapSet.ID); @@ -438,8 +373,6 @@ 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) @@ -465,9 +398,6 @@ namespace osu.Game.Screens.Select { beatmapSet = beatmapSet.Detach(); - originalBeatmapSetsDetached.RemoveAll(set => set.ID == beatmapSet.ID); - originalBeatmapSetsDetached.Add(beatmapSet); - var newSets = new List(); if (beatmapsSplitOut) @@ -766,7 +696,7 @@ namespace osu.Game.Screens.Select if (activeCriteria.SplitOutDifficulties != beatmapsSplitOut) { beatmapsSplitOut = activeCriteria.SplitOutDifficulties; - loadBeatmapSets(originalBeatmapSetsDetached); + loadBeatmapSets(detachedBeatmapStore.GetDetachedBeatmaps()); return; } From 4d42274771b770c4cb36e05a0611a2e20a4db324 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 27 Aug 2024 18:13:52 +0900 Subject: [PATCH 13/46] Use bindable list implementation --- osu.Game/Database/DetachedBeatmapStore.cs | 60 +++--------- osu.Game/Screens/Select/BeatmapCarousel.cs | 101 +++++++++++++-------- 2 files changed, 78 insertions(+), 83 deletions(-) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 0acc38a5a1..ff81784745 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -2,10 +2,10 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using osu.Framework.Allocation; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Logging; using osu.Game.Beatmaps; @@ -13,42 +13,36 @@ using Realms; namespace osu.Game.Database { - // TODO: handle realm migration public partial class DetachedBeatmapStore : Component { private readonly ManualResetEventSlim loaded = new ManualResetEventSlim(); - private List originalBeatmapSetsDetached = new List(); + private readonly BindableList detachedBeatmapSets = new BindableList(); - private IDisposable? subscriptionSets; - - /// - /// Track GUIDs of all sets in realm to allow handling deletions. - /// - private readonly List realmBeatmapSets = new List(); + private IDisposable? realmSubscription; [Resolved] private RealmAccess realm { get; set; } = null!; - public IReadOnlyList GetDetachedBeatmaps() + public IBindableList GetDetachedBeatmaps() { if (!loaded.Wait(60000)) Logger.Error(new TimeoutException("Beatmaps did not load in an acceptable time"), $"{nameof(DetachedBeatmapStore)} fell over"); - return originalBeatmapSetsDetached; + return detachedBeatmapSets.GetBoundCopy(); } [BackgroundDependencyLoader] private void load() { - subscriptionSets = realm.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); + realmSubscription = realm.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); } private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) { if (changes == null) { - if (originalBeatmapSetsDetached.Count > 0 && sender.Count == 0) + if (detachedBeatmapSets.Count > 0 && sender.Count == 0) { // Usually we'd reset stuff here, but doing so triggers a silly flow which ends up deadlocking realm. // Additionally, user should not be at song select when realm is blocking all operations in the first place. @@ -59,57 +53,29 @@ namespace osu.Game.Database return; } - originalBeatmapSetsDetached = sender.Detach(); - - realmBeatmapSets.Clear(); - realmBeatmapSets.AddRange(sender.Select(r => r.ID)); + detachedBeatmapSets.Clear(); + detachedBeatmapSets.AddRange(sender.Detach()); loaded.Set(); return; } - HashSet setsRequiringUpdate = new HashSet(); - HashSet setsRequiringRemoval = new HashSet(); - foreach (int i in changes.DeletedIndices.OrderDescending()) - { - Guid id = realmBeatmapSets[i]; - - setsRequiringRemoval.Add(id); - setsRequiringUpdate.Remove(id); - - realmBeatmapSets.RemoveAt(i); - } + detachedBeatmapSets.RemoveAt(i); foreach (int i in changes.InsertedIndices) { - Guid id = sender[i].ID; - - setsRequiringRemoval.Remove(id); - setsRequiringUpdate.Add(id); - - realmBeatmapSets.Insert(i, id); + detachedBeatmapSets.Insert(i, sender[i].Detach()); } foreach (int i in changes.NewModifiedIndices) - setsRequiringUpdate.Add(sender[i].ID); - - // deletions - foreach (Guid g in setsRequiringRemoval) - originalBeatmapSetsDetached.RemoveAll(set => set.ID == g); - - // updates - foreach (Guid g in setsRequiringUpdate) - { - originalBeatmapSetsDetached.RemoveAll(set => set.ID == g); - originalBeatmapSetsDetached.Add(fetchFromID(g)!); - } + detachedBeatmapSets.ReplaceRange(i, 1, new[] { sender[i].Detach() }); } protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - subscriptionSets?.Dispose(); + realmSubscription?.Dispose(); } private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index d06023258a..118ea45e45 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Diagnostics; using System.Linq; using osu.Framework.Allocation; @@ -21,6 +22,7 @@ using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Database; +using osu.Game.Extensions; using osu.Game.Graphics.Containers; using osu.Game.Input.Bindings; using osu.Game.Screens.Select.Carousel; @@ -108,7 +110,12 @@ namespace osu.Game.Screens.Select protected readonly CarouselScrollContainer Scroll; [Resolved] - private DetachedBeatmapStore detachedBeatmapStore { get; set; } = null!; + private RealmAccess realm { get; set; } = null!; + + [Resolved] + private DetachedBeatmapStore? detachedBeatmapStore { get; set; } + + private IBindableList detachedBeatmapSets = null!; private readonly NoResultsPlaceholder noResultsPlaceholder; @@ -165,12 +172,6 @@ namespace osu.Game.Screens.Select applyActiveCriteria(false); - if (loadedTestBeatmaps) - { - invalidateAfterChange(); - BeatmapSetsLoaded = true; - } - // Restore selection if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) { @@ -179,6 +180,12 @@ namespace osu.Game.Screens.Select if (found != null) found.State.Value = CarouselItemState.Selected; } + + Schedule(() => + { + invalidateAfterChange(); + BeatmapSetsLoaded = true; + }); } private readonly List visibleItems = new List(); @@ -194,7 +201,6 @@ namespace osu.Game.Screens.Select private CarouselRoot root; - private IDisposable? subscriptionSets; private IDisposable? subscriptionBeatmaps; private readonly DrawablePool setPool = new DrawablePool(100); @@ -245,32 +251,62 @@ namespace osu.Game.Screens.Select // This is performing an unnecessary second lookup on realm (in addition to the subscription), but for performance reasons // we require it to be separate: the subscription's initial callback (with `ChangeSet` of `null`) will run on the update // thread. If we attempt to detach beatmaps in this callback the game will fall over (it takes time). - loadBeatmapSets(detachedBeatmapStore.GetDetachedBeatmaps()); + detachedBeatmapSets = detachedBeatmapStore!.GetDetachedBeatmaps(); + detachedBeatmapSets.BindCollectionChanged(beatmapSetsChanged); + loadBeatmapSets(detachedBeatmapSets); } } - [Resolved] - private RealmAccess realm { get; set; } = null!; - protected override void LoadComplete() { base.LoadComplete(); - subscriptionSets = realm.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); subscriptionBeatmaps = realm.RegisterForNotifications(r => r.All().Where(b => !b.Hidden), beatmapsChanged); } - private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); + private readonly HashSet setsRequiringUpdate = new HashSet(); + private readonly HashSet setsRequiringRemoval = new HashSet(); - private readonly HashSet setsRequiringUpdate = new HashSet(); - private readonly HashSet setsRequiringRemoval = new HashSet(); - - private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) + private void beatmapSetsChanged(object? beatmaps, NotifyCollectionChangedEventArgs changed) { // If loading test beatmaps, avoid overwriting with realm subscription callbacks. if (loadedTestBeatmaps) return; + var newBeatmapSets = changed.NewItems!.Cast(); + var newBeatmapSetIDs = newBeatmapSets.Select(s => s.ID).ToHashSet(); + + var oldBeatmapSets = changed.OldItems!.Cast(); + var oldBeatmapSetIDs = oldBeatmapSets.Select(s => s.ID).ToHashSet(); + + switch (changed.Action) + { + case NotifyCollectionChangedAction.Add: + setsRequiringRemoval.RemoveWhere(s => newBeatmapSetIDs.Contains(s.ID)); + setsRequiringUpdate.AddRange(newBeatmapSets); + break; + + case NotifyCollectionChangedAction.Remove: + setsRequiringUpdate.RemoveWhere(s => oldBeatmapSetIDs.Contains(s.ID)); + setsRequiringRemoval.AddRange(oldBeatmapSets); + break; + + case NotifyCollectionChangedAction.Replace: + setsRequiringUpdate.AddRange(newBeatmapSets); + break; + + case NotifyCollectionChangedAction.Move: + setsRequiringUpdate.AddRange(newBeatmapSets); + break; + + case NotifyCollectionChangedAction.Reset: + setsRequiringRemoval.Clear(); + setsRequiringUpdate.Clear(); + + loadBeatmapSets(detachedBeatmapSets); + break; + } + Scheduler.AddOnce(processBeatmapChanges); } @@ -282,9 +318,10 @@ namespace osu.Game.Screens.Select { try { - foreach (var set in setsRequiringRemoval) removeBeatmapSet(set); + // TODO: chekc whether we still need beatmap sets by ID + foreach (var set in setsRequiringRemoval) removeBeatmapSet(set.ID); - foreach (var set in setsRequiringUpdate) updateBeatmapSet(fetchFromID(set)!); + foreach (var set in setsRequiringUpdate) updateBeatmapSet(set); if (setsRequiringRemoval.Count > 0 && SelectedBeatmapInfo != null) { @@ -302,7 +339,7 @@ namespace osu.Game.Screens.Select // This relies on the full update operation being in a single transaction, so please don't change that. foreach (var set in setsRequiringUpdate) { - foreach (var beatmapInfo in fetchFromID(set)!.Beatmaps) + foreach (var beatmapInfo in set.Beatmaps) { if (!((IBeatmapMetadataInfo)beatmapInfo.Metadata).Equals(SelectedBeatmapInfo.Metadata)) continue; @@ -317,7 +354,7 @@ namespace osu.Game.Screens.Select // If a direct selection couldn't be made, it's feasible that the difficulty name (or beatmap metadata) changed. // Let's attempt to follow set-level selection anyway. - SelectBeatmap(fetchFromID(setsRequiringUpdate.First())!.Beatmaps.First()); + SelectBeatmap(setsRequiringUpdate.First().Beatmaps.First()); } } } @@ -353,7 +390,7 @@ namespace osu.Game.Screens.Select if (root.BeatmapSetsByID.TryGetValue(beatmapSet.ID, out var existingSets) && existingSets.SelectMany(s => s.Beatmaps).All(b => b.BeatmapInfo.ID != beatmapInfo.ID)) { - updateBeatmapSet(beatmapSet.Detach()); + updateBeatmapSet(beatmapSet); changed = true; } } @@ -383,21 +420,14 @@ namespace osu.Game.Screens.Select } } - public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) + public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => { - beatmapSet = beatmapSet.Detach(); - - Schedule(() => - { - updateBeatmapSet(beatmapSet); - invalidateAfterChange(); - }); - } + updateBeatmapSet(beatmapSet); + invalidateAfterChange(); + }); private void updateBeatmapSet(BeatmapSetInfo beatmapSet) { - beatmapSet = beatmapSet.Detach(); - var newSets = new List(); if (beatmapsSplitOut) @@ -696,7 +726,7 @@ namespace osu.Game.Screens.Select if (activeCriteria.SplitOutDifficulties != beatmapsSplitOut) { beatmapsSplitOut = activeCriteria.SplitOutDifficulties; - loadBeatmapSets(detachedBeatmapStore.GetDetachedBeatmaps()); + loadBeatmapSets(detachedBeatmapSets); return; } @@ -1245,7 +1275,6 @@ namespace osu.Game.Screens.Select { base.Dispose(isDisposing); - subscriptionSets?.Dispose(); subscriptionBeatmaps?.Dispose(); } } From 081c9eb21bca77fb98094fe02463d01eb73b69d4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 16:17:03 +0900 Subject: [PATCH 14/46] Fix incorrect cancellation / disposal handling of `DetachedBeatmapStore` --- osu.Game/Database/DetachedBeatmapStore.cs | 8 +++----- osu.Game/Screens/Select/BeatmapCarousel.cs | 5 +++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index ff81784745..55ab836dd9 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -7,7 +7,6 @@ using System.Threading; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Logging; using osu.Game.Beatmaps; using Realms; @@ -24,11 +23,9 @@ namespace osu.Game.Database [Resolved] private RealmAccess realm { get; set; } = null!; - public IBindableList GetDetachedBeatmaps() + public IBindableList GetDetachedBeatmaps(CancellationToken cancellationToken) { - if (!loaded.Wait(60000)) - Logger.Error(new TimeoutException("Beatmaps did not load in an acceptable time"), $"{nameof(DetachedBeatmapStore)} fell over"); - + loaded.Wait(cancellationToken); return detachedBeatmapSets.GetBoundCopy(); } @@ -75,6 +72,7 @@ namespace osu.Game.Database protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); + loaded.Set(); realmSubscription?.Dispose(); } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 118ea45e45..94a6087741 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.Diagnostics; using System.Linq; +using System.Threading; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Audio.Sample; @@ -235,7 +236,7 @@ namespace osu.Game.Screens.Select } [BackgroundDependencyLoader] - private void load(OsuConfigManager config, AudioManager audio, DetachedBeatmapStore beatmapStore) + private void load(OsuConfigManager config, AudioManager audio, CancellationToken cancellationToken) { spinSample = audio.Samples.Get("SongSelect/random-spin"); randomSelectSample = audio.Samples.Get(@"SongSelect/select-random"); @@ -251,7 +252,7 @@ namespace osu.Game.Screens.Select // This is performing an unnecessary second lookup on realm (in addition to the subscription), but for performance reasons // we require it to be separate: the subscription's initial callback (with `ChangeSet` of `null`) will run on the update // thread. If we attempt to detach beatmaps in this callback the game will fall over (it takes time). - detachedBeatmapSets = detachedBeatmapStore!.GetDetachedBeatmaps(); + detachedBeatmapSets = detachedBeatmapStore!.GetDetachedBeatmaps(cancellationToken); detachedBeatmapSets.BindCollectionChanged(beatmapSetsChanged); loadBeatmapSets(detachedBeatmapSets); } From 81b36d897d5869184a1bc6b397718b71f4e143d6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 16:19:17 +0900 Subject: [PATCH 15/46] Fix null reference in change handling code --- osu.Game/Screens/Select/BeatmapCarousel.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 94a6087741..05e567b693 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -274,30 +274,31 @@ namespace osu.Game.Screens.Select if (loadedTestBeatmaps) return; - var newBeatmapSets = changed.NewItems!.Cast(); - var newBeatmapSetIDs = newBeatmapSets.Select(s => s.ID).ToHashSet(); - - var oldBeatmapSets = changed.OldItems!.Cast(); - var oldBeatmapSetIDs = oldBeatmapSets.Select(s => s.ID).ToHashSet(); + IEnumerable? newBeatmapSets = changed.NewItems?.Cast(); switch (changed.Action) { case NotifyCollectionChangedAction.Add: + HashSet newBeatmapSetIDs = newBeatmapSets!.Select(s => s.ID).ToHashSet(); + setsRequiringRemoval.RemoveWhere(s => newBeatmapSetIDs.Contains(s.ID)); - setsRequiringUpdate.AddRange(newBeatmapSets); + setsRequiringUpdate.AddRange(newBeatmapSets!); break; case NotifyCollectionChangedAction.Remove: + IEnumerable oldBeatmapSets = changed.OldItems!.Cast(); + HashSet oldBeatmapSetIDs = oldBeatmapSets.Select(s => s.ID).ToHashSet(); + setsRequiringUpdate.RemoveWhere(s => oldBeatmapSetIDs.Contains(s.ID)); setsRequiringRemoval.AddRange(oldBeatmapSets); break; case NotifyCollectionChangedAction.Replace: - setsRequiringUpdate.AddRange(newBeatmapSets); + setsRequiringUpdate.AddRange(newBeatmapSets!); break; case NotifyCollectionChangedAction.Move: - setsRequiringUpdate.AddRange(newBeatmapSets); + setsRequiringUpdate.AddRange(newBeatmapSets!); break; case NotifyCollectionChangedAction.Reset: From b1f653899c59cb8ef488e0e2ae09d44102f221db Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 16:30:09 +0900 Subject: [PATCH 16/46] Fix enumeration over modified collection --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 05e567b693..305deb4ba9 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -137,6 +137,10 @@ namespace osu.Game.Screens.Select private void loadBeatmapSets(IEnumerable beatmapSets) { + // Ensure no changes are made to the list while we are initialising items. + // We'll catch up on changes via subscriptions anyway. + beatmapSets = beatmapSets.ToArray(); + if (selectedBeatmapSet != null && !beatmapSets.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; From c2c1dccf2db2de855e9d76280eef4eed5cdcc845 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 17:46:36 +0900 Subject: [PATCH 17/46] Detach beatmap sets asynchronously --- osu.Game/Database/DetachedBeatmapStore.cs | 57 ++++++++++++++++++----- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 55ab836dd9..4e5ff23f7c 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -4,6 +4,7 @@ using System; using System.Linq; using System.Threading; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -30,9 +31,10 @@ namespace osu.Game.Database } [BackgroundDependencyLoader] - private void load() + private void load(CancellationToken cancellationToken) { - realmSubscription = realm.RegisterForNotifications(getBeatmapSets, beatmapSetsChanged); + realmSubscription = realm.RegisterForNotifications(r => r.All().Where(s => !s.DeletePending && !s.Protected), beatmapSetsChanged); + loaded.Wait(cancellationToken); } private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) @@ -50,23 +52,56 @@ namespace osu.Game.Database return; } - detachedBeatmapSets.Clear(); - detachedBeatmapSets.AddRange(sender.Detach()); + // Detaching beatmaps takes some time, so let's make sure it doesn't run on the update thread. + var frozenSets = sender.Freeze(); + + Task.Factory.StartNew(() => + { + realm.Run(_ => + { + var detached = frozenSets.Detach(); + + detachedBeatmapSets.Clear(); + detachedBeatmapSets.AddRange(detached); + loaded.Set(); + }); + }, TaskCreationOptions.LongRunning); - loaded.Set(); return; } foreach (int i in changes.DeletedIndices.OrderDescending()) - detachedBeatmapSets.RemoveAt(i); + removeAt(i); foreach (int i in changes.InsertedIndices) - { - detachedBeatmapSets.Insert(i, sender[i].Detach()); - } + insert(sender[i].Detach(), i); foreach (int i in changes.NewModifiedIndices) - detachedBeatmapSets.ReplaceRange(i, 1, new[] { sender[i].Detach() }); + replaceRange(sender[i].Detach(), i); + } + + private void replaceRange(BeatmapSetInfo set, int i) + { + if (loaded.IsSet) + detachedBeatmapSets.ReplaceRange(i, 1, new[] { set }); + else + Schedule(() => { detachedBeatmapSets.ReplaceRange(i, 1, new[] { set }); }); + } + + private void insert(BeatmapSetInfo set, int i) + { + if (loaded.IsSet) + detachedBeatmapSets.Insert(i, set); + else + Schedule(() => { detachedBeatmapSets.Insert(i, set); }); + } + + private void removeAt(int i) + { + if (loaded.IsSet) + detachedBeatmapSets.RemoveAt(i); + else + Schedule(() => { detachedBeatmapSets.RemoveAt(i); }); } protected override void Dispose(bool isDisposing) @@ -75,7 +110,5 @@ namespace osu.Game.Database loaded.Set(); realmSubscription?.Dispose(); } - - private IQueryable getBeatmapSets(Realm realm) => realm.All().Where(s => !s.DeletePending && !s.Protected); } } From 5ed0c6e91a9ea05c751ec9bb81b56bf17b919400 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 17:48:17 +0900 Subject: [PATCH 18/46] Remove song select preloading Really unnecessary now. --- osu.Game/Screens/Menu/MainMenu.cs | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs index dfe5460aee..c1d502bd41 100644 --- a/osu.Game/Screens/Menu/MainMenu.cs +++ b/osu.Game/Screens/Menu/MainMenu.cs @@ -54,8 +54,6 @@ namespace osu.Game.Screens.Menu public override bool? AllowGlobalTrackControl => true; - private Screen songSelect; - private MenuSideFlashes sideFlashes; protected ButtonSystem Buttons; @@ -220,26 +218,11 @@ namespace osu.Game.Screens.Menu Buttons.OnBeatmapListing = () => beatmapListing?.ToggleVisibility(); reappearSampleSwoosh = audio.Samples.Get(@"Menu/reappear-swoosh"); - - preloadSongSelect(); } public void ReturnToOsuLogo() => Buttons.State = ButtonSystemState.Initial; - private void preloadSongSelect() - { - if (songSelect == null) - LoadComponentAsync(songSelect = new PlaySongSelect()); - } - - private void loadSoloSongSelect() => this.Push(consumeSongSelect()); - - private Screen consumeSongSelect() - { - var s = songSelect; - songSelect = null; - return s; - } + private void loadSoloSongSelect() => this.Push(new PlaySongSelect()); public override void OnEntering(ScreenTransitionEvent e) { @@ -373,9 +356,6 @@ namespace osu.Game.Screens.Menu ApplyToBackground(b => (b as BackgroundScreenDefault)?.Next()); - // we may have consumed our preloaded instance, so let's make another. - preloadSongSelect(); - musicController.EnsurePlayingSomething(); // Cycle tip on resuming From 336abadbd1b9e36f2aa2f2ea6c05916494a19685 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 18:22:05 +0900 Subject: [PATCH 19/46] Allow running initial filter criteria asynchronously This reverts a portion of https://github.com/ppy/osu/pull/9539. The rearrangement in `SongSelect` is required to get the initial filter into `BeatmapCarousel` (and avoid the `FilterChanged` event firing, causing a delayed/scheduled filter application). --- .../SongSelect/TestSceneBeatmapCarousel.cs | 5 +++ .../TestSceneUpdateBeatmapSetButton.cs | 2 +- osu.Game/Screens/Select/BeatmapCarousel.cs | 9 ++--- osu.Game/Screens/Select/SongSelect.cs | 35 ++++++++++--------- 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index c0102b238c..24be242013 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -1389,6 +1389,11 @@ namespace osu.Game.Tests.Visual.SongSelect private partial class TestBeatmapCarousel : BeatmapCarousel { + public TestBeatmapCarousel() + : base(new FilterCriteria()) + { + } + public bool PendingFilterTask => PendingFilter != null; public IEnumerable Items diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs index 6d97be730b..0b0cd0317a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneUpdateBeatmapSetButton.cs @@ -246,7 +246,7 @@ namespace osu.Game.Tests.Visual.SongSelect private BeatmapCarousel createCarousel() { - return carousel = new BeatmapCarousel + return carousel = new BeatmapCarousel(new FilterCriteria()) { RelativeSizeAxes = Axes.Both, BeatmapSets = new List diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 305deb4ba9..5e79a8202e 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -170,13 +170,12 @@ namespace osu.Game.Screens.Select } root = newRoot; + root.Filter(activeCriteria); Scroll.Clear(false); itemsCache.Invalidate(); ScrollToSelected(); - applyActiveCriteria(false); - // Restore selection if (selectedBeatmapBefore != null && newRoot.BeatmapSetsByID.TryGetValue(selectedBeatmapBefore.BeatmapSet!.ID, out var newSelectionCandidates)) { @@ -215,7 +214,7 @@ namespace osu.Game.Screens.Select private int visibleSetsCount; - public BeatmapCarousel() + public BeatmapCarousel(FilterCriteria initialCriterial) { root = new CarouselRoot(this); InternalChild = new Container @@ -237,6 +236,8 @@ namespace osu.Game.Screens.Select noResultsPlaceholder = new NoResultsPlaceholder() } }; + + activeCriteria = initialCriterial; } [BackgroundDependencyLoader] @@ -662,7 +663,7 @@ namespace osu.Game.Screens.Select item.State.Value = CarouselItemState.Selected; } - private FilterCriteria activeCriteria = new FilterCriteria(); + private FilterCriteria activeCriteria; protected ScheduledDelegate? PendingFilter; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 2965aa383d..3cfc7623b9 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -162,20 +162,6 @@ namespace osu.Game.Screens.Select ApplyToBackground(applyBlurToBackground); }); - LoadComponentAsync(Carousel = new BeatmapCarousel - { - AllowSelection = false, // delay any selection until our bindables are ready to make a good choice. - Anchor = Anchor.CentreRight, - Origin = Anchor.CentreRight, - RelativeSizeAxes = Axes.Both, - BleedTop = FilterControl.HEIGHT, - BleedBottom = Select.Footer.HEIGHT, - SelectionChanged = updateSelectedBeatmap, - BeatmapSetsChanged = carouselBeatmapsLoaded, - FilterApplied = () => Scheduler.AddOnce(updateVisibleBeatmapCount), - GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s), - }, c => carouselContainer.Child = c); - // initial value transfer is required for FilterControl (it uses our re-cached bindables in its async load for the initial filter). transferRulesetValue(); @@ -227,7 +213,6 @@ namespace osu.Game.Screens.Select { RelativeSizeAxes = Axes.X, Height = FilterControl.HEIGHT, - FilterChanged = ApplyFilterToCarousel, }, new GridContainer // used for max width implementation { @@ -328,6 +313,23 @@ namespace osu.Game.Screens.Select modSpeedHotkeyHandler = new ModSpeedHotkeyHandler(), }); + // Important to load this after the filter control is loaded (so we have initial filter criteria prepared). + LoadComponentAsync(Carousel = new BeatmapCarousel(FilterControl.CreateCriteria()) + { + AllowSelection = false, // delay any selection until our bindables are ready to make a good choice. + Anchor = Anchor.CentreRight, + Origin = Anchor.CentreRight, + RelativeSizeAxes = Axes.Both, + BleedTop = FilterControl.HEIGHT, + BleedBottom = Select.Footer.HEIGHT, + SelectionChanged = updateSelectedBeatmap, + BeatmapSetsChanged = carouselBeatmapsLoaded, + FilterApplied = () => Scheduler.AddOnce(updateVisibleBeatmapCount), + GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s), + }, c => carouselContainer.Child = c); + + FilterControl.FilterChanged = ApplyFilterToCarousel; + if (ShowSongSelectFooter) { AddRangeInternal(new Drawable[] @@ -992,7 +994,8 @@ namespace osu.Game.Screens.Select // if we have a pending filter operation, we want to run it now. // it could change selection (ie. if the ruleset has been changed). - Carousel.FlushPendingFilterOperations(); + if (IsLoaded) + Carousel.FlushPendingFilterOperations(); return true; } From dd4a1104e45ddd50015608555227fe17afdb6754 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 18:56:09 +0900 Subject: [PATCH 20/46] Always debounce external `Filter` requests (except for tests) The only exception to the rule here was "when screen isn't active apply without debounce" but I'm not sure we want this. It would cause a stutter on returning to song select and I'm not even sure this is a common scenario. I'd rather remove it and see if someone finds an actual case where this is an issue. --- .../SongSelect/TestSceneBeatmapCarousel.cs | 88 ++++++++++--------- .../SongSelect/TestScenePlaySongSelect.cs | 12 ++- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 +- osu.Game/Screens/Select/SongSelect.cs | 10 +-- 4 files changed, 55 insertions(+), 59 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 24be242013..a075559f6a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -52,11 +52,11 @@ namespace osu.Game.Tests.Visual.SongSelect { createCarousel(new List()); - AddStep("filter to ruleset 0", () => carousel.Filter(new FilterCriteria + AddStep("filter to ruleset 0", () => carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0), AllowConvertedBeatmaps = true, - }, false)); + })); AddStep("add mixed ruleset beatmapset", () => { @@ -78,11 +78,11 @@ namespace osu.Game.Tests.Visual.SongSelect && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item)!.BeatmapInfo.Ruleset.OnlineID == 0) == 1; }); - AddStep("filter to ruleset 1", () => carousel.Filter(new FilterCriteria + AddStep("filter to ruleset 1", () => carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1), AllowConvertedBeatmaps = true, - }, false)); + })); AddUntilStep("wait for filtered difficulties", () => { @@ -93,11 +93,11 @@ namespace osu.Game.Tests.Visual.SongSelect && visibleBeatmapPanels.Count(p => ((CarouselBeatmap)p.Item)!.BeatmapInfo.Ruleset.OnlineID == 1) == 1; }); - AddStep("filter to ruleset 2", () => carousel.Filter(new FilterCriteria + AddStep("filter to ruleset 2", () => carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(2), AllowConvertedBeatmaps = true, - }, false)); + })); AddUntilStep("wait for filtered difficulties", () => { @@ -344,7 +344,7 @@ namespace osu.Game.Tests.Visual.SongSelect // basic filtering setSelected(1, 1); - AddStep("Filter", () => carousel.Filter(new FilterCriteria { SearchText = carousel.BeatmapSets.ElementAt(2).Metadata.Title }, false)); + AddStep("Filter", () => carousel.FilterImmediately(new FilterCriteria { SearchText = carousel.BeatmapSets.ElementAt(2).Metadata.Title })); checkVisibleItemCount(diff: false, count: 1); checkVisibleItemCount(diff: true, count: 3); waitForSelection(3, 1); @@ -360,13 +360,13 @@ namespace osu.Game.Tests.Visual.SongSelect // test filtering some difficulties (and keeping current beatmap set selected). setSelected(1, 2); - AddStep("Filter some difficulties", () => carousel.Filter(new FilterCriteria { SearchText = "Normal" }, false)); + AddStep("Filter some difficulties", () => carousel.FilterImmediately(new FilterCriteria { SearchText = "Normal" })); waitForSelection(1, 1); - AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); + AddStep("Un-filter", () => carousel.FilterImmediately(new FilterCriteria())); waitForSelection(1, 1); - AddStep("Filter all", () => carousel.Filter(new FilterCriteria { SearchText = "Dingo" }, false)); + AddStep("Filter all", () => carousel.FilterImmediately(new FilterCriteria { SearchText = "Dingo" })); checkVisibleItemCount(false, 0); checkVisibleItemCount(true, 0); @@ -378,7 +378,7 @@ namespace osu.Game.Tests.Visual.SongSelect advanceSelection(false); AddAssert("Selection is null", () => currentSelection == null); - AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); + AddStep("Un-filter", () => carousel.FilterImmediately(new FilterCriteria())); AddAssert("Selection is non-null", () => currentSelection != null); @@ -399,7 +399,7 @@ namespace osu.Game.Tests.Visual.SongSelect setSelected(1, 3); - AddStep("Apply a range filter", () => carousel.Filter(new FilterCriteria + AddStep("Apply a range filter", () => carousel.FilterImmediately(new FilterCriteria { SearchText = searchText, StarDifficulty = new FilterCriteria.OptionalRange @@ -408,7 +408,7 @@ namespace osu.Game.Tests.Visual.SongSelect Max = 5.5, IsLowerInclusive = true } - }, false)); + })); // should reselect the buffered selection. waitForSelection(3, 2); @@ -445,13 +445,13 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("ensure repeat", () => selectedSets.Contains(carousel.SelectedBeatmapSet)); AddStep("Add set with 100 difficulties", () => carousel.UpdateBeatmapSet(TestResources.CreateTestBeatmapSetInfo(100, rulesets.AvailableRulesets.ToArray()))); - AddStep("Filter Extra", () => carousel.Filter(new FilterCriteria { SearchText = "Extra 10" }, false)); + AddStep("Filter Extra", () => carousel.FilterImmediately(new FilterCriteria { SearchText = "Extra 10" })); checkInvisibleDifficultiesUnselectable(); checkInvisibleDifficultiesUnselectable(); checkInvisibleDifficultiesUnselectable(); checkInvisibleDifficultiesUnselectable(); checkInvisibleDifficultiesUnselectable(); - AddStep("Un-filter", () => carousel.Filter(new FilterCriteria(), false)); + AddStep("Un-filter", () => carousel.FilterImmediately(new FilterCriteria())); } [Test] @@ -527,7 +527,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(setCount: local_set_count, diffCount: local_diff_count); - AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + AddStep("Sort by difficulty", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Difficulty })); checkVisibleItemCount(false, local_set_count * local_diff_count); @@ -566,7 +566,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets, () => new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) }); AddStep("Set non-empty mode filter", () => - carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1) }, false)); + carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1) })); AddAssert("Something is selected", () => carousel.SelectedBeatmapInfo != null); } @@ -601,7 +601,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by date submitted", () => carousel.Filter(new FilterCriteria { Sort = SortMode.DateSubmitted }, false)); + AddStep("Sort by date submitted", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.DateSubmitted })); checkVisibleItemCount(diff: false, count: 10); checkVisibleItemCount(diff: true, count: 5); @@ -610,11 +610,11 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("rest are at start", () => carousel.Items.OfType().TakeWhile(i => i.Item is CarouselBeatmapSet s && s.BeatmapSet.DateSubmitted != null).Count(), () => Is.EqualTo(6)); - AddStep("Sort by date submitted and string", () => carousel.Filter(new FilterCriteria + AddStep("Sort by date submitted and string", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.DateSubmitted, SearchText = zzz_string - }, false)); + })); checkVisibleItemCount(diff: false, count: 5); checkVisibleItemCount(diff: true, count: 5); @@ -658,10 +658,10 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by author", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Author }, false)); + AddStep("Sort by author", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Author })); AddAssert($"Check {zzz_uppercase} is last", () => carousel.BeatmapSets.Last().Metadata.Author.Username == zzz_uppercase); AddAssert($"Check {zzz_lowercase} is second last", () => carousel.BeatmapSets.SkipLast(1).Last().Metadata.Author.Username == zzz_lowercase); - AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddStep("Sort by artist", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Artist })); AddAssert($"Check {zzz_uppercase} is last", () => carousel.BeatmapSets.Last().Metadata.Artist == zzz_uppercase); AddAssert($"Check {zzz_lowercase} is second last", () => carousel.BeatmapSets.SkipLast(1).Last().Metadata.Artist == zzz_lowercase); } @@ -703,7 +703,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddStep("Sort by artist", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Artist })); AddAssert("Check last item", () => { var lastItem = carousel.BeatmapSets.Last(); @@ -746,10 +746,10 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); + AddStep("Sort by title", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Title })); AddAssert("Items remain in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending); - AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddStep("Sort by artist", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Artist })); AddAssert("Items remain in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending); } @@ -786,7 +786,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddStep("Sort by artist", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Artist })); AddAssert("Items in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending); AddStep("Save order", () => originalOrder = carousel.BeatmapSets.Select(s => s.ID).ToArray()); @@ -796,7 +796,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder)); - AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); + AddStep("Sort by title", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Title })); AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder)); } @@ -833,7 +833,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); + AddStep("Sort by artist", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Artist })); AddAssert("Items in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending); AddStep("Save order", () => originalOrder = carousel.BeatmapSets.Select(s => s.ID).ToArray()); @@ -858,7 +858,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder)); - AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false)); + AddStep("Sort by title", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Title })); AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder)); } @@ -885,12 +885,12 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); - AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + AddStep("Sort by difficulty", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Difficulty })); 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)); + AddStep("Filter to normal", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Normal" })); checkVisibleItemCount(false, local_set_count); checkVisibleItemCount(true, 1); @@ -901,7 +901,7 @@ namespace osu.Game.Tests.Visual.SongSelect .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)); + AddStep("Filter to insane", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Difficulty, SearchText = "Insane" })); checkVisibleItemCount(false, local_set_count); checkVisibleItemCount(true, 1); @@ -1022,7 +1022,7 @@ namespace osu.Game.Tests.Visual.SongSelect carousel.UpdateBeatmapSet(testMixed); }); AddStep("filter to ruleset 0", () => - carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) }, false)); + carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(0) })); AddStep("select filtered map skipping filtered", () => carousel.SelectBeatmap(testMixed.Beatmaps[1], false)); AddAssert("unfiltered beatmap not selected", () => carousel.SelectedBeatmapInfo?.Ruleset.OnlineID == 0); @@ -1068,12 +1068,12 @@ namespace osu.Game.Tests.Visual.SongSelect { AddStep("Toggle non-matching filter", () => { - carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false); + carousel.FilterImmediately(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }); }); AddStep("Restore no filter", () => { - carousel.Filter(new FilterCriteria(), false); + carousel.FilterImmediately(new FilterCriteria()); eagerSelectedIDs.Add(carousel.SelectedBeatmapSet!.ID); }); } @@ -1097,7 +1097,7 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(manySets); - AddStep("Sort by difficulty", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Difficulty }, false)); + AddStep("Sort by difficulty", () => carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Difficulty })); advanceSelection(direction: 1, diff: false); @@ -1105,12 +1105,12 @@ namespace osu.Game.Tests.Visual.SongSelect { AddStep("Toggle non-matching filter", () => { - carousel.Filter(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }, false); + carousel.FilterImmediately(new FilterCriteria { SearchText = Guid.NewGuid().ToString() }); }); AddStep("Restore no filter", () => { - carousel.Filter(new FilterCriteria(), false); + carousel.FilterImmediately(new FilterCriteria()); eagerSelectedIDs.Add(carousel.SelectedBeatmapSet!.ID); }); } @@ -1185,7 +1185,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep($"Set ruleset to {rulesetInfo.ShortName}", () => { - carousel.Filter(new FilterCriteria { Ruleset = rulesetInfo, Sort = SortMode.Title }, false); + carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesetInfo, Sort = SortMode.Title }); }); waitForSelection(i + 1, 1); } @@ -1223,12 +1223,12 @@ namespace osu.Game.Tests.Visual.SongSelect setSelected(i, 1); AddStep("Set ruleset to taiko", () => { - carousel.Filter(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1), Sort = SortMode.Title }, false); + carousel.FilterImmediately(new FilterCriteria { Ruleset = rulesets.AvailableRulesets.ElementAt(1), Sort = SortMode.Title }); }); waitForSelection(i - 1, 1); AddStep("Remove ruleset filter", () => { - carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false); + carousel.FilterImmediately(new FilterCriteria { Sort = SortMode.Title }); }); } @@ -1415,6 +1415,12 @@ namespace osu.Game.Tests.Visual.SongSelect } } } + + public void FilterImmediately(FilterCriteria newCriteria) + { + Filter(newCriteria); + FlushPendingFilterOperations(); + } } } } diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 4c6a5c93d9..9df26e0da5 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -1397,8 +1397,6 @@ namespace osu.Game.Tests.Visual.SongSelect { public Action? StartRequested; - public new Bindable Ruleset => base.Ruleset; - public new FilterControl FilterControl => base.FilterControl; public WorkingBeatmap CurrentBeatmap => Beatmap.Value; @@ -1408,18 +1406,18 @@ namespace osu.Game.Tests.Visual.SongSelect public new void PresentScore(ScoreInfo score) => base.PresentScore(score); + public int FilterCount; + protected override bool OnStart() { StartRequested?.Invoke(); return base.OnStart(); } - public int FilterCount; - - protected override void ApplyFilterToCarousel(FilterCriteria criteria) + [BackgroundDependencyLoader] + private void load() { - FilterCount++; - base.ApplyFilterToCarousel(criteria); + FilterControl.FilterChanged += _ => FilterCount++; } } } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 5e79a8202e..ddc8f22c95 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -700,12 +700,12 @@ namespace osu.Game.Screens.Select } } - public void Filter(FilterCriteria? newCriteria, bool debounce = true) + public void Filter(FilterCriteria? newCriteria) { if (newCriteria != null) activeCriteria = newCriteria; - applyActiveCriteria(debounce); + applyActiveCriteria(true); } private bool beatmapsSplitOut; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 3cfc7623b9..bfbc50378a 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -328,7 +328,7 @@ namespace osu.Game.Screens.Select GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s), }, c => carouselContainer.Child = c); - FilterControl.FilterChanged = ApplyFilterToCarousel; + FilterControl.FilterChanged = Carousel.Filter; if (ShowSongSelectFooter) { @@ -403,14 +403,6 @@ namespace osu.Game.Screens.Select protected virtual ModSelectOverlay CreateModSelectOverlay() => new SoloModSelectOverlay(); - protected virtual void ApplyFilterToCarousel(FilterCriteria criteria) - { - // if not the current screen, we want to get carousel in a good presentation state before displaying (resume or enter). - bool shouldDebounce = this.IsCurrentScreen(); - - Carousel.Filter(criteria, shouldDebounce); - } - private DependencyContainer dependencies = null!; protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) From 9123d2cb7f18962f805b8a941f762f1c6583b73a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 19:19:04 +0900 Subject: [PATCH 21/46] Fix multiple test failures --- .../Visual/Background/TestSceneUserDimBackgrounds.cs | 6 ++++++ osu.Game.Tests/Visual/Multiplayer/QueueModeTestScene.cs | 6 ++++++ osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs | 6 ++++++ .../Multiplayer/TestSceneMultiplayerMatchSongSelect.cs | 5 +++++ .../Visual/Multiplayer/TestScenePlaylistsSongSelect.cs | 6 ++++++ .../Visual/Navigation/TestScenePresentBeatmap.cs | 6 ++++++ .../Visual/SongSelect/TestScenePlaySongSelect.cs | 4 ++++ osu.Game/Database/DetachedBeatmapStore.cs | 7 +++---- osu.Game/Screens/Select/BeatmapCarousel.cs | 6 +++--- 9 files changed, 45 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs index aac7689b1b..d8be57382f 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneUserDimBackgrounds.cs @@ -18,6 +18,7 @@ using osu.Framework.Screens; using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; +using osu.Game.Database; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Graphics.Sprites; @@ -48,13 +49,18 @@ namespace osu.Game.Tests.Visual.Background [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + Dependencies.Cache(rulesets = new RealmRulesetStore(Realm)); Dependencies.Cache(manager = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default)); Dependencies.Cache(new OsuConfigManager(LocalStorage)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(Realm); manager.Import(TestResources.GetQuickTestBeatmapForImport()).WaitSafely(); + Add(detachedBeatmapStore); + Beatmap.SetDefault(); } diff --git a/osu.Game.Tests/Visual/Multiplayer/QueueModeTestScene.cs b/osu.Game.Tests/Visual/Multiplayer/QueueModeTestScene.cs index 0f1ba9ba75..8bcd5aab1c 100644 --- a/osu.Game.Tests/Visual/Multiplayer/QueueModeTestScene.cs +++ b/osu.Game.Tests/Visual/Multiplayer/QueueModeTestScene.cs @@ -12,6 +12,7 @@ using osu.Framework.Platform; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Online.Multiplayer; using osu.Game.Online.Rooms; using osu.Game.Rulesets; @@ -45,9 +46,14 @@ namespace osu.Game.Tests.Visual.Multiplayer [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + Dependencies.Cache(new RealmRulesetStore(Realm)); Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(Realm); + + Add(detachedBeatmapStore); } public override void SetUpSteps() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index ad7e211354..df2021dbaf 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -19,6 +19,7 @@ using osu.Framework.Testing; using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; +using osu.Game.Database; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Online.API.Requests.Responses; @@ -65,9 +66,14 @@ namespace osu.Game.Tests.Visual.Multiplayer [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + Dependencies.Cache(new RealmRulesetStore(Realm)); Dependencies.Cache(beatmaps = new BeatmapManager(LocalStorage, Realm, API, audio, Resources, host, Beatmap.Default)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(Realm); + + Add(detachedBeatmapStore); } public override void SetUpSteps() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs index 8dc41cd707..88cc7eb9b3 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchSongSelect.cs @@ -45,11 +45,16 @@ namespace osu.Game.Tests.Visual.Multiplayer [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + Dependencies.Cache(rulesets = new RealmRulesetStore(Realm)); Dependencies.Cache(manager = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(Realm); importedBeatmapSet = manager.Import(TestResources.CreateTestBeatmapSetInfo(8, rulesets.AvailableRulesets.ToArray())); + + Add(detachedBeatmapStore); } public override void SetUpSteps() diff --git a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs index b0b753fc22..cc78bed5de 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestScenePlaylistsSongSelect.cs @@ -12,6 +12,7 @@ using osu.Framework.Platform; using osu.Framework.Screens; using osu.Framework.Utils; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Online.Rooms; using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; @@ -33,13 +34,18 @@ namespace osu.Game.Tests.Visual.Multiplayer [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + Dependencies.Cache(new RealmRulesetStore(Realm)); Dependencies.Cache(manager = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, Beatmap.Default)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(Realm); var beatmapSet = TestResources.CreateTestBeatmapSetInfo(); manager.Import(beatmapSet); + + Add(detachedBeatmapStore); } public override void SetUpSteps() diff --git a/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs b/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs index c054792168..fc711473f2 100644 --- a/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs +++ b/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs @@ -176,6 +176,12 @@ namespace osu.Game.Tests.Visual.Navigation private void confirmBeatmapInSongSelect(Func getImport) { + AddUntilStep("wait for carousel loaded", () => + { + var songSelect = (Screens.Select.SongSelect)Game.ScreenStack.CurrentScreen; + return songSelect.ChildrenOfType().SingleOrDefault()?.IsLoaded == true; + }); + AddUntilStep("beatmap in song select", () => { var songSelect = (Screens.Select.SongSelect)Game.ScreenStack.CurrentScreen; diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 9df26e0da5..1f298d2d2d 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -56,16 +56,20 @@ namespace osu.Game.Tests.Visual.SongSelect [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { + DetachedBeatmapStore detachedBeatmapStore; + // These DI caches are required to ensure for interactive runs this test scene doesn't nuke all user beatmaps in the local install. // At a point we have isolated interactive test runs enough, this can likely be removed. Dependencies.Cache(rulesets = new RealmRulesetStore(Realm)); Dependencies.Cache(Realm); Dependencies.Cache(manager = new BeatmapManager(LocalStorage, Realm, null, audio, Resources, host, defaultBeatmap = Beatmap.Default)); + Dependencies.Cache(detachedBeatmapStore = new DetachedBeatmapStore()); Dependencies.Cache(music = new MusicController()); // required to get bindables attached Add(music); + Add(detachedBeatmapStore); Dependencies.Cache(config = new OsuConfigManager(LocalStorage)); } diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 4e5ff23f7c..39f0bdaafe 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -24,17 +24,16 @@ namespace osu.Game.Database [Resolved] private RealmAccess realm { get; set; } = null!; - public IBindableList GetDetachedBeatmaps(CancellationToken cancellationToken) + public IBindableList GetDetachedBeatmaps(CancellationToken? cancellationToken) { - loaded.Wait(cancellationToken); + loaded.Wait(cancellationToken ?? CancellationToken.None); return detachedBeatmapSets.GetBoundCopy(); } [BackgroundDependencyLoader] - private void load(CancellationToken cancellationToken) + private void load() { realmSubscription = realm.RegisterForNotifications(r => r.All().Where(s => !s.DeletePending && !s.Protected), beatmapSetsChanged); - loaded.Wait(cancellationToken); } private void beatmapSetsChanged(IRealmCollection sender, ChangeSet? changes) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index ddc8f22c95..63b2bcf7b1 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -241,7 +241,7 @@ namespace osu.Game.Screens.Select } [BackgroundDependencyLoader] - private void load(OsuConfigManager config, AudioManager audio, CancellationToken cancellationToken) + private void load(OsuConfigManager config, AudioManager audio, CancellationToken? cancellationToken) { spinSample = audio.Samples.Get("SongSelect/random-spin"); randomSelectSample = audio.Samples.Get(@"SongSelect/select-random"); @@ -252,12 +252,12 @@ namespace osu.Game.Screens.Select RightClickScrollingEnabled.ValueChanged += enabled => Scroll.RightMouseScrollbar = enabled.NewValue; RightClickScrollingEnabled.TriggerChange(); - if (!loadedTestBeatmaps) + if (!loadedTestBeatmaps && detachedBeatmapStore != null) { // This is performing an unnecessary second lookup on realm (in addition to the subscription), but for performance reasons // we require it to be separate: the subscription's initial callback (with `ChangeSet` of `null`) will run on the update // thread. If we attempt to detach beatmaps in this callback the game will fall over (it takes time). - detachedBeatmapSets = detachedBeatmapStore!.GetDetachedBeatmaps(cancellationToken); + detachedBeatmapSets = detachedBeatmapStore.GetDetachedBeatmaps(cancellationToken); detachedBeatmapSets.BindCollectionChanged(beatmapSetsChanged); loadBeatmapSets(detachedBeatmapSets); } From 853023dfbac4a328d9281eb229fc51d917c84bbe Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 20:14:33 +0900 Subject: [PATCH 22/46] Reduce test filter count expectation by one due to initial filter being implicit --- .../SongSelect/TestScenePlaySongSelect.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 1f298d2d2d..6b8fa94336 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -246,7 +246,7 @@ namespace osu.Game.Tests.Visual.SongSelect createSongSelect(); - AddAssert("filter count is 1", () => songSelect?.FilterCount == 1); + AddAssert("filter count is 0", () => songSelect?.FilterCount, () => Is.EqualTo(0)); } [Test] @@ -366,7 +366,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("return", () => songSelect!.MakeCurrent()); AddUntilStep("wait for current", () => songSelect!.IsCurrentScreen()); - AddAssert("filter count is 1", () => songSelect!.FilterCount == 1); + AddAssert("filter count is 0", () => songSelect!.FilterCount, () => Is.EqualTo(0)); } [Test] @@ -386,7 +386,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("return", () => songSelect!.MakeCurrent()); AddUntilStep("wait for current", () => songSelect!.IsCurrentScreen()); - AddAssert("filter count is 2", () => songSelect!.FilterCount == 2); + AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); } [Test] @@ -1274,11 +1274,11 @@ namespace osu.Game.Tests.Visual.SongSelect // Mod that is guaranteed to never re-filter. AddStep("add non-filterable mod", () => SelectedMods.Value = new Mod[] { new OsuModCinema() }); - AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); + AddAssert("filter count is 0", () => songSelect!.FilterCount, () => Is.EqualTo(0)); // Removing the mod should still not re-filter. AddStep("remove non-filterable mod", () => SelectedMods.Value = Array.Empty()); - AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); + AddAssert("filter count is 0", () => songSelect!.FilterCount, () => Is.EqualTo(0)); } [Test] @@ -1290,35 +1290,35 @@ namespace osu.Game.Tests.Visual.SongSelect // Change to mania ruleset. AddStep("filter to mania ruleset", () => Ruleset.Value = rulesets.AvailableRulesets.First(r => r.OnlineID == 3)); - AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(2)); + AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(1)); // Apply a mod, but this should NOT re-filter because there's no search text. AddStep("add filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey3() }); - AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(2)); + AddAssert("filter count is 1", () => songSelect!.FilterCount, () => Is.EqualTo(1)); // Set search text. Should re-filter. AddStep("set search text to match mods", () => songSelect!.FilterControl.CurrentTextSearch.Value = "keys=3"); - AddAssert("filter count is 3", () => songSelect!.FilterCount, () => Is.EqualTo(3)); + AddAssert("filter count is 2", () => songSelect!.FilterCount, () => Is.EqualTo(2)); // Change filterable mod. Should re-filter. AddStep("change new filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey5() }); - AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); + AddAssert("filter count is 3", () => songSelect!.FilterCount, () => Is.EqualTo(3)); // Add non-filterable mod. Should NOT re-filter. AddStep("apply non-filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModNoFail(), new ManiaModKey5() }); - AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); + AddAssert("filter count is 3", () => songSelect!.FilterCount, () => Is.EqualTo(3)); // Remove filterable mod. Should re-filter. AddStep("remove filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModNoFail() }); - AddAssert("filter count is 5", () => songSelect!.FilterCount, () => Is.EqualTo(5)); + AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); // Remove non-filterable mod. Should NOT re-filter. AddStep("remove filterable mod", () => SelectedMods.Value = Array.Empty()); - AddAssert("filter count is 5", () => songSelect!.FilterCount, () => Is.EqualTo(5)); + AddAssert("filter count is 4", () => songSelect!.FilterCount, () => Is.EqualTo(4)); // Add filterable mod. Should re-filter. AddStep("add filterable mod", () => SelectedMods.Value = new Mod[] { new ManiaModKey3() }); - AddAssert("filter count is 6", () => songSelect!.FilterCount, () => Is.EqualTo(6)); + AddAssert("filter count is 5", () => songSelect!.FilterCount, () => Is.EqualTo(5)); } private void waitForInitialSelection() From e04b5bb3f260dd32794c00081263b6f7f61b3791 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 19:35:28 +0900 Subject: [PATCH 23/46] Tidy up test beatmap loading --- .../SongSelect/TestSceneBeatmapCarousel.cs | 16 ++++++------- osu.Game/Screens/Select/BeatmapCarousel.cs | 23 +++++++++++-------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index a075559f6a..ec072a3dd2 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Linq; +using JetBrains.Annotations; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; @@ -1268,26 +1269,23 @@ namespace osu.Game.Tests.Visual.SongSelect } } - createCarousel(beatmapSets, c => + createCarousel(beatmapSets, initialCriteria, c => { - carouselAdjust?.Invoke(c); - - carousel.Filter(initialCriteria?.Invoke() ?? new FilterCriteria()); carousel.BeatmapSetsChanged = () => changed = true; - carousel.BeatmapSets = beatmapSets; + carouselAdjust?.Invoke(c); }); AddUntilStep("Wait for load", () => changed); } - private void createCarousel(List beatmapSets, Action carouselAdjust = null, Container target = null) + private void createCarousel(List beatmapSets, [CanBeNull] Func initialCriteria = null, Action carouselAdjust = null, Container target = null) { AddStep("Create carousel", () => { selectedSets.Clear(); eagerSelectedIDs.Clear(); - carousel = new TestBeatmapCarousel + carousel = new TestBeatmapCarousel(initialCriteria?.Invoke() ?? new FilterCriteria()) { RelativeSizeAxes = Axes.Both, }; @@ -1389,8 +1387,8 @@ namespace osu.Game.Tests.Visual.SongSelect private partial class TestBeatmapCarousel : BeatmapCarousel { - public TestBeatmapCarousel() - : base(new FilterCriteria()) + public TestBeatmapCarousel(FilterCriteria criteria) + : base(criteria) { } diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63b2bcf7b1..20899d1869 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -130,18 +130,22 @@ namespace osu.Game.Screens.Select get => beatmapSets.Select(g => g.BeatmapSet); set { + if (LoadState != LoadState.NotLoaded) + throw new InvalidOperationException("If not using a realm source, beatmap sets must be set before load."); + loadedTestBeatmaps = true; - Schedule(() => loadBeatmapSets(value)); + detachedBeatmapSets = new BindableList(value); + Schedule(loadNewRoot); } } - private void loadBeatmapSets(IEnumerable beatmapSets) + private void loadNewRoot() { // Ensure no changes are made to the list while we are initialising items. // We'll catch up on changes via subscriptions anyway. - beatmapSets = beatmapSets.ToArray(); + BeatmapSetInfo[] loadableSets = detachedBeatmapSets.ToArray(); - if (selectedBeatmapSet != null && !beatmapSets.Contains(selectedBeatmapSet.BeatmapSet)) + if (selectedBeatmapSet != null && !loadableSets.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; var selectedBeatmapBefore = selectedBeatmap?.BeatmapInfo; @@ -150,7 +154,7 @@ namespace osu.Game.Screens.Select if (beatmapsSplitOut) { - var carouselBeatmapSets = beatmapSets.SelectMany(s => s.Beatmaps).Select(b => + var carouselBeatmapSets = loadableSets.SelectMany(s => s.Beatmaps).Select(b => { return createCarouselSet(new BeatmapSetInfo(new[] { b }) { @@ -164,7 +168,7 @@ namespace osu.Game.Screens.Select } else { - var carouselBeatmapSets = beatmapSets.Select(createCarouselSet).OfType(); + var carouselBeatmapSets = loadableSets.Select(createCarouselSet).OfType(); newRoot.AddItems(carouselBeatmapSets); } @@ -259,7 +263,7 @@ namespace osu.Game.Screens.Select // thread. If we attempt to detach beatmaps in this callback the game will fall over (it takes time). detachedBeatmapSets = detachedBeatmapStore.GetDetachedBeatmaps(cancellationToken); detachedBeatmapSets.BindCollectionChanged(beatmapSetsChanged); - loadBeatmapSets(detachedBeatmapSets); + loadNewRoot(); } } @@ -309,8 +313,7 @@ namespace osu.Game.Screens.Select case NotifyCollectionChangedAction.Reset: setsRequiringRemoval.Clear(); setsRequiringUpdate.Clear(); - - loadBeatmapSets(detachedBeatmapSets); + loadNewRoot(); break; } @@ -733,7 +736,7 @@ namespace osu.Game.Screens.Select if (activeCriteria.SplitOutDifficulties != beatmapsSplitOut) { beatmapsSplitOut = activeCriteria.SplitOutDifficulties; - loadBeatmapSets(detachedBeatmapSets); + loadNewRoot(); return; } From 1776d38809fbea7994614c34c489a7d740832089 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 20:06:44 +0900 Subject: [PATCH 24/46] Remove `loadedTestBeatmaps` flag --- osu.Game/Screens/Select/BeatmapCarousel.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 20899d1869..7f6921d768 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -116,15 +116,12 @@ namespace osu.Game.Screens.Select [Resolved] private DetachedBeatmapStore? detachedBeatmapStore { get; set; } - private IBindableList detachedBeatmapSets = null!; + private IBindableList? detachedBeatmapSets; private readonly NoResultsPlaceholder noResultsPlaceholder; private IEnumerable beatmapSets => root.Items.OfType(); - // todo: only used for testing, maybe remove. - private bool loadedTestBeatmaps; - public IEnumerable BeatmapSets { get => beatmapSets.Select(g => g.BeatmapSet); @@ -133,7 +130,6 @@ namespace osu.Game.Screens.Select if (LoadState != LoadState.NotLoaded) throw new InvalidOperationException("If not using a realm source, beatmap sets must be set before load."); - loadedTestBeatmaps = true; detachedBeatmapSets = new BindableList(value); Schedule(loadNewRoot); } @@ -143,7 +139,7 @@ namespace osu.Game.Screens.Select { // Ensure no changes are made to the list while we are initialising items. // We'll catch up on changes via subscriptions anyway. - BeatmapSetInfo[] loadableSets = detachedBeatmapSets.ToArray(); + BeatmapSetInfo[] loadableSets = detachedBeatmapSets!.ToArray(); if (selectedBeatmapSet != null && !loadableSets.Contains(selectedBeatmapSet.BeatmapSet)) selectedBeatmapSet = null; @@ -256,7 +252,7 @@ namespace osu.Game.Screens.Select RightClickScrollingEnabled.ValueChanged += enabled => Scroll.RightMouseScrollbar = enabled.NewValue; RightClickScrollingEnabled.TriggerChange(); - if (!loadedTestBeatmaps && detachedBeatmapStore != null) + if (detachedBeatmapStore != null && detachedBeatmapSets == null) { // This is performing an unnecessary second lookup on realm (in addition to the subscription), but for performance reasons // we require it to be separate: the subscription's initial callback (with `ChangeSet` of `null`) will run on the update @@ -279,10 +275,6 @@ namespace osu.Game.Screens.Select private void beatmapSetsChanged(object? beatmaps, NotifyCollectionChangedEventArgs changed) { - // If loading test beatmaps, avoid overwriting with realm subscription callbacks. - if (loadedTestBeatmaps) - return; - IEnumerable? newBeatmapSets = changed.NewItems?.Cast(); switch (changed.Action) From def1abaeca06161faee6422d8efbe1c68b03c4f3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 28 Aug 2024 23:29:32 +0900 Subject: [PATCH 25/46] Fix some tests not always waiting long enough for beatmap loading These used to work because there was a huge blocking load operation, which is now more asynchronous. Note that the change made in `SongSelect` is not required, but defensive (feels it should have been doing this the whole time). --- .../Visual/Editing/TestSceneOpenEditorTimestamp.cs | 4 ++-- .../Navigation/TestSceneBeatmapEditorNavigation.cs | 14 ++++++++------ .../Visual/Navigation/TestScenePresentBeatmap.cs | 4 ++-- .../Visual/Navigation/TestSceneScreenNavigation.cs | 6 ++++-- osu.Game/Screens/Select/SongSelect.cs | 3 ++- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneOpenEditorTimestamp.cs b/osu.Game.Tests/Visual/Editing/TestSceneOpenEditorTimestamp.cs index 1f46a08831..971eb223eb 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneOpenEditorTimestamp.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneOpenEditorTimestamp.cs @@ -36,7 +36,7 @@ namespace osu.Game.Tests.Visual.Editing () => Is.EqualTo(1)); AddStep("enter song select", () => Game.ChildrenOfType().Single().OnSolo?.Invoke()); - AddUntilStep("entered song select", () => Game.ScreenStack.CurrentScreen is PlaySongSelect); + AddUntilStep("entered song select", () => Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect && songSelect.BeatmapSetsLoaded); addStepClickLink("00:00:000 (1)", waitForSeek: false); AddUntilStep("received 'must be in edit'", @@ -138,7 +138,7 @@ namespace osu.Game.Tests.Visual.Editing AddUntilStep("Wait for song select", () => Game.Beatmap.Value.BeatmapSetInfo.Equals(beatmapSet) && Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect - && songSelect.IsLoaded + && songSelect.BeatmapSetsLoaded ); AddStep("Switch ruleset", () => Game.Ruleset.Value = ruleset); AddStep("Open editor for ruleset", () => diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs index 5640682d06..d76e0290ef 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneBeatmapEditorNavigation.cs @@ -165,16 +165,19 @@ namespace osu.Game.Tests.Visual.Navigation } [Test] + [Solo] public void TestEditorGameplayTestAlwaysUsesOriginalRuleset() { prepareBeatmap(); - AddStep("switch ruleset", () => Game.Ruleset.Value = new ManiaRuleset().RulesetInfo); + AddStep("switch ruleset at song select", () => Game.Ruleset.Value = new ManiaRuleset().RulesetInfo); 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("test gameplay", () => getEditor().TestGameplay()); + AddUntilStep("wait for editor open", () => Game.ScreenStack.CurrentScreen is Editor editor && editor.ReadyForUse); + AddAssert("editor ruleset is osu!", () => Game.Ruleset.Value, () => Is.EqualTo(new OsuRuleset().RulesetInfo)); + + AddStep("test gameplay", () => getEditor().TestGameplay()); AddUntilStep("wait for player", () => { // notifications may fire at almost any inopportune time and cause annoying test failures. @@ -183,8 +186,7 @@ namespace osu.Game.Tests.Visual.Navigation Game.CloseAllOverlays(); return Game.ScreenStack.CurrentScreen is EditorPlayer editorPlayer && editorPlayer.IsLoaded; }); - - AddAssert("current ruleset is osu!", () => Game.Ruleset.Value.Equals(new OsuRuleset().RulesetInfo)); + AddAssert("gameplay ruleset is osu!", () => Game.Ruleset.Value, () => Is.EqualTo(new OsuRuleset().RulesetInfo)); AddStep("exit to song select", () => Game.PerformFromScreen(_ => { }, typeof(PlaySongSelect).Yield())); AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is PlaySongSelect); @@ -352,7 +354,7 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for song select", () => Game.Beatmap.Value.BeatmapSetInfo.Equals(beatmapSet) && Game.ScreenStack.CurrentScreen is PlaySongSelect songSelect - && songSelect.IsLoaded); + && songSelect.BeatmapSetsLoaded); } private void openEditor() diff --git a/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs b/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs index fc711473f2..f036b4b3ef 100644 --- a/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs +++ b/osu.Game.Tests/Visual/Navigation/TestScenePresentBeatmap.cs @@ -193,7 +193,7 @@ namespace osu.Game.Tests.Visual.Navigation { AddStep("present beatmap", () => Game.PresentBeatmap(getImport())); - AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect songSelect && songSelect.IsLoaded); + AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect songSelect && songSelect.BeatmapSetsLoaded); AddUntilStep("correct beatmap displayed", () => Game.Beatmap.Value.BeatmapSetInfo.OnlineID, () => Is.EqualTo(getImport().OnlineID)); AddAssert("correct ruleset selected", () => Game.Ruleset.Value, () => Is.EqualTo(getImport().Beatmaps.First().Ruleset)); } @@ -203,7 +203,7 @@ namespace osu.Game.Tests.Visual.Navigation Predicate pred = b => b.OnlineID == importedID * 1024 + 2; AddStep("present difficulty", () => Game.PresentBeatmap(getImport(), pred)); - AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect songSelect && songSelect.IsLoaded); + AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect songSelect && songSelect.BeatmapSetsLoaded); AddUntilStep("correct beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.OnlineID, () => Is.EqualTo(importedID * 1024 + 2)); AddAssert("correct ruleset selected", () => Game.Ruleset.Value.OnlineID, () => Is.EqualTo(expectedRulesetOnlineID ?? getImport().Beatmaps.First().Ruleset.OnlineID)); } diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index db9ecd90b9..f02c2fd4f0 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -1035,9 +1035,11 @@ namespace osu.Game.Tests.Visual.Navigation [Test] public void TestTouchScreenDetectionInGame() { + BeatmapSetInfo beatmapSet = null; + PushAndConfirm(() => new TestPlaySongSelect()); - AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); - AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + AddStep("import beatmap", () => beatmapSet = BeatmapImportHelper.LoadQuickOszIntoOsu(Game).GetResultSafely()); + AddUntilStep("wait for selected", () => Game.Beatmap.Value.BeatmapSetInfo.Equals(beatmapSet)); AddStep("select", () => InputManager.Key(Key.Enter)); Player player = null; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index bfbc50378a..6da72ee660 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -428,7 +428,8 @@ namespace osu.Game.Screens.Select // Forced refetch is important here to guarantee correct invalidation across all difficulties. Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmapInfo ?? beatmapInfoNoDebounce, true); - this.Push(new EditorLoader()); + + FinaliseSelection(customStartAction: () => this.Push(new EditorLoader())); } /// From d1d2591b6737c9fa6ee5806d6c2c1038db0aba57 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 29 Aug 2024 18:29:58 +0900 Subject: [PATCH 26/46] Fix realm changes being applied before detach finishes --- osu.Game/Database/DetachedBeatmapStore.cs | 85 +++++++++++++++++------ 1 file changed, 63 insertions(+), 22 deletions(-) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 39f0bdaafe..17d2dd15b6 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -21,6 +22,8 @@ namespace osu.Game.Database private IDisposable? realmSubscription; + private readonly Queue pendingOperations = new Queue(); + [Resolved] private RealmAccess realm { get; set; } = null!; @@ -70,37 +73,61 @@ namespace osu.Game.Database } foreach (int i in changes.DeletedIndices.OrderDescending()) - removeAt(i); + { + pendingOperations.Enqueue(new OperationArgs + { + Type = OperationType.Remove, + Index = i, + }); + } foreach (int i in changes.InsertedIndices) - insert(sender[i].Detach(), i); + { + pendingOperations.Enqueue(new OperationArgs + { + Type = OperationType.Insert, + BeatmapSet = sender[i].Detach(), + Index = i, + }); + } foreach (int i in changes.NewModifiedIndices) - replaceRange(sender[i].Detach(), i); + { + pendingOperations.Enqueue(new OperationArgs + { + Type = OperationType.Update, + BeatmapSet = sender[i].Detach(), + Index = i, + }); + } } - private void replaceRange(BeatmapSetInfo set, int i) + protected override void Update() { - if (loaded.IsSet) - detachedBeatmapSets.ReplaceRange(i, 1, new[] { set }); - else - Schedule(() => { detachedBeatmapSets.ReplaceRange(i, 1, new[] { set }); }); - } + base.Update(); - private void insert(BeatmapSetInfo set, int i) - { - if (loaded.IsSet) - detachedBeatmapSets.Insert(i, set); - else - Schedule(() => { detachedBeatmapSets.Insert(i, set); }); - } + // We can't start processing operations until we have finished detaching the initial list. + if (!loaded.IsSet) + return; - private void removeAt(int i) - { - if (loaded.IsSet) - detachedBeatmapSets.RemoveAt(i); - else - Schedule(() => { detachedBeatmapSets.RemoveAt(i); }); + // If this ever leads to performance issues, we could dequeue a limited number of operations per update frame. + while (pendingOperations.TryDequeue(out var op)) + { + switch (op.Type) + { + case OperationType.Insert: + detachedBeatmapSets.Insert(op.Index, op.BeatmapSet!); + break; + + case OperationType.Update: + detachedBeatmapSets.ReplaceRange(op.Index, 1, new[] { op.BeatmapSet! }); + break; + + case OperationType.Remove: + detachedBeatmapSets.RemoveAt(op.Index); + break; + } + } } protected override void Dispose(bool isDisposing) @@ -109,5 +136,19 @@ namespace osu.Game.Database loaded.Set(); realmSubscription?.Dispose(); } + + private record OperationArgs + { + public OperationType Type; + public BeatmapSetInfo? BeatmapSet; + public int Index; + } + + private enum OperationType + { + Insert, + Update, + Remove + } } } From 7435e8aa00a35e91b5334126384eae7182ad1ed2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 30 Aug 2024 00:48:53 +0900 Subject: [PATCH 27/46] Fix catch auto generator not considering circle size --- osu.Game.Rulesets.Catch/Replays/CatchAutoGenerator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Replays/CatchAutoGenerator.cs b/osu.Game.Rulesets.Catch/Replays/CatchAutoGenerator.cs index 7c84cb24f3..7c62f9692f 100644 --- a/osu.Game.Rulesets.Catch/Replays/CatchAutoGenerator.cs +++ b/osu.Game.Rulesets.Catch/Replays/CatchAutoGenerator.cs @@ -16,9 +16,12 @@ namespace osu.Game.Rulesets.Catch.Replays { public new CatchBeatmap Beatmap => (CatchBeatmap)base.Beatmap; + private readonly float halfCatcherWidth; + public CatchAutoGenerator(IBeatmap beatmap) : base(beatmap) { + halfCatcherWidth = Catcher.CalculateCatchWidth(beatmap.Difficulty) * 0.5f; } protected override void GenerateFrames() @@ -47,10 +50,7 @@ namespace osu.Game.Rulesets.Catch.Replays bool dashRequired = speedRequired > Catcher.BASE_WALK_SPEED; bool impossibleJump = speedRequired > Catcher.BASE_DASH_SPEED; - // todo: get correct catcher size, based on difficulty CS. - const float catcher_width_half = Catcher.BASE_SIZE * 0.3f * 0.5f; - - if (lastPosition - catcher_width_half < h.EffectiveX && lastPosition + catcher_width_half > h.EffectiveX) + if (lastPosition - halfCatcherWidth < h.EffectiveX && lastPosition + halfCatcherWidth > h.EffectiveX) { // we are already in the correct range. lastTime = h.StartTime; From 7f41d5f4e7e7fa0b27192a2eb5ba85045508e8a1 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 30 Aug 2024 16:32:15 +0900 Subject: [PATCH 28/46] Remove mouse input from mania touch controls --- .../UI/ManiaTouchInputArea.cs | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/osu.Game.Rulesets.Mania/UI/ManiaTouchInputArea.cs b/osu.Game.Rulesets.Mania/UI/ManiaTouchInputArea.cs index 453b75ac84..8c4a71cf24 100644 --- a/osu.Game.Rulesets.Mania/UI/ManiaTouchInputArea.cs +++ b/osu.Game.Rulesets.Mania/UI/ManiaTouchInputArea.cs @@ -99,12 +99,6 @@ namespace osu.Game.Rulesets.Mania.UI return false; } - protected override bool OnMouseDown(MouseDownEvent e) - { - Show(); - return true; - } - protected override bool OnTouchDown(TouchDownEvent e) { Show(); @@ -172,17 +166,6 @@ namespace osu.Game.Rulesets.Mania.UI updateButton(false); } - protected override bool OnMouseDown(MouseDownEvent e) - { - updateButton(true); - return false; // handled by parent container to show overlay. - } - - protected override void OnMouseUp(MouseUpEvent e) - { - updateButton(false); - } - private void updateButton(bool press) { if (press == isPressed) From 5836f497ac13d168ab077946a67f8d349079794f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:03:30 +0900 Subject: [PATCH 29/46] Provide API context earlier to api requests in order to fix missing schedules Closes https://github.com/ppy/osu/issues/29546. --- osu.Game/Online/API/APIAccess.cs | 8 +++++-- osu.Game/Online/API/APIRequest.cs | 37 +++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index 716d1e4466..a9ad561163 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -385,7 +385,8 @@ namespace osu.Game.Online.API { try { - request.Perform(this); + request.AttachAPI(this); + request.Perform(); } catch (Exception e) { @@ -483,7 +484,8 @@ namespace osu.Game.Online.API { try { - req.Perform(this); + req.AttachAPI(this); + req.Perform(); if (req.CompletionState != APIRequestCompletionState.Completed) return false; @@ -568,6 +570,8 @@ namespace osu.Game.Online.API { lock (queue) { + request.AttachAPI(this); + if (state.Value == APIState.Offline) { request.Fail(new WebException(@"User not logged in")); diff --git a/osu.Game/Online/API/APIRequest.cs b/osu.Game/Online/API/APIRequest.cs index 6b6b222043..d062b8f3de 100644 --- a/osu.Game/Online/API/APIRequest.cs +++ b/osu.Game/Online/API/APIRequest.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.Diagnostics; using System.Globalization; using JetBrains.Annotations; using Newtonsoft.Json; @@ -74,6 +75,7 @@ namespace osu.Game.Online.API protected virtual string Uri => $@"{API.APIEndpointUrl}/api/v2/{Target}"; protected APIAccess API; + protected WebRequest WebRequest; /// @@ -101,16 +103,29 @@ namespace osu.Game.Online.API /// public APIRequestCompletionState CompletionState { get; private set; } - public void Perform(IAPIProvider api) + /// + /// Should be called before to give API context. + /// + /// + /// This allows scheduling of operations back to the correct thread (which may be required before is called). + /// + public void AttachAPI(APIAccess apiAccess) { - if (!(api is APIAccess apiAccess)) + if (API != null && API != apiAccess) + throw new InvalidOperationException("Attached API cannot be changed after initial set."); + + API = apiAccess; + } + + public void Perform() + { + if (API == null) { Fail(new NotSupportedException($"A {nameof(APIAccess)} is required to perform requests.")); return; } - API = apiAccess; - User = apiAccess.LocalUser.Value; + User = API.LocalUser.Value; if (isFailing) return; @@ -153,6 +168,8 @@ namespace osu.Game.Online.API internal void TriggerSuccess() { + Debug.Assert(API != null); + lock (completionStateLock) { if (CompletionState != APIRequestCompletionState.Waiting) @@ -161,14 +178,13 @@ namespace osu.Game.Online.API CompletionState = APIRequestCompletionState.Completed; } - if (API == null) - Success?.Invoke(); - else - API.Schedule(() => Success?.Invoke()); + API.Schedule(() => Success?.Invoke()); } internal void TriggerFailure(Exception e) { + Debug.Assert(API != null); + lock (completionStateLock) { if (CompletionState != APIRequestCompletionState.Waiting) @@ -177,10 +193,7 @@ namespace osu.Game.Online.API CompletionState = APIRequestCompletionState.Failed; } - if (API == null) - Failure?.Invoke(e); - else - API.Schedule(() => Failure?.Invoke(e)); + API.Schedule(() => Failure?.Invoke(e)); } public void Cancel() => Fail(new OperationCanceledException(@"Request cancelled")); From 07611bd8f5f30617a78649cc9eb513c89844a552 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:10:33 +0900 Subject: [PATCH 30/46] Use `IAPIProvider` interface and correctly support scheduling from `DummyAPIAccess` --- osu.Game/Online/API/APIAccess.cs | 2 +- osu.Game/Online/API/APIRequest.cs | 4 ++-- osu.Game/Online/API/DummyAPIAccess.cs | 13 ++++++++++++- osu.Game/Online/API/IAPIProvider.cs | 5 +++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index a9ad561163..a9ccbf9b18 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -159,7 +159,7 @@ namespace osu.Game.Online.API private void onTokenChanged(ValueChangedEvent e) => config.SetValue(OsuSetting.Token, config.Get(OsuSetting.SavePassword) ? authentication.TokenString : string.Empty); - internal new void Schedule(Action action) => base.Schedule(action); + void IAPIProvider.Schedule(Action action) => base.Schedule(action); public string AccessToken => authentication.RequestAccessToken(); diff --git a/osu.Game/Online/API/APIRequest.cs b/osu.Game/Online/API/APIRequest.cs index d062b8f3de..37ad5fff0e 100644 --- a/osu.Game/Online/API/APIRequest.cs +++ b/osu.Game/Online/API/APIRequest.cs @@ -74,7 +74,7 @@ namespace osu.Game.Online.API protected virtual string Uri => $@"{API.APIEndpointUrl}/api/v2/{Target}"; - protected APIAccess API; + protected IAPIProvider API; protected WebRequest WebRequest; @@ -109,7 +109,7 @@ namespace osu.Game.Online.API /// /// This allows scheduling of operations back to the correct thread (which may be required before is called). /// - public void AttachAPI(APIAccess apiAccess) + public void AttachAPI(IAPIProvider apiAccess) { if (API != null && API != apiAccess) throw new InvalidOperationException("Attached API cannot be changed after initial set."); diff --git a/osu.Game/Online/API/DummyAPIAccess.cs b/osu.Game/Online/API/DummyAPIAccess.cs index 0af76537cd..7ac5c45fad 100644 --- a/osu.Game/Online/API/DummyAPIAccess.cs +++ b/osu.Game/Online/API/DummyAPIAccess.cs @@ -82,6 +82,8 @@ namespace osu.Game.Online.API public virtual void Queue(APIRequest request) { + request.AttachAPI(this); + Schedule(() => { if (HandleRequest?.Invoke(request) != true) @@ -98,10 +100,17 @@ namespace osu.Game.Online.API }); } - public void Perform(APIRequest request) => HandleRequest?.Invoke(request); + void IAPIProvider.Schedule(Action action) => base.Schedule(action); + + public void Perform(APIRequest request) + { + request.AttachAPI(this); + HandleRequest?.Invoke(request); + } public Task PerformAsync(APIRequest request) { + request.AttachAPI(this); HandleRequest?.Invoke(request); return Task.CompletedTask; } @@ -155,6 +164,8 @@ namespace osu.Game.Online.API state.Value = APIState.Connecting; LastLoginError = null; + request.AttachAPI(this); + // if no handler installed / handler can't handle verification, just assume that the server would verify for simplicity. if (HandleRequest?.Invoke(request) != true) onSuccessfulLogin(); diff --git a/osu.Game/Online/API/IAPIProvider.cs b/osu.Game/Online/API/IAPIProvider.cs index d8194dc32b..eccfb36546 100644 --- a/osu.Game/Online/API/IAPIProvider.cs +++ b/osu.Game/Online/API/IAPIProvider.cs @@ -134,6 +134,11 @@ namespace osu.Game.Online.API /// void UpdateStatistics(UserStatistics newStatistics); + /// + /// Schedule a callback to run on the update thread. + /// + internal void Schedule(Action action); + /// /// Constructs a new . May be null if not supported. /// From dd7133657dbe57c3aa99ef8266b52ca6bebf62a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:14:10 +0900 Subject: [PATCH 31/46] Fix weird test critical failure if exception happens too early in execution Noticed in passing. ``` Exit code is 134 (Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object. at osu.Game.OsuGameBase.onExceptionThrown(Exception ex) in /Users/dean/Projects/osu/osu.Game/OsuGameBase.cs:line 695 at osu.Framework.Platform.GameHost.abortExecutionFromException(Object sender, Exception exception, Boolean isTerminating) at osu.Framework.Platform.GameHost.unobservedExceptionHandler(Object sender, UnobservedTaskExceptionEventArgs args) at System.Threading.Tasks.TaskExceptionHolder.Finalize()) ``` --- osu.Game/OsuGameBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 1988a06503..ce0c288934 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -692,7 +692,7 @@ namespace osu.Game if (Interlocked.Decrement(ref allowableExceptions) < 0) { Logger.Log("Too many unhandled exceptions, crashing out."); - RulesetStore.TryDisableCustomRulesetsCausing(ex); + RulesetStore?.TryDisableCustomRulesetsCausing(ex); return false; } From 2d745fb67e9210ae4eb7d0b5a702729ca4ac8ce3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:21:30 +0900 Subject: [PATCH 32/46] Apply NRT to `APIRequest` --- osu.Game/Online/API/APIRequest.cs | 26 ++++++++----------- .../Online/API/Requests/JoinChannelRequest.cs | 2 +- .../API/Requests/LeaveChannelRequest.cs | 2 +- osu.Game/Online/Chat/WebSocketChatClient.cs | 2 +- osu.Game/Online/Rooms/JoinRoomRequest.cs | 2 +- osu.Game/Online/Rooms/PartRoomRequest.cs | 2 +- osu.Game/Tests/PollingChatClient.cs | 2 +- 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/osu.Game/Online/API/APIRequest.cs b/osu.Game/Online/API/APIRequest.cs index 37ad5fff0e..45ebbcd76d 100644 --- a/osu.Game/Online/API/APIRequest.cs +++ b/osu.Game/Online/API/APIRequest.cs @@ -1,12 +1,9 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System; using System.Diagnostics; using System.Globalization; -using JetBrains.Annotations; using Newtonsoft.Json; using osu.Framework.Extensions.TypeExtensions; using osu.Framework.IO.Network; @@ -27,18 +24,17 @@ namespace osu.Game.Online.API /// /// The deserialised response object. May be null if the request or deserialisation failed. /// - [CanBeNull] - public T Response { get; private set; } + public T? Response { get; private set; } /// /// Invoked on successful completion of an API request. /// This will be scheduled to the API's internal scheduler (run on update thread automatically). /// - public new event APISuccessHandler Success; + public new event APISuccessHandler? Success; protected APIRequest() { - base.Success += () => Success?.Invoke(Response); + base.Success += () => Success?.Invoke(Response!); } protected override void PostProcess() @@ -72,28 +68,28 @@ namespace osu.Game.Online.API protected virtual WebRequest CreateWebRequest() => new OsuWebRequest(Uri); - protected virtual string Uri => $@"{API.APIEndpointUrl}/api/v2/{Target}"; + protected virtual string Uri => $@"{API!.APIEndpointUrl}/api/v2/{Target}"; - protected IAPIProvider API; + protected IAPIProvider? API; - protected WebRequest WebRequest; + protected WebRequest? WebRequest; /// /// The currently logged in user. Note that this will only be populated during . /// - protected APIUser User { get; private set; } + protected APIUser? User { get; private set; } /// /// Invoked on successful completion of an API request. /// This will be scheduled to the API's internal scheduler (run on update thread automatically). /// - public event APISuccessHandler Success; + public event APISuccessHandler? Success; /// /// Invoked on failure to complete an API request. /// This will be scheduled to the API's internal scheduler (run on update thread automatically). /// - public event APIFailureHandler Failure; + public event APIFailureHandler? Failure; private readonly object completionStateLock = new object(); @@ -210,7 +206,7 @@ namespace osu.Game.Online.API // in the case of a cancellation we don't care about whether there's an error in the response. if (!(e is OperationCanceledException)) { - string responseString = WebRequest?.GetResponseString(); + string? responseString = WebRequest?.GetResponseString(); // naive check whether there's an error in the response to avoid unnecessary JSON deserialisation. if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error""")) @@ -248,7 +244,7 @@ namespace osu.Game.Online.API private class DisplayableError { [JsonProperty("error")] - public string ErrorMessage { get; set; } + public string ErrorMessage { get; set; } = string.Empty; } } diff --git a/osu.Game/Online/API/Requests/JoinChannelRequest.cs b/osu.Game/Online/API/Requests/JoinChannelRequest.cs index 33eab7e355..0109e653d9 100644 --- a/osu.Game/Online/API/Requests/JoinChannelRequest.cs +++ b/osu.Game/Online/API/Requests/JoinChannelRequest.cs @@ -23,6 +23,6 @@ namespace osu.Game.Online.API.Requests return req; } - protected override string Target => $@"chat/channels/{channel.Id}/users/{User.Id}"; + protected override string Target => $@"chat/channels/{channel.Id}/users/{User!.Id}"; } } diff --git a/osu.Game/Online/API/Requests/LeaveChannelRequest.cs b/osu.Game/Online/API/Requests/LeaveChannelRequest.cs index 7dfc9a0aed..36cfd79c60 100644 --- a/osu.Game/Online/API/Requests/LeaveChannelRequest.cs +++ b/osu.Game/Online/API/Requests/LeaveChannelRequest.cs @@ -23,6 +23,6 @@ namespace osu.Game.Online.API.Requests return req; } - protected override string Target => $@"chat/channels/{channel.Id}/users/{User.Id}"; + protected override string Target => $@"chat/channels/{channel.Id}/users/{User!.Id}"; } } diff --git a/osu.Game/Online/Chat/WebSocketChatClient.cs b/osu.Game/Online/Chat/WebSocketChatClient.cs index 37774a1f5d..a74f0222f2 100644 --- a/osu.Game/Online/Chat/WebSocketChatClient.cs +++ b/osu.Game/Online/Chat/WebSocketChatClient.cs @@ -80,7 +80,7 @@ namespace osu.Game.Online.Chat fetchReq.Success += updates => { - if (updates?.Presence != null) + if (updates.Presence != null) { foreach (var channel in updates.Presence) joinChannel(channel); diff --git a/osu.Game/Online/Rooms/JoinRoomRequest.cs b/osu.Game/Online/Rooms/JoinRoomRequest.cs index 8645f2a2c0..9a73104b60 100644 --- a/osu.Game/Online/Rooms/JoinRoomRequest.cs +++ b/osu.Game/Online/Rooms/JoinRoomRequest.cs @@ -27,6 +27,6 @@ namespace osu.Game.Online.Rooms return req; } - protected override string Target => $@"rooms/{Room.RoomID.Value}/users/{User.Id}"; + protected override string Target => $@"rooms/{Room.RoomID.Value}/users/{User!.Id}"; } } diff --git a/osu.Game/Online/Rooms/PartRoomRequest.cs b/osu.Game/Online/Rooms/PartRoomRequest.cs index 09ba6f65c3..2416833a1e 100644 --- a/osu.Game/Online/Rooms/PartRoomRequest.cs +++ b/osu.Game/Online/Rooms/PartRoomRequest.cs @@ -23,6 +23,6 @@ namespace osu.Game.Online.Rooms return req; } - protected override string Target => $"rooms/{room.RoomID.Value}/users/{User.Id}"; + protected override string Target => $"rooms/{room.RoomID.Value}/users/{User!.Id}"; } } diff --git a/osu.Game/Tests/PollingChatClient.cs b/osu.Game/Tests/PollingChatClient.cs index eb29b35c1d..75975c716b 100644 --- a/osu.Game/Tests/PollingChatClient.cs +++ b/osu.Game/Tests/PollingChatClient.cs @@ -48,7 +48,7 @@ namespace osu.Game.Tests fetchReq.Success += updates => { - if (updates?.Presence != null) + if (updates.Presence != null) { foreach (var channel in updates.Presence) handleChannelJoined(channel); From 291dd5b1016081e534b78e0d894a688bd9dec74a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:37:27 +0900 Subject: [PATCH 33/46] Remove TODO --- osu.Game/Screens/Select/BeatmapCarousel.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 7f6921d768..32f85824fa 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -320,7 +320,6 @@ namespace osu.Game.Screens.Select { try { - // TODO: chekc whether we still need beatmap sets by ID foreach (var set in setsRequiringRemoval) removeBeatmapSet(set.ID); foreach (var set in setsRequiringUpdate) updateBeatmapSet(set); From 1b9942cb3092b7f5a3f256e611d1e33c4455a76d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:44:04 +0900 Subject: [PATCH 34/46] Mark `BeatmapSets` as `internal` --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 32f85824fa..ed3fbc4054 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -122,7 +122,7 @@ namespace osu.Game.Screens.Select private IEnumerable beatmapSets => root.Items.OfType(); - public IEnumerable BeatmapSets + internal IEnumerable BeatmapSets { get => beatmapSets.Select(g => g.BeatmapSet); set From 2033a5e1579ff8cd3264b9f65c148ea700005929 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:44:51 +0900 Subject: [PATCH 35/46] Add disposal of `ManualResetEventSlim` --- osu.Game/Database/DetachedBeatmapStore.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 17d2dd15b6..7920f24a0b 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -133,7 +133,9 @@ namespace osu.Game.Database protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); + loaded.Set(); + loaded.Dispose(); realmSubscription?.Dispose(); } From de208fd5c385fe128968eeab4f182dfaf4c3abd3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:45:45 +0900 Subject: [PATCH 36/46] Add very basic error handling for failed beatmap detach --- osu.Game/Database/DetachedBeatmapStore.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/DetachedBeatmapStore.cs b/osu.Game/Database/DetachedBeatmapStore.cs index 7920f24a0b..64aeeccd9a 100644 --- a/osu.Game/Database/DetachedBeatmapStore.cs +++ b/osu.Game/Database/DetachedBeatmapStore.cs @@ -10,6 +10,7 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Game.Beatmaps; +using osu.Game.Online.Multiplayer; using Realms; namespace osu.Game.Database @@ -59,15 +60,21 @@ namespace osu.Game.Database Task.Factory.StartNew(() => { - realm.Run(_ => + try { - var detached = frozenSets.Detach(); + realm.Run(_ => + { + var detached = frozenSets.Detach(); - detachedBeatmapSets.Clear(); - detachedBeatmapSets.AddRange(detached); + detachedBeatmapSets.Clear(); + detachedBeatmapSets.AddRange(detached); + }); + } + finally + { loaded.Set(); - }); - }, TaskCreationOptions.LongRunning); + } + }, TaskCreationOptions.LongRunning).FireAndForget(); return; } From 7b6e62283ffa0a56a99581863b74735957ebacca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 30 Aug 2024 18:50:00 +0900 Subject: [PATCH 37/46] Fix beatmap not being detached on hide/unhide The explicit detach call was removed from `updateBeatmapSet`, causing this to occur. We could optionally add it back (it will be a noop in all cases though). --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index ed3fbc4054..87cea45e87 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -391,7 +391,7 @@ namespace osu.Game.Screens.Select if (root.BeatmapSetsByID.TryGetValue(beatmapSet.ID, out var existingSets) && existingSets.SelectMany(s => s.Beatmaps).All(b => b.BeatmapInfo.ID != beatmapInfo.ID)) { - updateBeatmapSet(beatmapSet); + updateBeatmapSet(beatmapSet.Detach()); changed = true; } } From 8ffd4aa82c5e1afd4b4fe56773d6043248b54a12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 30 Aug 2024 13:41:34 +0200 Subject: [PATCH 38/46] Fix NRT inspections --- .../TestSceneOnlinePlayBeatmapAvailabilityTracker.cs | 2 +- osu.Game/Online/API/APIDownloadRequest.cs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 585fd516bd..ae3451c3e0 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -257,7 +257,7 @@ namespace osu.Game.Tests.Online { } - protected override string Target => null; + protected override string Target => string.Empty; } } } diff --git a/osu.Game/Online/API/APIDownloadRequest.cs b/osu.Game/Online/API/APIDownloadRequest.cs index c48372278a..f8db52139d 100644 --- a/osu.Game/Online/API/APIDownloadRequest.cs +++ b/osu.Game/Online/API/APIDownloadRequest.cs @@ -4,6 +4,7 @@ #nullable disable using System; +using System.Diagnostics; using System.IO; using osu.Framework.IO.Network; @@ -34,7 +35,11 @@ namespace osu.Game.Online.API return request; } - private void request_Progress(long current, long total) => API.Schedule(() => Progressed?.Invoke(current, total)); + private void request_Progress(long current, long total) + { + Debug.Assert(API != null); + API.Schedule(() => Progressed?.Invoke(current, total)); + } protected void TriggerSuccess(string filename) { From 8b04455c29fd41dfab49974f8883acb7ef60d8fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 30 Aug 2024 14:57:15 +0200 Subject: [PATCH 39/46] Fix chat overlay tests Not entirely sure why they were failing previously, but the most likely explanation is that by freak accident some mock requests would previously execute immediately rather than be scheduled on the API thread, which would change execution ordering and ensure that `ChannelManager.CurrentChannel` would become the joined channel, rather than remaining at the channel listing. --- osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs index a47205094e..b6445dec6b 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatOverlay.cs @@ -446,7 +446,7 @@ namespace osu.Game.Tests.Visual.Online { AddStep("Show overlay with channel 1", () => { - channelManager.JoinChannel(testChannel1); + channelManager.CurrentChannel.Value = channelManager.JoinChannel(testChannel1); chatOverlay.Show(); }); waitForChannel1Visible(); @@ -462,7 +462,7 @@ namespace osu.Game.Tests.Visual.Online { AddStep("Show overlay with channel 1", () => { - channelManager.JoinChannel(testChannel1); + channelManager.CurrentChannel.Value = channelManager.JoinChannel(testChannel1); chatOverlay.Show(); }); waitForChannel1Visible(); From f5a2b5ea03caa71e4122926ccea6d0b53e1b781f Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Wed, 28 Aug 2024 02:20:11 +0300 Subject: [PATCH 40/46] Use FastCircle in demanding places in the editor --- .../Blueprints/Sliders/Components/PathControlPointPiece.cs | 4 ++-- .../Timelines/Summary/Parts/EffectPointVisualisation.cs | 2 +- .../Timelines/Summary/Visualisations/PointVisualisation.cs | 2 +- osu.Game/Screens/Loader.cs | 1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs index 9d819f6cc0..3337e99215 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointPiece.cs @@ -40,7 +40,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public readonly PathControlPoint ControlPoint; private readonly T hitObject; - private readonly Circle circle; + private readonly FastCircle circle; private readonly Drawable markerRing; [Resolved] @@ -62,7 +62,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components InternalChildren = new[] { - circle = new Circle + circle = new FastCircle { Anchor = Anchor.Centre, Origin = Anchor.Centre, diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs index 17fedb933a..1d71bc100c 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs @@ -105,7 +105,7 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts } } - private partial class KiaiVisualisation : Circle, IHasTooltip + private partial class KiaiVisualisation : FastCircle, IHasTooltip { private readonly double startTime; private readonly double endTime; diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Visualisations/PointVisualisation.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Visualisations/PointVisualisation.cs index 9c16f457f7..6c9af53964 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Visualisations/PointVisualisation.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Visualisations/PointVisualisation.cs @@ -9,7 +9,7 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Visualisations /// /// Represents a singular point on a timeline part. /// - public partial class PointVisualisation : Circle + public partial class PointVisualisation : FastCircle { public readonly double StartTime; diff --git a/osu.Game/Screens/Loader.cs b/osu.Game/Screens/Loader.cs index 4dba512cbd..57e3998646 100644 --- a/osu.Game/Screens/Loader.cs +++ b/osu.Game/Screens/Loader.cs @@ -122,6 +122,7 @@ namespace osu.Game.Screens loadTargets.Add(manager.Load(@"CursorTrail", FragmentShaderDescriptor.TEXTURE)); loadTargets.Add(manager.Load(VertexShaderDescriptor.TEXTURE_2, "TriangleBorder")); + loadTargets.Add(manager.Load(VertexShaderDescriptor.TEXTURE_2, "FastCircle")); loadTargets.Add(manager.Load(VertexShaderDescriptor.TEXTURE_3, FragmentShaderDescriptor.TEXTURE)); } From 225418dbb36db38ac9d97c7b7bd960380018be4f Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Sat, 31 Aug 2024 01:59:40 +0300 Subject: [PATCH 41/46] Rework kiai handling in summary timeline --- .../Summary/Parts/EffectPointVisualisation.cs | 93 +------------ .../Timelines/Summary/Parts/KiaiPart.cs | 123 ++++++++++++++++++ .../Timelines/Summary/SummaryTimeline.cs | 6 + 3 files changed, 130 insertions(+), 92 deletions(-) create mode 100644 osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs index b4e6d1ece2..25d50a97be 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs @@ -2,29 +2,19 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Cursor; -using osu.Framework.Graphics.Shapes; -using osu.Framework.Localisation; using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Extensions; -using osu.Game.Graphics; namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts { public partial class EffectPointVisualisation : CompositeDrawable, IControlPointVisualisation { private readonly EffectControlPoint effect; - private Bindable kiai = null!; [Resolved] private EditorBeatmap beatmap { get; set; } = null!; - [Resolved] - private OsuColour colours { get; set; } = null!; - public EffectPointVisualisation(EffectControlPoint point) { RelativePositionAxes = Axes.Both; @@ -36,49 +26,6 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts [BackgroundDependencyLoader] private void load() { - kiai = effect.KiaiModeBindable.GetBoundCopy(); - kiai.BindValueChanged(_ => refreshDisplay(), true); - } - - private EffectControlPoint? nextControlPoint; - - protected override void LoadComplete() - { - base.LoadComplete(); - - // Due to the limitations of ControlPointInfo, it's impossible to know via event flow when the next kiai point has changed. - // This is due to the fact that an EffectPoint can be added to an existing group. We would need to bind to ItemAdded on *every* - // future group to track this. - // - // I foresee this being a potential performance issue on beatmaps with many control points, so let's limit how often we check - // for changes. ControlPointInfo needs a refactor to make this flow better, but it should do for now. - Scheduler.AddDelayed(() => - { - EffectControlPoint? next = null; - - for (int i = 0; i < beatmap.ControlPointInfo.EffectPoints.Count; i++) - { - var point = beatmap.ControlPointInfo.EffectPoints[i]; - - if (point.Time > effect.Time) - { - next = point; - break; - } - } - - if (!ReferenceEquals(nextControlPoint, next)) - { - nextControlPoint = next; - refreshDisplay(); - } - }, 100, true); - } - - private void refreshDisplay() - { - ClearInternal(); - if (beatmap.BeatmapInfo.Ruleset.CreateInstance().EditorShowScrollSpeed) { AddInternal(new ControlPointVisualisation(effect) @@ -87,46 +34,8 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts X = 0, }); } - - if (!kiai.Value) - return; - - // handle kiai duration - // eventually this will be simpler when we have control points with durations. - if (nextControlPoint != null) - { - RelativeSizeAxes = Axes.Both; - Origin = Anchor.TopLeft; - - Width = (float)(nextControlPoint.Time - effect.Time); - - AddInternal(new KiaiVisualisation(effect.Time, nextControlPoint.Time) - { - RelativeSizeAxes = Axes.Both, - Anchor = Anchor.BottomLeft, - Origin = Anchor.CentreLeft, - Height = 0.4f, - Depth = float.MaxValue, - Colour = colours.Purple1, - }); - } } - private partial class KiaiVisualisation : Circle, IHasTooltip - { - private readonly double startTime; - private readonly double endTime; - - public KiaiVisualisation(double startTime, double endTime) - { - this.startTime = startTime; - this.endTime = endTime; - } - - public LocalisableString TooltipText => $"{startTime.ToEditorFormattedString()} - {endTime.ToEditorFormattedString()} kiai time"; - } - - // kiai sections display duration, so are required to be visualised. - public bool IsVisuallyRedundant(ControlPoint other) => other is EffectControlPoint otherEffect && effect.KiaiMode == otherEffect.KiaiMode; + public bool IsVisuallyRedundant(ControlPoint other) => other is EffectControlPoint; } } diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs new file mode 100644 index 0000000000..d61d4580fe --- /dev/null +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs @@ -0,0 +1,123 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Cursor; +using osu.Framework.Graphics.Pooling; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Localisation; +using osu.Game.Extensions; +using osu.Game.Graphics; + +namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts +{ + /// + /// The part of the timeline that displays kiai sections in the song. + /// + public partial class KiaiPart : TimelinePart + { + private DrawablePool pool = null!; + + [BackgroundDependencyLoader] + private void load() + { + AddInternal(pool = new DrawablePool(10)); + } + + protected override void LoadBeatmap(EditorBeatmap beatmap) + { + base.LoadBeatmap(beatmap); + EditorBeatmap.ControlPointInfo.ControlPointsChanged += updateParts; + } + + protected override void LoadComplete() + { + base.LoadComplete(); + updateParts(); + } + + private void updateParts() => Scheduler.AddOnce(() => + { + Clear(disposeChildren: false); + + double? startTime = null; + + foreach (var effectPoint in EditorBeatmap.ControlPointInfo.EffectPoints) + { + if (startTime.HasValue) + { + if (effectPoint.KiaiMode) + continue; + + var section = new KiaiSection + { + StartTime = startTime.Value, + EndTime = effectPoint.Time + }; + + Add(pool.Get(v => v.Section = section)); + + startTime = null; + } + else + { + if (!effectPoint.KiaiMode) + continue; + + startTime = effectPoint.Time; + } + } + + // last effect point has kiai enabled, kiai should last until the end of the map + if (startTime.HasValue) + { + Add(pool.Get(v => v.Section = new KiaiSection + { + StartTime = startTime.Value, + EndTime = Content.RelativeChildSize.X + })); + } + }); + + private partial class KiaiVisualisation : PoolableDrawable, IHasTooltip + { + private KiaiSection section; + + public KiaiSection Section + { + set + { + section = value; + + X = (float)value.StartTime; + Width = (float)value.Duration; + } + } + + [BackgroundDependencyLoader] + private void load(OsuColour colours) + { + RelativePositionAxes = Axes.X; + RelativeSizeAxes = Axes.Both; + Anchor = Anchor.CentreLeft; + Origin = Anchor.CentreLeft; + Height = 0.2f; + AddInternal(new Circle + { + RelativeSizeAxes = Axes.Both, + Colour = colours.Purple1 + }); + } + + public LocalisableString TooltipText => $"{section.StartTime.ToEditorFormattedString()} - {section.EndTime.ToEditorFormattedString()} kiai time"; + } + + private readonly struct KiaiSection + { + public double StartTime { get; init; } + public double EndTime { get; init; } + public double Duration => EndTime - StartTime; + } + } +} diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/SummaryTimeline.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/SummaryTimeline.cs index 4ab7c88178..c01481e840 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/SummaryTimeline.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/SummaryTimeline.cs @@ -65,6 +65,12 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary Origin = Anchor.Centre, RelativeSizeAxes = Axes.Both, }, + new KiaiPart + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + RelativeSizeAxes = Axes.Both, + }, new ControlPointPart { Anchor = Anchor.Centre, From 6b8b49e4f181cdf0feed5952d5a8f50f583ebd71 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sat, 31 Aug 2024 13:14:56 +0900 Subject: [PATCH 42/46] Simplify scroll speed point display code now that it only serves one purpose --- .../Summary/Parts/EffectPointVisualisation.cs | 41 ------------------- .../Summary/Parts/GroupVisualisation.cs | 21 ++++++++-- 2 files changed, 18 insertions(+), 44 deletions(-) delete mode 100644 osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs deleted file mode 100644 index 25d50a97be..0000000000 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/EffectPointVisualisation.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Allocation; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Game.Beatmaps.ControlPoints; - -namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts -{ - public partial class EffectPointVisualisation : CompositeDrawable, IControlPointVisualisation - { - private readonly EffectControlPoint effect; - - [Resolved] - private EditorBeatmap beatmap { get; set; } = null!; - - public EffectPointVisualisation(EffectControlPoint point) - { - RelativePositionAxes = Axes.Both; - RelativeSizeAxes = Axes.Y; - - effect = point; - } - - [BackgroundDependencyLoader] - private void load() - { - if (beatmap.BeatmapInfo.Ruleset.CreateInstance().EditorShowScrollSpeed) - { - AddInternal(new ControlPointVisualisation(effect) - { - // importantly, override the x position being set since we do that in the GroupVisualisation parent drawable. - X = 0, - }); - } - } - - public bool IsVisuallyRedundant(ControlPoint other) => other is EffectControlPoint; - } -} diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/GroupVisualisation.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/GroupVisualisation.cs index b872c3725c..0dd945805b 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/GroupVisualisation.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/GroupVisualisation.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -15,6 +16,8 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts private readonly IBindableList controlPoints = new BindableList(); + private bool showScrollSpeed; + public GroupVisualisation(ControlPointGroup group) { RelativePositionAxes = Axes.X; @@ -24,8 +27,13 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts Group = group; X = (float)group.Time; + } + + [BackgroundDependencyLoader] + private void load(EditorBeatmap beatmap) + { + showScrollSpeed = beatmap.BeatmapInfo.Ruleset.CreateInstance().EditorShowScrollSpeed; - // Run in constructor so IsRedundant calls can work correctly. controlPoints.BindTo(Group.ControlPoints); controlPoints.BindCollectionChanged((_, _) => { @@ -47,8 +55,15 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts }); break; - case EffectControlPoint effect: - AddInternal(new EffectPointVisualisation(effect)); + case EffectControlPoint: + if (!showScrollSpeed) + return; + + AddInternal(new ControlPointVisualisation(point) + { + // importantly, override the x position being set since we do that above. + X = 0, + }); break; } } From 837fa1b8dc397c370bd43bc640610405f6fcffc9 Mon Sep 17 00:00:00 2001 From: Andrei Zavatski Date: Sat, 31 Aug 2024 17:32:24 +0300 Subject: [PATCH 43/46] Use FastCircle for kiai visualisation --- .../Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs index d61d4580fe..ee44df8598 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/KiaiPart.cs @@ -103,7 +103,7 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts Anchor = Anchor.CentreLeft; Origin = Anchor.CentreLeft; Height = 0.2f; - AddInternal(new Circle + AddInternal(new FastCircle { RelativeSizeAxes = Axes.Both, Colour = colours.Purple1 From f7da7193ff683c5fb7b9ced964fff3090e2b00af Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 1 Sep 2024 19:10:08 +0900 Subject: [PATCH 44/46] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index b5a355a77f..2609fd42c3 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -10,7 +10,7 @@ true - + diff --git a/osu.iOS.props b/osu.iOS.props index a94b9375c9..1056f4b441 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -17,6 +17,6 @@ -all - + From 30fb3c3999c9b5a230865377a8721bef6e54545b Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Mon, 2 Sep 2024 15:23:40 +0900 Subject: [PATCH 45/46] Fix osu!catch fruits not resizing on texture change --- .../Legacy/LegacyCatchHitObjectPiece.cs | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyCatchHitObjectPiece.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyCatchHitObjectPiece.cs index 2184ecc363..15b168b8c2 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyCatchHitObjectPiece.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/LegacyCatchHitObjectPiece.cs @@ -85,9 +85,25 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy protected void SetTexture(Texture? texture, Texture? overlayTexture) { - colouredSprite.Texture = texture; - overlaySprite.Texture = overlayTexture; - hyperSprite.Texture = texture; + // Sizes are reset due to an arguable osu!framework bug where Sprite retains the size of the first set texture. + + if (colouredSprite.Texture != texture) + { + colouredSprite.Size = Vector2.Zero; + colouredSprite.Texture = texture; + } + + if (overlaySprite.Texture != overlayTexture) + { + overlaySprite.Size = Vector2.Zero; + overlaySprite.Texture = overlayTexture; + } + + if (hyperSprite.Texture != texture) + { + hyperSprite.Size = Vector2.Zero; + hyperSprite.Texture = texture; + } } } } From 171ac0f5104efea07c262cffa1622d6d758ed289 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Sep 2024 17:26:14 +0900 Subject: [PATCH 46/46] Fix incorrect osu!catch snap display when last object is a juice stream Addresses https://github.com/ppy/osu/discussions/29678. --- osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs b/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs index c3103bd204..6f5b32a41d 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs @@ -18,7 +18,9 @@ namespace osu.Game.Rulesets.Catch.Edit // The implementation below is probably correct but should be checked if/when exposed via controls. float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime()); - float actualDistance = Math.Abs(((CatchHitObject)before).EffectiveX - ((CatchHitObject)after).EffectiveX); + + float previousEndX = (before as JuiceStream)?.EndX ?? ((CatchHitObject)before).EffectiveX; + float actualDistance = Math.Abs(previousEndX - ((CatchHitObject)after).EffectiveX); return actualDistance / expectedDistance; }