1
0
mirror of https://github.com/ppy/osu.git synced 2024-09-21 16:07:24 +08:00

Merge pull request #24114 from peppy/editor-save-local-score-management

Ensure scores always have the correct linked `BeatmapInfo`
This commit is contained in:
Dean Herbert 2023-07-07 15:40:54 +09:00 committed by GitHub
commit d72765b6f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 128 additions and 46 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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<ScoreInfo>().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 reverting the local changes made above
// 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()
{
@ -431,19 +485,19 @@ 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();
beatmap.UpdateLocalScores(r);
});
// 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 +515,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));
});
}

View File

@ -383,14 +383,14 @@ namespace osu.Game.Tests.Database
beatmapInfo.Hash = new_beatmap_hash;
beatmapInfo.ResetOnlineInfo();
beatmapInfo.UpdateLocalScores(s.Realm);
});
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<ScoreInfo>(realm, 1);
Assert.That(realm.Run(r => r.All<ScoreInfo>().First().BeatmapInfo), Is.Null);
// reimport the original beatmap before local modifications
var importAfterUpdate = await importer.ImportAsUpdate(new ProgressNotification(), new ImportTask(pathOnlineCopy), importBeforeUpdate.Value);

View File

@ -87,10 +87,10 @@ namespace osu.Game.Tests.Models
var mock = new Mock<IScoreInfo>();
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]"));
}

View File

@ -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);
});
}

View File

@ -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);
});

View File

@ -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;
}

View File

@ -163,8 +163,12 @@ namespace osu.Game
{
foreach (var score in r.All<ScoreInfo>())
{
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);
}
}
});
@ -204,7 +208,9 @@ namespace osu.Game
{
Logger.Log("Querying for scores that need total score conversion...");
HashSet<Guid> scoreIds = realmAccess.Run(r => new HashSet<Guid>(r.All<ScoreInfo>().Where(s => s.TotalScoreVersion == 30000002).AsEnumerable().Select(s => s.ID)));
HashSet<Guid> scoreIds = realmAccess.Run(r => new HashSet<Guid>(r.All<ScoreInfo>()
.Where(s => s.BeatmapInfo != null && s.TotalScoreVersion == 30000002)
.AsEnumerable().Select(s => s.ID)));
Logger.Log($"Found {scoreIds.Count} scores which require total score conversion.");

View File

@ -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<ScoreInfo>().Where(score => score.BeatmapHash == beatmap.Hash))
score.BeatmapInfo = beatmap;
beatmap.UpdateLocalScores(realm);
}
ProcessBeatmap?.Invoke(model, parameters.Batch ? MetadataLookupScope.LocalCacheFirst : MetadataLookupScope.OnlineFirst);

View File

@ -234,6 +234,22 @@ namespace osu.Game.Beatmaps
}
}
/// <summary>
/// Local scores are retained separate from a beatmap's lifetime, matched via <see cref="ScoreInfo.BeatmapHash"/>.
/// Therefore we need to detach / reattach scores when a beatmap is edited or imported.
/// </summary>
/// <param name="realm">A realm instance in an active write transaction.</param>
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<ScoreInfo>().Where(s => s.BeatmapHash == Hash))
score.BeatmapInfo = this;
}
IBeatmapMetadataInfo IBeatmapInfo.Metadata => Metadata;
IBeatmapSetInfo? IBeatmapInfo.BeatmapSet => BeatmapSet;
IRulesetInfo IBeatmapInfo.Ruleset => Ruleset;

View File

@ -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);

View File

@ -897,7 +897,7 @@ namespace osu.Game.Database
var scores = migration.NewRealm.All<ScoreInfo>();
foreach (var score in scores)
score.BeatmapHash = score.BeatmapInfo.Hash;
score.BeatmapHash = score.BeatmapInfo?.Hash ?? string.Empty;
break;
}

View File

@ -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;

View File

@ -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
};

View File

@ -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");

View File

@ -28,7 +28,7 @@ namespace osu.Game.Scoring
double? PP { get; }
IBeatmapInfo Beatmap { get; }
IBeatmapInfo? Beatmap { get; }
IRulesetInfo Ruleset { get; }

View File

@ -66,7 +66,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));

View File

@ -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<BeatmapInfo>(model.BeatmapInfo.ID)!;
@ -101,10 +103,12 @@ namespace osu.Game.Scoring
/// <param name="score">The score to populate the statistics of.</param>
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();

View File

@ -35,9 +35,16 @@ namespace osu.Game.Scoring
/// The <see cref="BeatmapInfo"/> this score was made against.
/// </summary>
/// <remarks>
/// When setting this, make sure to also set <see cref="BeatmapHash"/> to allow relational consistency when a beatmap is potentially changed.
/// <para>
/// This property may be <see langword="null"/> 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 <see cref="BeatmapInfo"/> if its <see cref="Beatmaps.BeatmapInfo.Hash"/> matches <see cref="BeatmapHash"/>.
/// </para>
/// <para>
/// Due to the above, whenever setting this, make sure to also set <see cref="BeatmapHash"/> to allow relational consistency when a beatmap is potentially changed.
/// </para>
/// </remarks>
public BeatmapInfo BeatmapInfo { get; set; } = null!;
public BeatmapInfo? BeatmapInfo { get; set; }
/// <summary>
/// The <see cref="osu.Game.Beatmaps.BeatmapInfo.Hash"/> at the point in time when the score was set.
@ -150,14 +157,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<INamedFileUsage> 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;

View File

@ -13,7 +13,7 @@ namespace osu.Game.Scoring
/// <summary>
/// A user-presentable display title representing this score.
/// </summary>
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"}";
/// <summary>
/// Orders an array of <see cref="ScoreInfo"/>s by total score.

View File

@ -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)

View File

@ -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);
}

View File

@ -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;

View File

@ -63,7 +63,7 @@ namespace osu.Game.Screens.Ranking
protected override APIRequest? FetchScores(Action<IEnumerable<ScoreInfo>>? 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);

View File

@ -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;

View File

@ -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)
{