From 7462a9f4ab84997dfd550f8b3025f5630c72beab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 18:17:47 +0900 Subject: [PATCH 1/7] Add helper method to handle progress notifications for background jobs --- osu.Game/BackgroundDataStoreProcessor.cs | 51 +++++++++++++++++++----- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 10cc13dc29c7..794d534f10e6 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -279,20 +279,17 @@ private void convertLegacyTotalScoreToStandardised() if (scoreIds.Count == 0) return; - ProgressNotification notification = new ProgressNotification { State = ProgressNotificationState.Active }; - - notificationOverlay?.Post(notification); + var notification = showProgressNotification("Upgrading scores to new scoring algorithm", "scores have been upgraded to the new scoring algorithm"); int processedCount = 0; int failedCount = 0; foreach (var id in scoreIds) { - if (notification.State == ProgressNotificationState.Cancelled) + if (notification?.State == ProgressNotificationState.Cancelled) break; - notification.Text = $"Upgrading scores to new scoring algorithm ({processedCount} of {scoreIds.Count})"; - notification.Progress = (float)processedCount / scoreIds.Count; + updateNotificationProgress(notification, processedCount, scoreIds.Count); sleepIfRequired(); @@ -325,24 +322,58 @@ private void convertLegacyTotalScoreToStandardised() } } - if (processedCount == scoreIds.Count) + completeNotification(notification, processedCount, scoreIds.Count, failedCount); + } + + private void updateNotificationProgress(ProgressNotification? notification, int processedCount, int totalCount) + { + if (notification == null) + return; + + notification.Text = notification.Text.ToString().Split('(').First().TrimEnd() + $" ({processedCount} of {totalCount})"; + notification.Progress = (float)processedCount / totalCount; + } + + private void completeNotification(ProgressNotification? notification, int processedCount, int totalCount, int? failedCount = null) + { + if (notification == null) + return; + + if (processedCount == totalCount) { - notification.CompletionText = $"{processedCount} score(s) have been upgraded to the new scoring algorithm"; + notification.CompletionText = $"{processedCount} {notification.CompletionText}"; notification.Progress = 1; notification.State = ProgressNotificationState.Completed; } else { - notification.Text = $"{processedCount} of {scoreIds.Count} score(s) have been upgraded to the new scoring algorithm."; + notification.Text = $"{processedCount} of {totalCount} {notification.CompletionText}"; // We may have arrived here due to user cancellation or completion with failures. if (failedCount > 0) - notification.Text += $" Check logs for issues with {failedCount} failed upgrades."; + notification.Text += $" Check logs for issues with {failedCount} failed items."; notification.State = ProgressNotificationState.Cancelled; } } + private ProgressNotification? showProgressNotification(string running, string completed) + { + if (notificationOverlay == null) + return null; + + ProgressNotification notification = new ProgressNotification + { + Text = running, + CompletionText = completed, + State = ProgressNotificationState.Active + }; + + notificationOverlay?.Post(notification); + + return notification; + } + private void sleepIfRequired() { while (localUserPlayInfo?.IsPlaying.Value == true) From e7d1cf7868663126479d2997b7d0da3062817072 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 18:22:40 +0900 Subject: [PATCH 2/7] Add progress notifications for background tasks which don't already have them --- osu.Game/BackgroundDataStoreProcessor.cs | 60 +++++++++++++++++++++--- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 794d534f10e6..58b1c912c492 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -144,12 +144,24 @@ private void processBeatmapSetsWithMissingMetrics() } }); + if (beatmapSetIds.Count == 0) + return; + Logger.Log($"Found {beatmapSetIds.Count} beatmap sets which require reprocessing."); - int i = 0; + // Technically this is doing more than just star ratings, but easier for the end user to understand. + var notification = showProgressNotification("Reprocessing star rating for beatmaps", "beatmaps' star ratings have been updated"); + + int processedCount = 0; + int failedCount = 0; foreach (var id in beatmapSetIds) { + if (notification?.State == ProgressNotificationState.Cancelled) + break; + + updateNotificationProgress(notification, processedCount, beatmapSetIds.Count); + sleepIfRequired(); realmAccess.Run(r => @@ -160,16 +172,19 @@ private void processBeatmapSetsWithMissingMetrics() { try { - Logger.Log($"Background processing {set} ({++i} / {beatmapSetIds.Count})"); beatmapUpdater.Process(set); + ++processedCount; } catch (Exception e) { Logger.Log($"Background processing failed on {set}: {e}"); + ++failedCount; } } }); } + + completeNotification(notification, processedCount, beatmapSetIds.Count, failedCount); } private void processBeatmapsWithMissingObjectCounts() @@ -184,12 +199,23 @@ private void processBeatmapsWithMissingObjectCounts() beatmapIds.Add(b.ID); }); - Logger.Log($"Found {beatmapIds.Count} beatmaps which require reprocessing."); + if (beatmapIds.Count == 0) + return; + + Logger.Log($"Found {beatmapIds.Count} beatmaps which require statistics population."); + + var notification = showProgressNotification("Populating missing statistics for beatmaps", "beatmaps have been populated with missing statistics"); - int i = 0; + int processedCount = 0; + int failedCount = 0; foreach (var id in beatmapIds) { + if (notification?.State == ProgressNotificationState.Cancelled) + break; + + updateNotificationProgress(notification, processedCount, beatmapIds.Count); + sleepIfRequired(); realmAccess.Run(r => @@ -200,16 +226,19 @@ private void processBeatmapsWithMissingObjectCounts() { try { - Logger.Log($"Background processing {beatmap} ({++i} / {beatmapIds.Count})"); beatmapUpdater.ProcessObjectCounts(beatmap); + ++processedCount; } catch (Exception e) { Logger.Log($"Background processing failed on {beatmap}: {e}"); + ++failedCount; } } }); } + + completeNotification(notification, processedCount, beatmapIds.Count, failedCount); } private void processScoresWithMissingStatistics() @@ -231,10 +260,23 @@ private void processScoresWithMissingStatistics() } }); - Logger.Log($"Found {scoreIds.Count} scores which require reprocessing."); + if (scoreIds.Count == 0) + return; + + Logger.Log($"Found {scoreIds.Count} scores which require statistics population."); + + var notification = showProgressNotification("Populating missing statistics for scores", "scores have been populated with missing statistics"); + + int processedCount = 0; + int failedCount = 0; foreach (var id in scoreIds) { + if (notification?.State == ProgressNotificationState.Cancelled) + break; + + updateNotificationProgress(notification, processedCount, scoreIds.Count); + sleepIfRequired(); try @@ -251,6 +293,7 @@ private void processScoresWithMissingStatistics() }); Logger.Log($"Populated maximum statistics for score {id}"); + ++processedCount; } catch (ObjectDisposedException) { @@ -260,8 +303,11 @@ private void processScoresWithMissingStatistics() { Logger.Log(@$"Failed to populate maximum statistics for {id}: {e}"); realmAccess.Write(r => r.Find(id)!.BackgroundReprocessingFailed = true); + ++failedCount; } } + + completeNotification(notification, processedCount, scoreIds.Count, failedCount); } private void convertLegacyTotalScoreToStandardised() @@ -332,6 +378,8 @@ private void updateNotificationProgress(ProgressNotification? notification, int notification.Text = notification.Text.ToString().Split('(').First().TrimEnd() + $" ({processedCount} of {totalCount})"; notification.Progress = (float)processedCount / totalCount; + + // TODO add log output } private void completeNotification(ProgressNotification? notification, int processedCount, int totalCount, int? failedCount = null) From bfa90e9dcb73f0c8687a184e4b91b95d2d98d72f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 18:34:55 +0900 Subject: [PATCH 3/7] Also populate `ObjectCount`s when running a full beatmap process Saves running things twice on an old install --- osu.Game/BackgroundDataStoreProcessor.cs | 1 + osu.Game/Beatmaps/BeatmapUpdater.cs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 58b1c912c492..38dc170ccc17 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -67,6 +67,7 @@ protected override void LoadComplete() checkForOutdatedStarRatings(); processBeatmapSetsWithMissingMetrics(); + // Note that the previous method will also update these on a fresh run. processBeatmapsWithMissingObjectCounts(); processScoresWithMissingStatistics(); convertLegacyTotalScoreToStandardised(); diff --git a/osu.Game/Beatmaps/BeatmapUpdater.cs b/osu.Game/Beatmaps/BeatmapUpdater.cs index 27492b8bac67..e897d28916ac 100644 --- a/osu.Game/Beatmaps/BeatmapUpdater.cs +++ b/osu.Game/Beatmaps/BeatmapUpdater.cs @@ -77,6 +77,8 @@ public void Process(BeatmapSetInfo beatmapSet, MetadataLookupScope lookupScope = beatmap.StarRating = calculator.Calculate().StarRating; beatmap.Length = working.Beatmap.CalculatePlayableLength(); beatmap.BPM = 60000 / working.Beatmap.GetMostCommonBeatLength(); + beatmap.EndTimeObjectCount = working.Beatmap.HitObjects.Count(h => h is IHasDuration); + beatmap.TotalObjectCount = working.Beatmap.HitObjects.Count; } // And invalidate again afterwards as re-fetching the most up-to-date database metadata will be required. From 5ab38151239d39d4feec8b2f84d92e651c992a60 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 18:41:36 +0900 Subject: [PATCH 4/7] Add logging of progress every so often --- osu.Game/BackgroundDataStoreProcessor.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 38dc170ccc17..6a801ab5bf97 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -380,7 +380,8 @@ private void updateNotificationProgress(ProgressNotification? notification, int notification.Text = notification.Text.ToString().Split('(').First().TrimEnd() + $" ({processedCount} of {totalCount})"; notification.Progress = (float)processedCount / totalCount; - // TODO add log output + if (processedCount % 100 == 0) + Logger.Log(notification.Text.ToString()); } private void completeNotification(ProgressNotification? notification, int processedCount, int totalCount, int? failedCount = null) From e3251b40b311e504c6bb22e75e1b1ab347704714 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 19:00:44 +0900 Subject: [PATCH 5/7] Fix progress notifications queueing up infinite text changes when not visible --- osu.Game/Overlays/Notifications/ProgressNotification.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Notifications/ProgressNotification.cs b/osu.Game/Overlays/Notifications/ProgressNotification.cs index 6ea032213e71..2362cb11f6f1 100644 --- a/osu.Game/Overlays/Notifications/ProgressNotification.cs +++ b/osu.Game/Overlays/Notifications/ProgressNotification.cs @@ -54,7 +54,7 @@ public override LocalisableString Text set { text = value; - Schedule(() => textDrawable.Text = text); + Scheduler.AddOnce(t => textDrawable.Text = t, text); } } From 03ac2c30949c66900e04a67250ff21e2f34f2dfe Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 18 Dec 2023 19:20:57 +0900 Subject: [PATCH 6/7] Remove some excessive logging --- osu.Game/BackgroundDataStoreProcessor.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 6a801ab5bf97..55be7f2c9efa 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -293,7 +293,6 @@ private void processScoresWithMissingStatistics() r.Find(id)!.MaximumStatisticsJson = JsonConvert.SerializeObject(score.MaximumStatistics); }); - Logger.Log($"Populated maximum statistics for score {id}"); ++processedCount; } catch (ObjectDisposedException) @@ -354,7 +353,6 @@ private void convertLegacyTotalScoreToStandardised() s.TotalScoreVersion = LegacyScoreEncoder.LATEST_VERSION; }); - Logger.Log($"Converted total score for score {id}"); ++processedCount; } catch (ObjectDisposedException) From 9aaaa128098af6a8e57e07d5131bb719779b428c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 19 Dec 2023 00:01:09 +0900 Subject: [PATCH 7/7] Don't show progress notifications when there are too few items to be worthwhile --- osu.Game/BackgroundDataStoreProcessor.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/osu.Game/BackgroundDataStoreProcessor.cs b/osu.Game/BackgroundDataStoreProcessor.cs index 55be7f2c9efa..33b66ecfc7f5 100644 --- a/osu.Game/BackgroundDataStoreProcessor.cs +++ b/osu.Game/BackgroundDataStoreProcessor.cs @@ -151,7 +151,7 @@ private void processBeatmapSetsWithMissingMetrics() Logger.Log($"Found {beatmapSetIds.Count} beatmap sets which require reprocessing."); // Technically this is doing more than just star ratings, but easier for the end user to understand. - var notification = showProgressNotification("Reprocessing star rating for beatmaps", "beatmaps' star ratings have been updated"); + var notification = showProgressNotification(beatmapSetIds.Count, "Reprocessing star rating for beatmaps", "beatmaps' star ratings have been updated"); int processedCount = 0; int failedCount = 0; @@ -205,7 +205,7 @@ private void processBeatmapsWithMissingObjectCounts() Logger.Log($"Found {beatmapIds.Count} beatmaps which require statistics population."); - var notification = showProgressNotification("Populating missing statistics for beatmaps", "beatmaps have been populated with missing statistics"); + var notification = showProgressNotification(beatmapIds.Count, "Populating missing statistics for beatmaps", "beatmaps have been populated with missing statistics"); int processedCount = 0; int failedCount = 0; @@ -266,7 +266,7 @@ private void processScoresWithMissingStatistics() Logger.Log($"Found {scoreIds.Count} scores which require statistics population."); - var notification = showProgressNotification("Populating missing statistics for scores", "scores have been populated with missing statistics"); + var notification = showProgressNotification(scoreIds.Count, "Populating missing statistics for scores", "scores have been populated with missing statistics"); int processedCount = 0; int failedCount = 0; @@ -325,7 +325,7 @@ private void convertLegacyTotalScoreToStandardised() if (scoreIds.Count == 0) return; - var notification = showProgressNotification("Upgrading scores to new scoring algorithm", "scores have been upgraded to the new scoring algorithm"); + var notification = showProgressNotification(scoreIds.Count, "Upgrading scores to new scoring algorithm", "scores have been upgraded to the new scoring algorithm"); int processedCount = 0; int failedCount = 0; @@ -405,11 +405,14 @@ private void completeNotification(ProgressNotification? notification, int proces } } - private ProgressNotification? showProgressNotification(string running, string completed) + private ProgressNotification? showProgressNotification(int totalCount, string running, string completed) { if (notificationOverlay == null) return null; + if (totalCount < 10) + return null; + ProgressNotification notification = new ProgressNotification { Text = running,