From d74b1e148dd0afae80603afb2b16e68e1c3640a3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 4 Jul 2023 14:50:34 +0900 Subject: [PATCH 01/10] Make `ScoreInfo.BeatmapInfo` nullable --- osu.Desktop/DiscordRichPresence.cs | 2 +- .../Difficulty/TaikoPerformanceCalculator.cs | 2 +- osu.Game.Tests/Models/DisplayStringTest.cs | 8 ++++---- .../Ranking/TestSceneContractedPanelMiddleContent.cs | 2 +- osu.Game/Database/RealmAccess.cs | 2 +- osu.Game/Online/Spectator/SpectatorClient.cs | 2 +- osu.Game/Scoring/IScoreInfo.cs | 2 +- osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs | 2 +- osu.Game/Scoring/ScoreImporter.cs | 4 +++- osu.Game/Scoring/ScoreInfo.cs | 6 ++---- osu.Game/Scoring/ScoreInfoExtensions.cs | 2 +- osu.Game/Scoring/ScorePerformanceCache.cs | 2 +- osu.Game/Screens/Ranking/SoloResultsScreen.cs | 2 +- osu.Game/Screens/Select/LocalScoreDeleteDialog.cs | 7 +------ osu.Game/Users/UserActivity.cs | 2 +- 15 files changed, 21 insertions(+), 26 deletions(-) diff --git a/osu.Desktop/DiscordRichPresence.cs b/osu.Desktop/DiscordRichPresence.cs index fe3e08537e..b1e11d7a60 100644 --- a/osu.Desktop/DiscordRichPresence.cs +++ b/osu.Desktop/DiscordRichPresence.cs @@ -187,7 +187,7 @@ namespace osu.Desktop return edit.BeatmapInfo.ToString() ?? string.Empty; case UserActivity.WatchingReplay watching: - return watching.BeatmapInfo.ToString(); + return watching.BeatmapInfo?.ToString() ?? string.Empty; case UserActivity.InLobby lobby: return privacyMode.Value == DiscordRichPresenceMode.Limited ? string.Empty : lobby.Room.Name.Value; diff --git a/osu.Game.Rulesets.Taiko/Difficulty/TaikoPerformanceCalculator.cs b/osu.Game.Rulesets.Taiko/Difficulty/TaikoPerformanceCalculator.cs index a193bacde5..ac4462c18b 100644 --- a/osu.Game.Rulesets.Taiko/Difficulty/TaikoPerformanceCalculator.cs +++ b/osu.Game.Rulesets.Taiko/Difficulty/TaikoPerformanceCalculator.cs @@ -42,7 +42,7 @@ namespace osu.Game.Rulesets.Taiko.Difficulty effectiveMissCount = Math.Max(1.0, 1000.0 / totalSuccessfulHits) * countMiss; // TODO: The detection of rulesets is temporary until the leftover old skills have been reworked. - bool isConvert = score.BeatmapInfo.Ruleset.OnlineID != 1; + bool isConvert = score.BeatmapInfo!.Ruleset.OnlineID != 1; double multiplier = 1.13; diff --git a/osu.Game.Tests/Models/DisplayStringTest.cs b/osu.Game.Tests/Models/DisplayStringTest.cs index d585a0eb9f..b5303e1dd6 100644 --- a/osu.Game.Tests/Models/DisplayStringTest.cs +++ b/osu.Game.Tests/Models/DisplayStringTest.cs @@ -87,10 +87,10 @@ namespace osu.Game.Tests.Models var mock = new Mock(); mock.Setup(m => m.User).Returns(new APIUser { Username = "user" }); // TODO: temporary. - mock.Setup(m => m.Beatmap.Metadata.Artist).Returns("artist"); - mock.Setup(m => m.Beatmap.Metadata.Title).Returns("title"); - mock.Setup(m => m.Beatmap.Metadata.Author.Username).Returns("author"); - mock.Setup(m => m.Beatmap.DifficultyName).Returns("difficulty"); + mock.Setup(m => m.Beatmap!.Metadata.Artist).Returns("artist"); + mock.Setup(m => m.Beatmap!.Metadata.Title).Returns("title"); + mock.Setup(m => m.Beatmap!.Metadata.Author.Username).Returns("author"); + mock.Setup(m => m.Beatmap!.DifficultyName).Returns("difficulty"); Assert.That(mock.Object.GetDisplayString(), Is.EqualTo("user playing artist - title (author) [difficulty]")); } diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneContractedPanelMiddleContent.cs b/osu.Game.Tests/Visual/Ranking/TestSceneContractedPanelMiddleContent.cs index 3004cb8a0c..0f17b08b7b 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneContractedPanelMiddleContent.cs +++ b/osu.Game.Tests/Visual/Ranking/TestSceneContractedPanelMiddleContent.cs @@ -33,7 +33,7 @@ namespace osu.Game.Tests.Visual.Ranking AddStep("show excess mods score", () => { var score = TestResources.CreateTestScoreInfo(); - score.Mods = score.BeatmapInfo.Ruleset.CreateInstance().CreateAllMods().ToArray(); + score.Mods = score.BeatmapInfo!.Ruleset.CreateInstance().CreateAllMods().ToArray(); showPanel(CreateWorkingBeatmap(CreateBeatmap(new OsuRuleset().RulesetInfo)), score); }); } diff --git a/osu.Game/Database/RealmAccess.cs b/osu.Game/Database/RealmAccess.cs index da4caa42ba..f8b3f24a72 100644 --- a/osu.Game/Database/RealmAccess.cs +++ b/osu.Game/Database/RealmAccess.cs @@ -896,7 +896,7 @@ namespace osu.Game.Database var scores = migration.NewRealm.All(); foreach (var score in scores) - score.BeatmapHash = score.BeatmapInfo.Hash; + score.BeatmapHash = score.BeatmapInfo?.Hash ?? string.Empty; break; } diff --git a/osu.Game/Online/Spectator/SpectatorClient.cs b/osu.Game/Online/Spectator/SpectatorClient.cs index 89da8b9d32..14e137caf1 100644 --- a/osu.Game/Online/Spectator/SpectatorClient.cs +++ b/osu.Game/Online/Spectator/SpectatorClient.cs @@ -185,7 +185,7 @@ namespace osu.Game.Online.Spectator IsPlaying = true; // transfer state at point of beginning play - currentState.BeatmapID = score.ScoreInfo.BeatmapInfo.OnlineID; + currentState.BeatmapID = score.ScoreInfo.BeatmapInfo!.OnlineID; currentState.RulesetID = score.ScoreInfo.RulesetID; currentState.Mods = score.ScoreInfo.Mods.Select(m => new APIMod(m)).ToArray(); currentState.State = SpectatedUserState.Playing; diff --git a/osu.Game/Scoring/IScoreInfo.cs b/osu.Game/Scoring/IScoreInfo.cs index 3644d099d9..d17558f800 100644 --- a/osu.Game/Scoring/IScoreInfo.cs +++ b/osu.Game/Scoring/IScoreInfo.cs @@ -28,7 +28,7 @@ namespace osu.Game.Scoring double? PP { get; } - IBeatmapInfo Beatmap { get; } + IBeatmapInfo? Beatmap { get; } IRulesetInfo Ruleset { get; } diff --git a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs index f71da6c7e0..6cad8716b1 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreEncoder.cs @@ -65,7 +65,7 @@ namespace osu.Game.Scoring.Legacy { sw.Write((byte)(score.ScoreInfo.Ruleset.OnlineID)); sw.Write(LATEST_VERSION); - sw.Write(score.ScoreInfo.BeatmapInfo.MD5Hash); + sw.Write(score.ScoreInfo.BeatmapInfo!.MD5Hash); sw.Write(score.ScoreInfo.User.Username); sw.Write(FormattableString.Invariant($"lazer-{score.ScoreInfo.User.Username}-{score.ScoreInfo.Date}").ComputeMD5Hash()); sw.Write((ushort)(score.ScoreInfo.GetCount300() ?? 0)); diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index 16658a598a..5770da10be 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -64,6 +64,8 @@ namespace osu.Game.Scoring protected override void Populate(ScoreInfo model, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default) { + Debug.Assert(model.BeatmapInfo != null); + // Ensure the beatmap is not detached. if (!model.BeatmapInfo.IsManaged) model.BeatmapInfo = realm.Find(model.BeatmapInfo.ID); @@ -99,7 +101,7 @@ namespace osu.Game.Scoring if (score.MaximumStatistics.Select(kvp => kvp.Value).Sum() > 0) return; - var beatmap = score.BeatmapInfo.Detach(); + var beatmap = score.BeatmapInfo?.Detach(); var ruleset = score.Ruleset.Detach(); var rulesetInstance = ruleset.CreateInstance(); diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 6dfac419e5..6816e86e9d 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -36,7 +36,7 @@ namespace osu.Game.Scoring /// /// When setting this, make sure to also set to allow relational consistency when a beatmap is potentially changed. /// - public BeatmapInfo BeatmapInfo { get; set; } = null!; + public BeatmapInfo? BeatmapInfo { get; set; } /// /// The at the point in time when the score was set. @@ -129,14 +129,12 @@ namespace osu.Game.Scoring public int RankInt { get; set; } IRulesetInfo IScoreInfo.Ruleset => Ruleset; - IBeatmapInfo IScoreInfo.Beatmap => BeatmapInfo; + IBeatmapInfo? IScoreInfo.Beatmap => BeatmapInfo; IUser IScoreInfo.User => User; IEnumerable IHasNamedFiles.Files => Files; #region Properties required to make things work with existing usages - public Guid BeatmapInfoID => BeatmapInfo.ID; - public int UserID => RealmUser.OnlineID; public int RulesetID => Ruleset.OnlineID; diff --git a/osu.Game/Scoring/ScoreInfoExtensions.cs b/osu.Game/Scoring/ScoreInfoExtensions.cs index 85598076d6..6e57a9fd0b 100644 --- a/osu.Game/Scoring/ScoreInfoExtensions.cs +++ b/osu.Game/Scoring/ScoreInfoExtensions.cs @@ -13,7 +13,7 @@ namespace osu.Game.Scoring /// /// A user-presentable display title representing this score. /// - public static string GetDisplayTitle(this IScoreInfo scoreInfo) => $"{scoreInfo.User.Username} playing {scoreInfo.Beatmap.GetDisplayTitle()}"; + public static string GetDisplayTitle(this IScoreInfo scoreInfo) => $"{scoreInfo.User.Username} playing {scoreInfo.Beatmap?.GetDisplayTitle() ?? "unknown"}"; /// /// Orders an array of s by total score. diff --git a/osu.Game/Scoring/ScorePerformanceCache.cs b/osu.Game/Scoring/ScorePerformanceCache.cs index bdbcfe4efe..1f2b1aeb95 100644 --- a/osu.Game/Scoring/ScorePerformanceCache.cs +++ b/osu.Game/Scoring/ScorePerformanceCache.cs @@ -34,7 +34,7 @@ namespace osu.Game.Scoring { var score = lookup.ScoreInfo; - var attributes = await difficultyCache.GetDifficultyAsync(score.BeatmapInfo, score.Ruleset, score.Mods, token).ConfigureAwait(false); + var attributes = await difficultyCache.GetDifficultyAsync(score.BeatmapInfo!, score.Ruleset, score.Mods, token).ConfigureAwait(false); // Performance calculation requires the beatmap and ruleset to be locally available. If not, return a default value. if (attributes?.Attributes == null) diff --git a/osu.Game/Screens/Ranking/SoloResultsScreen.cs b/osu.Game/Screens/Ranking/SoloResultsScreen.cs index c8920a734d..f187b8a302 100644 --- a/osu.Game/Screens/Ranking/SoloResultsScreen.cs +++ b/osu.Game/Screens/Ranking/SoloResultsScreen.cs @@ -63,7 +63,7 @@ namespace osu.Game.Screens.Ranking protected override APIRequest? FetchScores(Action>? scoresCallback) { - if (Score.BeatmapInfo.OnlineID <= 0 || Score.BeatmapInfo.Status <= BeatmapOnlineStatus.Pending) + if (Score.BeatmapInfo!.OnlineID <= 0 || Score.BeatmapInfo.Status <= BeatmapOnlineStatus.Pending) return null; getScoreRequest = new GetScoresRequest(Score.BeatmapInfo, Score.Ruleset); diff --git a/osu.Game/Screens/Select/LocalScoreDeleteDialog.cs b/osu.Game/Screens/Select/LocalScoreDeleteDialog.cs index c4add31a4f..cd98872b65 100644 --- a/osu.Game/Screens/Select/LocalScoreDeleteDialog.cs +++ b/osu.Game/Screens/Select/LocalScoreDeleteDialog.cs @@ -4,9 +4,7 @@ using osu.Framework.Allocation; using osu.Game.Overlays.Dialog; using osu.Game.Scoring; -using System.Diagnostics; using osu.Framework.Graphics.Sprites; -using osu.Game.Beatmaps; namespace osu.Game.Screens.Select { @@ -20,11 +18,8 @@ namespace osu.Game.Screens.Select } [BackgroundDependencyLoader] - private void load(BeatmapManager beatmapManager, ScoreManager scoreManager) + private void load(ScoreManager scoreManager) { - BeatmapInfo? beatmapInfo = beatmapManager.QueryBeatmap(b => b.ID == score.BeatmapInfoID); - Debug.Assert(beatmapInfo != null); - BodyText = $"{score.User} ({score.DisplayAccuracy}, {score.Rank})"; Icon = FontAwesome.Regular.TrashAlt; diff --git a/osu.Game/Users/UserActivity.cs b/osu.Game/Users/UserActivity.cs index 1761282e2e..c82f642fdc 100644 --- a/osu.Game/Users/UserActivity.cs +++ b/osu.Game/Users/UserActivity.cs @@ -111,7 +111,7 @@ namespace osu.Game.Users protected string Username => score.User.Username; - public BeatmapInfo BeatmapInfo => score.BeatmapInfo; + public BeatmapInfo? BeatmapInfo => score.BeatmapInfo; public WatchingReplay(ScoreInfo score) { From f30dc59afec1c670e4c791026224a4c50862e6e1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 4 Jul 2023 14:50:50 +0900 Subject: [PATCH 02/10] Update tests to show expected score retention behaviour when saving a beatmap --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 10 +++++----- osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs | 5 ++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 84e6a6c00f..84d13ac85e 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -431,19 +431,18 @@ namespace osu.Game.Tests.Database await createScoreForBeatmap(realm.Realm, imported.Beatmaps.First()); + Assert.That(imported.Beatmaps.First().Scores.Any()); + // imitate making local changes via editor // ReSharper disable once MethodHasAsyncOverload - realm.Write(_ => + realm.Write(r => { BeatmapInfo beatmap = imported.Beatmaps.First(); beatmap.Hash = "new_hash"; beatmap.ResetOnlineInfo(); }); - // for now, making changes to a beatmap doesn't remove the backlink from the score to the beatmap. - // the logic of ensuring that scores match the beatmap is upheld via comparing the hash in usages (see: https://github.com/ppy/osu/pull/22539). - // TODO: revisit when fixing https://github.com/ppy/osu/issues/24069. - Assert.That(imported.Beatmaps.First().Scores.Any()); + Assert.That(!imported.Beatmaps.First().Scores.Any()); var importedSecondTime = await importer.Import(new ImportTask(temp)); @@ -461,6 +460,7 @@ namespace osu.Game.Tests.Database Assert.That(importedFirstTimeBeatmap.Hash != importedSecondTimeBeatmap.Hash); Assert.That(!importedFirstTimeBeatmap.Scores.Any()); Assert.That(importedSecondTimeBeatmap.Scores.Count() == 1); + Assert.That(importedSecondTimeBeatmap.Scores.Single().BeatmapInfo, Is.EqualTo(importedSecondTimeBeatmap)); }); } diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 83cb54df3f..3934a67be0 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -387,10 +387,9 @@ namespace osu.Game.Tests.Database realm.Run(r => r.Refresh()); - // for now, making changes to a beatmap doesn't remove the backlink from the score to the beatmap. - // the logic of ensuring that scores match the beatmap is upheld via comparing the hash in usages (https://github.com/ppy/osu/pull/22539). - // TODO: revisit when fixing https://github.com/ppy/osu/issues/24069. + // making changes to a beatmap doesn't remove the score from realm, but should disassociate the beatmap. checkCount(realm, 1); + Assert.That(realm.Run(r => r.All().First().BeatmapInfo), Is.Null); // reimport the original beatmap before local modifications var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOnlineCopy), importBeforeUpdate.Value); From 64fc5e40e8377b4a38e3ef28f9e8544021485566 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 4 Jul 2023 14:51:09 +0900 Subject: [PATCH 03/10] Move score attach logic to a helper method and call during editor save --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 1 + .../Database/BeatmapImporterUpdateTests.cs | 1 + osu.Game/Beatmaps/BeatmapImporter.cs | 4 +--- osu.Game/Beatmaps/BeatmapInfo.cs | 16 ++++++++++++++++ osu.Game/Beatmaps/BeatmapManager.cs | 3 +++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 84d13ac85e..1440f540b4 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -440,6 +440,7 @@ namespace osu.Game.Tests.Database BeatmapInfo beatmap = imported.Beatmaps.First(); beatmap.Hash = "new_hash"; beatmap.ResetOnlineInfo(); + beatmap.UpdateLocalScores(r); }); Assert.That(!imported.Beatmaps.First().Scores.Any()); diff --git a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs index 3934a67be0..e1bf8f5eae 100644 --- a/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterUpdateTests.cs @@ -383,6 +383,7 @@ namespace osu.Game.Tests.Database beatmapInfo.Hash = new_beatmap_hash; beatmapInfo.ResetOnlineInfo(); + beatmapInfo.UpdateLocalScores(s.Realm); }); realm.Run(r => r.Refresh()); diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index fd766490fc..d20027892f 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -20,7 +20,6 @@ using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.Overlays.Notifications; using osu.Game.Rulesets; -using osu.Game.Scoring; using Realms; namespace osu.Game.Beatmaps @@ -210,8 +209,7 @@ namespace osu.Game.Beatmaps // Let's reattach any matching scores that exist in the database, based on hash. foreach (BeatmapInfo beatmap in model.Beatmaps) { - foreach (var score in realm.All().Where(score => score.BeatmapHash == beatmap.Hash)) - score.BeatmapInfo = beatmap; + beatmap.UpdateLocalScores(realm); } ProcessBeatmap?.Invoke(model, parameters.Batch ? MetadataLookupScope.LocalCacheFirst : MetadataLookupScope.OnlineFirst); diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 5019d64276..c1aeec1f71 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -234,6 +234,22 @@ namespace osu.Game.Beatmaps } } + /// + /// Local scores are retained separate from a beatmap's lifetime, matched via . + /// Therefore we need to detach / reattach scores when a beatmap is edited or imported. + /// + /// A realm instance in an active write transaction. + public void UpdateLocalScores(Realm realm) + { + // first disassociate any scores which are already attached and no longer valid. + foreach (var score in Scores) + score.BeatmapInfo = null; + + // then attach any scores which match the new hash. + foreach (var score in realm.All().Where(s => s.BeatmapHash == Hash)) + score.BeatmapInfo = this; + } + IBeatmapMetadataInfo IBeatmapInfo.Metadata => Metadata; IBeatmapSetInfo? IBeatmapInfo.BeatmapSet => BeatmapSet; IRulesetInfo IBeatmapInfo.Ruleset => Ruleset; diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 73811b2e62..295f6ed91a 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -467,6 +467,9 @@ namespace osu.Game.Beatmaps if (transferCollections) beatmapInfo.TransferCollectionReferences(r, oldMd5Hash); + liveBeatmapSet.Beatmaps.Single(b => b.ID == beatmapInfo.ID) + .UpdateLocalScores(r); + // do not look up metadata. // this is a locally-modified set now, so looking up metadata is busy work at best and harmful at worst. ProcessBeatmap?.Invoke(liveBeatmapSet, MetadataLookupScope.None); From a0bed0fceccb370700b98c559572c7f4f5dbbd08 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 4 Jul 2023 15:18:51 +0900 Subject: [PATCH 04/10] Add full flow test of `UpdateLocalScores` --- .../Database/BeatmapImporterTests.cs | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 1440f540b4..2766e4509e 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -417,6 +417,60 @@ namespace osu.Game.Tests.Database }); } + [Test] + public void TestImport_Modify_Revert() + { + RunTestWithRealmAsync(async (realm, storage) => + { + var importer = new BeatmapImporter(storage, realm); + using var store = new RealmRulesetStore(realm, storage); + + var imported = await LoadOszIntoStore(importer, realm.Realm); + + await createScoreForBeatmap(realm.Realm, imported.Beatmaps.First()); + + var score = realm.Run(r => r.All().Single()); + + string originalHash = imported.Beatmaps.First().Hash; + const string modified_hash = "new_hash"; + + Assert.That(imported.Beatmaps.First().Scores.Single(), Is.EqualTo(score)); + + Assert.That(score.BeatmapHash, Is.EqualTo(originalHash)); + Assert.That(score.BeatmapInfo, Is.EqualTo(imported.Beatmaps.First())); + + // imitate making local changes via editor + // ReSharper disable once MethodHasAsyncOverload + realm.Write(r => + { + BeatmapInfo beatmap = imported.Beatmaps.First(); + beatmap.Hash = modified_hash; + beatmap.ResetOnlineInfo(); + beatmap.UpdateLocalScores(r); + }); + + Assert.That(!imported.Beatmaps.First().Scores.Any()); + + Assert.That(score.BeatmapInfo, Is.Null); + Assert.That(score.BeatmapHash, Is.EqualTo(originalHash)); + + // imitate making local changes via editor + // ReSharper disable once MethodHasAsyncOverload + realm.Write(r => + { + BeatmapInfo beatmap = imported.Beatmaps.First(); + beatmap.Hash = originalHash; + beatmap.ResetOnlineInfo(); + beatmap.UpdateLocalScores(r); + }); + + Assert.That(imported.Beatmaps.First().Scores.Single(), Is.EqualTo(score)); + + Assert.That(score.BeatmapHash, Is.EqualTo(originalHash)); + Assert.That(score.BeatmapInfo, Is.EqualTo(imported.Beatmaps.First())); + }); + } + [Test] public void TestImport_ThenModifyMapWithScore_ThenImport() { From a55809733dd1c9ec05f2a9ecfadcc97da827c17e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 4 Jul 2023 22:20:50 +0200 Subject: [PATCH 05/10] Expand `ScoreInfo.BeatmapInfo` xmldoc --- osu.Game/Scoring/ScoreInfo.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index 6816e86e9d..4798a105c5 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -34,7 +34,14 @@ namespace osu.Game.Scoring /// The this score was made against. /// /// - /// When setting this, make sure to also set to allow relational consistency when a beatmap is potentially changed. + /// + /// This property may be if the score was set on a beatmap (or a version of the beatmap) that is not available locally + /// e.g. due to online updates, or local modifications to the beatmap. + /// The property will only link to a if its matches . + /// + /// + /// Due to the above, whenever setting this, make sure to also set to allow relational consistency when a beatmap is potentially changed. + /// /// public BeatmapInfo? BeatmapInfo { get; set; } From bcdbdf57efe1218e2f378a2a4830425bf508a0b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 4 Jul 2023 22:22:57 +0200 Subject: [PATCH 06/10] Reword comment --- osu.Game.Tests/Database/BeatmapImporterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index 2766e4509e..0eac70f9c8 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -454,7 +454,7 @@ namespace osu.Game.Tests.Database Assert.That(score.BeatmapInfo, Is.Null); Assert.That(score.BeatmapHash, Is.EqualTo(originalHash)); - // imitate making local changes via editor + // imitate reverting the local changes made above // ReSharper disable once MethodHasAsyncOverload realm.Write(r => { From e2ddcb23497468c45d1285797984fb531d0cef1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 4 Jul 2023 22:39:26 +0200 Subject: [PATCH 07/10] Silence a few remaining nullability warnings --- .../Visual/Ranking/TestSceneExpandedPanelMiddleContent.cs | 2 +- osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs | 2 +- osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs | 2 +- .../Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs | 2 +- osu.Game/Screens/Play/SoloPlayer.cs | 2 +- osu.Game/Screens/Ranking/Expanded/ExpandedPanelMiddleContent.cs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneExpandedPanelMiddleContent.cs b/osu.Game.Tests/Visual/Ranking/TestSceneExpandedPanelMiddleContent.cs index c05774400f..d71c72f4ec 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneExpandedPanelMiddleContent.cs +++ b/osu.Game.Tests/Visual/Ranking/TestSceneExpandedPanelMiddleContent.cs @@ -48,7 +48,7 @@ namespace osu.Game.Tests.Visual.Ranking var author = new RealmUser { Username = "mapper_name" }; var score = TestResources.CreateTestScoreInfo(createTestBeatmap(author)); - score.Mods = score.BeatmapInfo.Ruleset.CreateInstance().CreateAllMods().ToArray(); + score.Mods = score.BeatmapInfo!.Ruleset.CreateInstance().CreateAllMods().ToArray(); showPanel(score); }); diff --git a/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs b/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs index 42068ff117..c5b61c1a90 100644 --- a/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs +++ b/osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs @@ -405,7 +405,7 @@ namespace osu.Game.Tests.Visual.Ranking public UnrankedSoloResultsScreen(ScoreInfo score) : base(score, true) { - Score.BeatmapInfo.OnlineID = 0; + Score.BeatmapInfo!.OnlineID = 0; Score.BeatmapInfo.Status = BeatmapOnlineStatus.Pending; } diff --git a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs index 425f40258e..615a3e39af 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/ScoreTable.cs @@ -163,7 +163,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores }, username, #pragma warning disable 618 - new StatisticText(score.MaxCombo, score.BeatmapInfo.MaxCombo, @"0\x"), + new StatisticText(score.MaxCombo, score.BeatmapInfo!.MaxCombo, @"0\x"), #pragma warning restore 618 }; diff --git a/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs b/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs index e030b1e34f..c92b79cb4d 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs @@ -123,7 +123,7 @@ namespace osu.Game.Overlays.BeatmapSet.Scores accuracyColumn.Text = value.DisplayAccuracy; maxComboColumn.Text = value.MaxCombo.ToLocalisableString(@"0\x"); - ppColumn.Alpha = value.BeatmapInfo.Status.GrantsPerformancePoints() ? 1 : 0; + ppColumn.Alpha = value.BeatmapInfo!.Status.GrantsPerformancePoints() ? 1 : 0; if (value.PP is double pp) ppColumn.Text = pp.ToLocalisableString(@"N0"); diff --git a/osu.Game/Screens/Play/SoloPlayer.cs b/osu.Game/Screens/Play/SoloPlayer.cs index dafdf00136..f7ae3eb62b 100644 --- a/osu.Game/Screens/Play/SoloPlayer.cs +++ b/osu.Game/Screens/Play/SoloPlayer.cs @@ -68,7 +68,7 @@ namespace osu.Game.Screens.Play { IBeatmapInfo beatmap = score.ScoreInfo.BeatmapInfo; - Debug.Assert(beatmap.OnlineID > 0); + Debug.Assert(beatmap!.OnlineID > 0); return new SubmitSoloScoreRequest(score.ScoreInfo, token, beatmap.OnlineID); } diff --git a/osu.Game/Screens/Ranking/Expanded/ExpandedPanelMiddleContent.cs b/osu.Game/Screens/Ranking/Expanded/ExpandedPanelMiddleContent.cs index 82c429798e..d1dc1a81db 100644 --- a/osu.Game/Screens/Ranking/Expanded/ExpandedPanelMiddleContent.cs +++ b/osu.Game/Screens/Ranking/Expanded/ExpandedPanelMiddleContent.cs @@ -66,7 +66,7 @@ namespace osu.Game.Screens.Ranking.Expanded [BackgroundDependencyLoader] private void load(BeatmapDifficultyCache beatmapDifficultyCache) { - var beatmap = score.BeatmapInfo; + var beatmap = score.BeatmapInfo!; var metadata = beatmap.BeatmapSet?.Metadata ?? beatmap.Metadata; string creator = metadata.Author.Username; From 5947c2b298096e936eefb70943615153c20f5a87 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 5 Jul 2023 16:07:12 +0900 Subject: [PATCH 08/10] Throw if a null `BeatmapInfo` arrives during score import process --- osu.Game/Scoring/ScoreImporter.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreImporter.cs b/osu.Game/Scoring/ScoreImporter.cs index 5770da10be..5a52f72cd5 100644 --- a/osu.Game/Scoring/ScoreImporter.cs +++ b/osu.Game/Scoring/ScoreImporter.cs @@ -98,10 +98,12 @@ namespace osu.Game.Scoring /// The score to populate the statistics of. public void PopulateMaximumStatistics(ScoreInfo score) { + Debug.Assert(score.BeatmapInfo != null); + if (score.MaximumStatistics.Select(kvp => kvp.Value).Sum() > 0) return; - var beatmap = score.BeatmapInfo?.Detach(); + var beatmap = score.BeatmapInfo!.Detach(); var ruleset = score.Ruleset.Detach(); var rulesetInstance = ruleset.CreateInstance(); From b679ab88a161cca1331c5bf939927c498af44df0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 6 Jul 2023 12:29:03 +0900 Subject: [PATCH 09/10] Avoid attempting to process missing statistics on scores without linked beatmaps --- osu.Game/BackgroundBeatmapProcessor.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/osu.Game/BackgroundBeatmapProcessor.cs b/osu.Game/BackgroundBeatmapProcessor.cs index 9fe3a41b03..9c140bdda9 100644 --- a/osu.Game/BackgroundBeatmapProcessor.cs +++ b/osu.Game/BackgroundBeatmapProcessor.cs @@ -163,8 +163,12 @@ namespace osu.Game { foreach (var score in r.All()) { - if (score.Statistics.Sum(kvp => kvp.Value) > 0 && score.MaximumStatistics.Sum(kvp => kvp.Value) == 0) + if (score.BeatmapInfo != null + && score.Statistics.Sum(kvp => kvp.Value) > 0 + && score.MaximumStatistics.Sum(kvp => kvp.Value) == 0) + { scoreIds.Add(score.ID); + } } }); From f3f4bb635680b28c6f47090d2cbc2660992362f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 6 Jul 2023 20:56:24 +0200 Subject: [PATCH 10/10] Add one more safety against processing scores with missing beatmap --- osu.Game/BackgroundBeatmapProcessor.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/BackgroundBeatmapProcessor.cs b/osu.Game/BackgroundBeatmapProcessor.cs index 9c140bdda9..323e783d8d 100644 --- a/osu.Game/BackgroundBeatmapProcessor.cs +++ b/osu.Game/BackgroundBeatmapProcessor.cs @@ -208,7 +208,9 @@ namespace osu.Game { Logger.Log("Querying for scores that need total score conversion..."); - HashSet scoreIds = realmAccess.Run(r => new HashSet(r.All().Where(s => s.TotalScoreVersion == 30000002).AsEnumerable().Select(s => s.ID))); + HashSet scoreIds = realmAccess.Run(r => new HashSet(r.All() + .Where(s => s.BeatmapInfo != null && s.TotalScoreVersion == 30000002) + .AsEnumerable().Select(s => s.ID))); Logger.Log($"Found {scoreIds.Count} scores which require total score conversion.");