From aa252d562a11e44dbf660213ec38931fce43dd17 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 12:42:45 +0900 Subject: [PATCH 01/23] Rename top user request to make way for new type --- .../API/Requests/{GetUsersRequest.cs => GetTopUsersRequest.cs} | 2 +- .../Requests/{GetUsersResponse.cs => GetTopUsersResponse.cs} | 2 +- osu.Game/Online/API/Requests/GetUserRankingsRequest.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename osu.Game/Online/API/Requests/{GetUsersRequest.cs => GetTopUsersRequest.cs} (80%) rename osu.Game/Online/API/Requests/{GetUsersResponse.cs => GetTopUsersResponse.cs} (86%) diff --git a/osu.Game/Online/API/Requests/GetUsersRequest.cs b/osu.Game/Online/API/Requests/GetTopUsersRequest.cs similarity index 80% rename from osu.Game/Online/API/Requests/GetUsersRequest.cs rename to osu.Game/Online/API/Requests/GetTopUsersRequest.cs index b75ecd5bd7..dbbd2119db 100644 --- a/osu.Game/Online/API/Requests/GetUsersRequest.cs +++ b/osu.Game/Online/API/Requests/GetTopUsersRequest.cs @@ -3,7 +3,7 @@ namespace osu.Game.Online.API.Requests { - public class GetUsersRequest : APIRequest + public class GetTopUsersRequest : APIRequest { protected override string Target => @"rankings/osu/performance"; } diff --git a/osu.Game/Online/API/Requests/GetUsersResponse.cs b/osu.Game/Online/API/Requests/GetTopUsersResponse.cs similarity index 86% rename from osu.Game/Online/API/Requests/GetUsersResponse.cs rename to osu.Game/Online/API/Requests/GetTopUsersResponse.cs index b301f551e3..b37b8b3499 100644 --- a/osu.Game/Online/API/Requests/GetUsersResponse.cs +++ b/osu.Game/Online/API/Requests/GetTopUsersResponse.cs @@ -7,7 +7,7 @@ using osu.Game.Users; namespace osu.Game.Online.API.Requests { - public class GetUsersResponse : ResponseWithCursor + public class GetTopUsersResponse : ResponseWithCursor { [JsonProperty("ranking")] public List Users; diff --git a/osu.Game/Online/API/Requests/GetUserRankingsRequest.cs b/osu.Game/Online/API/Requests/GetUserRankingsRequest.cs index 143d21e40d..bccc3bc0c3 100644 --- a/osu.Game/Online/API/Requests/GetUserRankingsRequest.cs +++ b/osu.Game/Online/API/Requests/GetUserRankingsRequest.cs @@ -6,7 +6,7 @@ using osu.Game.Rulesets; namespace osu.Game.Online.API.Requests { - public class GetUserRankingsRequest : GetRankingsRequest + public class GetUserRankingsRequest : GetRankingsRequest { public readonly UserRankingsType Type; From db039da668755507f5ba34985601c2e733d6e1b2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 12:54:08 +0900 Subject: [PATCH 02/23] Add and consume multi-lookup API endpoint --- .../Online/API/Requests/GetUsersRequest.cs | 19 +++++++++++++++++++ .../Online/API/Requests/GetUsersResponse.cs | 15 +++++++++++++++ .../Dashboard/CurrentlyPlayingDisplay.cs | 13 +++++++------ osu.Game/Overlays/DashboardOverlay.cs | 3 +-- 4 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 osu.Game/Online/API/Requests/GetUsersRequest.cs create mode 100644 osu.Game/Online/API/Requests/GetUsersResponse.cs diff --git a/osu.Game/Online/API/Requests/GetUsersRequest.cs b/osu.Game/Online/API/Requests/GetUsersRequest.cs new file mode 100644 index 0000000000..9a7006f5d6 --- /dev/null +++ b/osu.Game/Online/API/Requests/GetUsersRequest.cs @@ -0,0 +1,19 @@ +// 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; + +namespace osu.Game.Online.API.Requests +{ + public class GetUsersRequest : APIRequest + { + private readonly int[] userIds; + + public GetUsersRequest(int[] userIds) + { + this.userIds = userIds; + } + + protected override string Target => $@"users/?{userIds.Select(u => $"ids[]={u}&").Aggregate((a, b) => a + b)}"; + } +} diff --git a/osu.Game/Online/API/Requests/GetUsersResponse.cs b/osu.Game/Online/API/Requests/GetUsersResponse.cs new file mode 100644 index 0000000000..6f49d5cd53 --- /dev/null +++ b/osu.Game/Online/API/Requests/GetUsersResponse.cs @@ -0,0 +1,15 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using Newtonsoft.Json; +using osu.Game.Users; + +namespace osu.Game.Online.API.Requests +{ + public class GetUsersResponse : ResponseWithCursor + { + [JsonProperty("users")] + public List Users; + } +} diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index dae27f35ae..9020b317ef 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -54,17 +54,18 @@ namespace osu.Game.Overlays.Dashboard switch (e.Action) { case NotifyCollectionChangedAction.Add: - foreach (var u in e.NewItems.OfType()) + var request = new GetUsersRequest(e.NewItems.OfType().ToArray()); + + request.Success += users => Schedule(() => { - var request = new GetUserRequest(u); - request.Success += user => Schedule(() => + foreach (var user in users.Users) { if (playingUsers.Contains(user.Id)) userFlow.Add(createUserPanel(user)); - }); - api.Queue(request); - } + } + }); + api.Queue(request); break; case NotifyCollectionChangedAction.Remove: diff --git a/osu.Game/Overlays/DashboardOverlay.cs b/osu.Game/Overlays/DashboardOverlay.cs index 787a4985d7..04defce636 100644 --- a/osu.Game/Overlays/DashboardOverlay.cs +++ b/osu.Game/Overlays/DashboardOverlay.cs @@ -131,8 +131,7 @@ namespace osu.Game.Overlays break; case DashboardOverlayTabs.CurrentlyPlaying: - //todo: enable once caching logic is better - //loadDisplay(new CurrentlyPlayingDisplay()); + loadDisplay(new CurrentlyPlayingDisplay()); break; default: From 2457083d8bffda0e1b97c0db609dac0e92331bf8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 13:08:07 +0900 Subject: [PATCH 03/23] Add padding to currently playing view --- osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 9020b317ef..e9915df801 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -37,6 +37,7 @@ namespace osu.Game.Overlays.Dashboard { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, + Padding = new MarginPadding(10), Spacing = new Vector2(10), }; } From 893979b3deab4b95cc62814c18ba2db2fa3a9d87 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 13:10:18 +0900 Subject: [PATCH 04/23] Add exception if attempting to exceed the maximum supported lookup size for one request --- osu.Game/Online/API/Requests/GetUsersRequest.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Online/API/Requests/GetUsersRequest.cs b/osu.Game/Online/API/Requests/GetUsersRequest.cs index 9a7006f5d6..0ec5173fb6 100644 --- a/osu.Game/Online/API/Requests/GetUsersRequest.cs +++ b/osu.Game/Online/API/Requests/GetUsersRequest.cs @@ -1,6 +1,7 @@ // 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.Linq; namespace osu.Game.Online.API.Requests @@ -9,8 +10,13 @@ namespace osu.Game.Online.API.Requests { private readonly int[] userIds; + private const int max_ids_per_request = 50; + public GetUsersRequest(int[] userIds) { + if (userIds.Length > max_ids_per_request) + throw new ArgumentException($"{nameof(GetUsersRequest)} calls only support up to {max_ids_per_request} IDs at once"); + this.userIds = userIds; } From c97c6bbf52046504e31a829435447ddc1f993881 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 16:38:57 +0900 Subject: [PATCH 05/23] Add and consume user cache class --- osu.Game/Database/UserLookupCache.cs | 118 ++++++++++++++++++ osu.Game/OsuGameBase.cs | 5 + .../Dashboard/CurrentlyPlayingDisplay.cs | 22 ++-- 3 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 osu.Game/Database/UserLookupCache.cs diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs new file mode 100644 index 0000000000..f5c84c9edf --- /dev/null +++ b/osu.Game/Database/UserLookupCache.cs @@ -0,0 +1,118 @@ +// 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.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using osu.Framework.Allocation; +using osu.Game.Online.API; +using osu.Game.Online.API.Requests; +using osu.Game.Users; + +namespace osu.Game.Database +{ + public class UserLookupCache : MemoryCachingComponent + { + private readonly ConcurrentBag nextTaskIDs = new ConcurrentBag(); + + [Resolved] + private IAPIProvider api { get; set; } + + private readonly object taskAssignmentLock = new object(); + + private Task> pendingRequest; + + /// + /// Whether has already grabbed its IDs. + /// + private bool pendingRequestConsumedIDs; + + public Task GetUser(int userId, CancellationToken token = default) => GetAsync(userId, token); + + protected override async Task ComputeValueAsync(int lookup, CancellationToken token = default) + { + var users = await getQueryTaskForUser(lookup); + return users.FirstOrDefault(u => u.Id == lookup); + } + + /// + /// Return the task responsible for fetching the provided user. + /// This may be part of a larger batch lookup to reduce web requests. + /// + /// The user to lookup. + /// The task responsible for the lookup. + private Task> getQueryTaskForUser(int userId) + { + lock (taskAssignmentLock) + { + nextTaskIDs.Add(userId); + + // if there's a pending request which hasn't been started yet (and is not yet full), we can wait on it. + if (pendingRequest != null && !pendingRequestConsumedIDs && nextTaskIDs.Count < 50) + return pendingRequest; + + return queueNextTask(nextLookup); + } + + List nextLookup() + { + int[] lookupItems; + + lock (taskAssignmentLock) + { + pendingRequestConsumedIDs = true; + lookupItems = nextTaskIDs.ToArray(); + nextTaskIDs.Clear(); + + if (lookupItems.Length == 0) + { + queueNextTask(null); + return new List(); + } + } + + var request = new GetUsersRequest(lookupItems); + + // rather than queueing, we maintain our own single-threaded request stream. + request.Perform(api); + + return request.Result.Users; + } + } + + /// + /// Queues new work at the end of the current work tasks. + /// Ensures the provided work is eventually run. + /// + /// The work to run. Can be null to signify the end of available work. + /// The task tracking this work. + private Task> queueNextTask(Func> work) + { + lock (taskAssignmentLock) + { + if (work == null) + { + pendingRequest = null; + pendingRequestConsumedIDs = false; + } + else if (pendingRequest == null) + { + // special case for the first request ever. + pendingRequest = Task.Run(work); + pendingRequestConsumedIDs = false; + } + else + { + // append the new request on to the last to be executed. + pendingRequest = pendingRequest.ContinueWith(_ => work()); + pendingRequestConsumedIDs = false; + } + + return pendingRequest; + } + } + } +} diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 3da692249d..193f6fe61b 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -61,6 +61,8 @@ namespace osu.Game protected BeatmapDifficultyCache DifficultyCache; + protected UserLookupCache UserCache; + protected SkinManager SkinManager; protected RulesetStore RulesetStore; @@ -229,6 +231,9 @@ namespace osu.Game dependencies.Cache(DifficultyCache = new BeatmapDifficultyCache()); AddInternal(DifficultyCache); + dependencies.Cache(UserCache = new UserLookupCache()); + AddInternal(UserCache); + var scorePerformanceManager = new ScorePerformanceCache(); dependencies.Cache(scorePerformanceManager); AddInternal(scorePerformanceManager); diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index e9915df801..a988381f29 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -8,8 +8,8 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Screens; +using osu.Game.Database; using osu.Game.Online.API; -using osu.Game.Online.API.Requests; using osu.Game.Online.Spectator; using osu.Game.Screens.Multi.Match.Components; using osu.Game.Screens.Play; @@ -45,6 +45,9 @@ namespace osu.Game.Overlays.Dashboard [Resolved] private IAPIProvider api { get; set; } + [Resolved] + private UserLookupCache users { get; set; } + protected override void LoadComplete() { base.LoadComplete(); @@ -55,18 +58,19 @@ namespace osu.Game.Overlays.Dashboard switch (e.Action) { case NotifyCollectionChangedAction.Add: - var request = new GetUsersRequest(e.NewItems.OfType().ToArray()); - request.Success += users => Schedule(() => + foreach (var id in e.NewItems.OfType().ToArray()) { - foreach (var user in users.Users) + users.GetUser(id).ContinueWith(u => { - if (playingUsers.Contains(user.Id)) - userFlow.Add(createUserPanel(user)); - } - }); + Schedule(() => + { + if (playingUsers.Contains(u.Result.Id)) + userFlow.Add(createUserPanel(u.Result)); + }); + }); + } - api.Queue(request); break; case NotifyCollectionChangedAction.Remove: From c3c288145a81d61bf90c823aa7bbab5e302d76d3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 17:55:29 +0900 Subject: [PATCH 06/23] Ignore null results for now --- osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index a988381f29..8299619a18 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -63,6 +63,9 @@ namespace osu.Game.Overlays.Dashboard { users.GetUser(id).ContinueWith(u => { + if (u.Result == null) + return; + Schedule(() => { if (playingUsers.Contains(u.Result.Id)) From 4bbd3fe8864b2e7aea89b971e09913a53a094004 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 18:37:27 +0900 Subject: [PATCH 07/23] Handle null result --- osu.Game/Database/UserLookupCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs index f5c84c9edf..b7c6c480a9 100644 --- a/osu.Game/Database/UserLookupCache.cs +++ b/osu.Game/Database/UserLookupCache.cs @@ -79,7 +79,7 @@ namespace osu.Game.Database // rather than queueing, we maintain our own single-threaded request stream. request.Perform(api); - return request.Result.Users; + return request.Result?.Users; } } From ee84a9827ee702cc1a66c535ab17a1787b71074e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 Nov 2020 18:41:05 +0900 Subject: [PATCH 08/23] Fix regressed test --- .../TestSceneCurrentlyPlayingDisplay.cs | 44 ++++++++++++------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs index d6fd33bce7..7eba64f418 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneCurrentlyPlayingDisplay.cs @@ -2,12 +2,14 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Testing; -using osu.Game.Online.API; -using osu.Game.Online.API.Requests; +using osu.Game.Database; using osu.Game.Online.Spectator; using osu.Game.Overlays.Dashboard; using osu.Game.Tests.Visual.Gameplay; @@ -22,32 +24,34 @@ namespace osu.Game.Tests.Visual.Online private CurrentlyPlayingDisplay currentlyPlaying; - private DummyAPIAccess dummyAPI => (DummyAPIAccess)API; + [Cached(typeof(UserLookupCache))] + private UserLookupCache lookupCache = new TestUserLookupCache(); + + private Container nestedContainer; [SetUpSteps] public void SetUpSteps() { - AddStep("register request handling", () => dummyAPI.HandleRequest = req => - { - switch (req) - { - case GetUserRequest cRequest: - cRequest.TriggerSuccess(new User { Username = "peppy", Id = 2 }); - break; - } - }); - AddStep("add streaming client", () => { - Remove(testSpectatorStreamingClient); + nestedContainer?.Remove(testSpectatorStreamingClient); + Remove(lookupCache); Children = new Drawable[] { - testSpectatorStreamingClient, - currentlyPlaying = new CurrentlyPlayingDisplay + lookupCache, + nestedContainer = new Container { RelativeSizeAxes = Axes.Both, - } + Children = new Drawable[] + { + testSpectatorStreamingClient, + currentlyPlaying = new CurrentlyPlayingDisplay + { + RelativeSizeAxes = Axes.Both, + } + } + }, }; }); @@ -62,5 +66,11 @@ namespace osu.Game.Tests.Visual.Online AddStep("Remove playing user", () => testSpectatorStreamingClient.PlayingUsers.Remove(2)); AddUntilStep("Panel no longer present", () => !currentlyPlaying.ChildrenOfType().Any()); } + + internal class TestUserLookupCache : UserLookupCache + { + protected override Task ComputeValueAsync(int lookup, CancellationToken token = default) + => Task.FromResult(new User { Username = "peppy", Id = 2 }); + } } } From b47a2a03d5563e71bec072ee3921221af30e35af Mon Sep 17 00:00:00 2001 From: kamp Date: Sat, 7 Nov 2020 01:53:14 +0100 Subject: [PATCH 09/23] Fix nullref when quickdeleting slider that hasn't been selected yet --- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 9b758ec898..34dc356bc3 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -75,7 +75,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders public override bool HandleQuickDeletion() { - var hoveredControlPoint = ControlPointVisualiser.Pieces.FirstOrDefault(p => p.IsHovered); + var hoveredControlPoint = ControlPointVisualiser?.Pieces?.FirstOrDefault(p => p.IsHovered); if (hoveredControlPoint == null) return false; From 42c543472de01cfda43b3f4dfaa41698ced8db52 Mon Sep 17 00:00:00 2001 From: kamp Date: Sat, 7 Nov 2020 01:56:41 +0100 Subject: [PATCH 10/23] Remove unnecessary null coalesce --- .../Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 34dc356bc3..7ae4f387ca 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -75,7 +75,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders public override bool HandleQuickDeletion() { - var hoveredControlPoint = ControlPointVisualiser?.Pieces?.FirstOrDefault(p => p.IsHovered); + var hoveredControlPoint = ControlPointVisualiser?.Pieces.FirstOrDefault(p => p.IsHovered); if (hoveredControlPoint == null) return false; From c2a5fd2832b12f7bd259d0646c0d6feded3f0b96 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 8 Nov 2020 00:17:09 +0900 Subject: [PATCH 11/23] Add test coverage --- osu.Game.Tests/Editing/LegacyEditorBeatmapPatcherTest.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game.Tests/Editing/LegacyEditorBeatmapPatcherTest.cs b/osu.Game.Tests/Editing/LegacyEditorBeatmapPatcherTest.cs index afaaafdd26..bb56131b04 100644 --- a/osu.Game.Tests/Editing/LegacyEditorBeatmapPatcherTest.cs +++ b/osu.Game.Tests/Editing/LegacyEditorBeatmapPatcherTest.cs @@ -37,6 +37,12 @@ namespace osu.Game.Tests.Editing })); } + [Test] + public void TestPatchNoObjectChanges() + { + runTest(new OsuBeatmap()); + } + [Test] public void TestAddHitObject() { From c5b6908e71cdeaadad6f52b531a2e30c9b8158b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 8 Nov 2020 00:17:23 +0900 Subject: [PATCH 12/23] Always write [HitObjects] to file I think this is expected. If not, there's an alternative solution. --- osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index 80fd6c22bb..7ddb0b4caa 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -235,11 +235,11 @@ namespace osu.Game.Beatmaps.Formats private void handleHitObjects(TextWriter writer) { + writer.WriteLine("[HitObjects]"); + if (beatmap.HitObjects.Count == 0) return; - writer.WriteLine("[HitObjects]"); - foreach (var h in beatmap.HitObjects) handleHitObject(writer, h); } From b0052210b6a925d709dff9854b20795702fcfa03 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 8 Nov 2020 00:18:25 +0900 Subject: [PATCH 13/23] Add asserts of HitObjects indices --- osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs index 72d3421755..f2e0320ce3 100644 --- a/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs +++ b/osu.Game/Screens/Edit/LegacyEditorBeatmapPatcher.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Text; using DiffPlex; @@ -35,6 +36,9 @@ namespace osu.Game.Screens.Edit int oldHitObjectsIndex = Array.IndexOf(result.PiecesOld, "[HitObjects]"); int newHitObjectsIndex = Array.IndexOf(result.PiecesNew, "[HitObjects]"); + Debug.Assert(oldHitObjectsIndex >= 0); + Debug.Assert(newHitObjectsIndex >= 0); + var toRemove = new List(); var toAdd = new List(); From e078b78dcc6e5a244bf95376859390205d1f74c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 7 Nov 2020 20:31:44 +0100 Subject: [PATCH 14/23] Ensure callbacks don't fire when restoring default beatmap --- osu.Game/Screens/Edit/Editor.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 13d1f378a6..85467d3bbb 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -499,6 +499,9 @@ namespace osu.Game.Screens.Edit // confirming exit without save means we should delete the new beatmap completely. beatmapManager.Delete(playableBeatmap.BeatmapInfo.BeatmapSet); + // eagerly clear contents before restoring default beatmap to prevent value change callbacks from firing. + ClearInternal(); + // in theory this shouldn't be required but due to EF core not sharing instance states 100% // MusicController is unaware of the changed DeletePending state. Beatmap.SetDefault(); From ddbd6df24d825750065ec55942d725320ae211d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sat, 7 Nov 2020 20:59:41 +0100 Subject: [PATCH 15/23] Unbind bindable lists for general safety --- .../Edit/Components/Timelines/Summary/Parts/ControlPointPart.cs | 1 + .../Compose/Components/Timeline/TimelineControlPointDisplay.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/ControlPointPart.cs b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/ControlPointPart.cs index ba3ac9113e..e76ab71e54 100644 --- a/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/ControlPointPart.cs +++ b/osu.Game/Screens/Edit/Components/Timelines/Summary/Parts/ControlPointPart.cs @@ -20,6 +20,7 @@ namespace osu.Game.Screens.Edit.Components.Timelines.Summary.Parts { base.LoadBeatmap(beatmap); + controlPointGroups.UnbindAll(); controlPointGroups.BindTo(beatmap.Beatmap.ControlPointInfo.Groups); controlPointGroups.BindCollectionChanged((sender, args) => { diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineControlPointDisplay.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineControlPointDisplay.cs index 0da1b43201..13191df13c 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineControlPointDisplay.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineControlPointDisplay.cs @@ -27,6 +27,7 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline { base.LoadBeatmap(beatmap); + controlPointGroups.UnbindAll(); controlPointGroups.BindTo(beatmap.Beatmap.ControlPointInfo.Groups); controlPointGroups.BindCollectionChanged((sender, args) => { From 6d4bb4316c4a28f518d6543f28557ad4885cffb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 8 Nov 2020 00:12:25 +0100 Subject: [PATCH 16/23] Fix difficulty retrieval for online-sourced beatmaps --- osu.Game/Beatmaps/BeatmapDifficultyCache.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index 9820d508dd..eb83c88318 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -202,7 +202,9 @@ namespace osu.Game.Beatmaps /// A token that may be used to cancel this update. private void updateBindable([NotNull] BindableStarDifficulty bindable, [CanBeNull] RulesetInfo rulesetInfo, [CanBeNull] IEnumerable mods, CancellationToken cancellationToken = default) { - GetAsync(new DifficultyCacheLookup(bindable.Beatmap, rulesetInfo, mods), cancellationToken) + // GetDifficultyAsync will fall back to existing data from BeatmapInfo if not locally available + // (contrary to GetAsync) + GetDifficultyAsync(bindable.Beatmap, rulesetInfo, mods, cancellationToken) .ContinueWith(t => { // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events. From 90ce1bd5f05dae59094aa5ae6921cb056a0ff42e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 10:40:16 +0900 Subject: [PATCH 17/23] Add missing async suffix --- osu.Game/Database/UserLookupCache.cs | 2 +- osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs index b7c6c480a9..01231f3f2b 100644 --- a/osu.Game/Database/UserLookupCache.cs +++ b/osu.Game/Database/UserLookupCache.cs @@ -30,7 +30,7 @@ namespace osu.Game.Database /// private bool pendingRequestConsumedIDs; - public Task GetUser(int userId, CancellationToken token = default) => GetAsync(userId, token); + public Task GetUserAsync(int userId, CancellationToken token = default) => GetAsync(userId, token); protected override async Task ComputeValueAsync(int lookup, CancellationToken token = default) { diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index 8299619a18..f6833385a4 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -61,7 +61,7 @@ namespace osu.Game.Overlays.Dashboard foreach (var id in e.NewItems.OfType().ToArray()) { - users.GetUser(id).ContinueWith(u => + users.GetUserAsync(id).ContinueWith(u => { if (u.Result == null) return; From dc69eefa51490208a965103ca59417f1038a9e7e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 11:54:28 +0900 Subject: [PATCH 18/23] Use HashSet instead of ConcurentBag --- osu.Game/Database/UserLookupCache.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs index 01231f3f2b..adf6b4e9f8 100644 --- a/osu.Game/Database/UserLookupCache.cs +++ b/osu.Game/Database/UserLookupCache.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -16,7 +15,7 @@ namespace osu.Game.Database { public class UserLookupCache : MemoryCachingComponent { - private readonly ConcurrentBag nextTaskIDs = new ConcurrentBag(); + private readonly HashSet nextTaskIDs = new HashSet(); [Resolved] private IAPIProvider api { get; set; } From 690e69bcc6738cd68013619248b4c6afee912c91 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 12:22:54 +0900 Subject: [PATCH 19/23] Reforamt for legibility --- .../Dashboard/CurrentlyPlayingDisplay.cs | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs index f6833385a4..c3ab9e86d4 100644 --- a/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs +++ b/osu.Game/Overlays/Dashboard/CurrentlyPlayingDisplay.cs @@ -53,40 +53,43 @@ namespace osu.Game.Overlays.Dashboard base.LoadComplete(); playingUsers.BindTo(spectatorStreaming.PlayingUsers); - playingUsers.BindCollectionChanged((sender, e) => Schedule(() => - { - switch (e.Action) - { - case NotifyCollectionChangedAction.Add: + playingUsers.BindCollectionChanged(onUsersChanged, true); + } - foreach (var id in e.NewItems.OfType().ToArray()) + private void onUsersChanged(object sender, NotifyCollectionChangedEventArgs e) => Schedule(() => + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + foreach (var id in e.NewItems.OfType().ToArray()) + { + users.GetUserAsync(id).ContinueWith(u => { - users.GetUserAsync(id).ContinueWith(u => + if (u.Result == null) return; + + Schedule(() => { - if (u.Result == null) + // user may no longer be playing. + if (!playingUsers.Contains(u.Result.Id)) return; - Schedule(() => - { - if (playingUsers.Contains(u.Result.Id)) - userFlow.Add(createUserPanel(u.Result)); - }); + userFlow.Add(createUserPanel(u.Result)); }); - } + }); + } - break; + break; - case NotifyCollectionChangedAction.Remove: - foreach (var u in e.OldItems.OfType()) - userFlow.FirstOrDefault(card => card.User.Id == u)?.Expire(); - break; + case NotifyCollectionChangedAction.Remove: + foreach (var u in e.OldItems.OfType()) + userFlow.FirstOrDefault(card => card.User.Id == u)?.Expire(); + break; - case NotifyCollectionChangedAction.Reset: - userFlow.Clear(); - break; - } - }), true); - } + case NotifyCollectionChangedAction.Reset: + userFlow.Clear(); + break; + } + }); private PlayingUserPanel createUserPanel(User user) => new PlayingUserPanel(user).With(panel => From cfb42037cff74a3db3dbcf80cb176003f428c7ae Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 12:23:29 +0900 Subject: [PATCH 20/23] Refactor request string logic to avoid linq usage --- osu.Game/Online/API/Requests/GetUsersRequest.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Online/API/Requests/GetUsersRequest.cs b/osu.Game/Online/API/Requests/GetUsersRequest.cs index 0ec5173fb6..969d7fdba3 100644 --- a/osu.Game/Online/API/Requests/GetUsersRequest.cs +++ b/osu.Game/Online/API/Requests/GetUsersRequest.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Linq; namespace osu.Game.Online.API.Requests { @@ -20,6 +19,6 @@ namespace osu.Game.Online.API.Requests this.userIds = userIds; } - protected override string Target => $@"users/?{userIds.Select(u => $"ids[]={u}&").Aggregate((a, b) => a + b)}"; + protected override string Target => "users/?ids[]=" + string.Join("&ids[]=", userIds); } } From 4d6f0a8ea743cfa7aa67b58cfb629b514f529dfd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 19:42:00 +0900 Subject: [PATCH 21/23] Fix API request error handling --- osu.Game/Database/UserLookupCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/UserLookupCache.cs b/osu.Game/Database/UserLookupCache.cs index adf6b4e9f8..c85ad6d651 100644 --- a/osu.Game/Database/UserLookupCache.cs +++ b/osu.Game/Database/UserLookupCache.cs @@ -76,7 +76,7 @@ namespace osu.Game.Database var request = new GetUsersRequest(lookupItems); // rather than queueing, we maintain our own single-threaded request stream. - request.Perform(api); + api.Perform(request); return request.Result?.Users; } From ba137aadc82d979f970c72f29b7dfe14756d0752 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 20:44:12 +0900 Subject: [PATCH 22/23] Update framework --- osu.Android.props | 2 +- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Android.props b/osu.Android.props index f56baf4e5f..e3285222f8 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,6 +52,6 @@ - + diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 3783ae7d5c..832722c729 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -26,7 +26,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index ed3ec9e48b..ad6dd2a0b5 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -70,7 +70,7 @@ - + @@ -88,7 +88,7 @@ - + From 2e3fdf8116a55b1de874066e27d65f0b3345d2a5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 9 Nov 2020 20:50:36 +0900 Subject: [PATCH 23/23] Update reference to TK game window class --- osu.Desktop/OsuGameDesktop.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index 836b968a67..b17611f23f 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -130,7 +130,7 @@ namespace osu.Desktop switch (host.Window) { // Legacy osuTK DesktopGameWindow - case DesktopGameWindow desktopGameWindow: + case OsuTKDesktopWindow desktopGameWindow: desktopGameWindow.CursorState |= CursorState.Hidden; desktopGameWindow.SetIconFromStream(iconStream); desktopGameWindow.Title = Name;