1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-11 14:17:26 +08:00

Merge pull request #19308 from frenzibyte/top-rank-badge-order

Fix best rank badge not handling legacy scores properly
This commit is contained in:
Dan Balasescu 2022-09-12 16:20:21 +09:00 committed by GitHub
commit 31225066a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 97 additions and 55 deletions

View File

@ -1,8 +1,7 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
#nullable disable
using System.Diagnostics;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Allocation;
@ -13,6 +12,7 @@ using osu.Framework.Platform;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Rulesets;
using osu.Game.Rulesets.Scoring;
using osu.Game.Scoring;
using osu.Game.Screens.Select.Carousel;
using osu.Game.Tests.Resources;
@ -22,10 +22,10 @@ namespace osu.Game.Tests.Visual.SongSelect
{
public class TestSceneTopLocalRank : OsuTestScene
{
private RulesetStore rulesets;
private BeatmapManager beatmapManager;
private ScoreManager scoreManager;
private TopLocalRank topLocalRank;
private RulesetStore rulesets = null!;
private BeatmapManager beatmapManager = null!;
private ScoreManager scoreManager = null!;
private TopLocalRank topLocalRank = null!;
[BackgroundDependencyLoader]
private void load(GameHost host, AudioManager audio)
@ -47,21 +47,21 @@ namespace osu.Game.Tests.Visual.SongSelect
AddStep("Create local rank", () =>
{
Add(topLocalRank = new TopLocalRank(importedBeatmap)
Child = topLocalRank = new TopLocalRank(importedBeatmap)
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
Scale = new Vector2(10),
});
};
});
AddAssert("No rank displayed initially", () => topLocalRank.DisplayedRank == null);
}
[Test]
public void TestBasicImportDelete()
{
ScoreInfo testScoreInfo = null;
AddAssert("Initially not present", () => !topLocalRank.IsPresent);
ScoreInfo testScoreInfo = null!;
AddStep("Add score for current user", () =>
{
@ -73,25 +73,19 @@ namespace osu.Game.Tests.Visual.SongSelect
scoreManager.Import(testScoreInfo);
});
AddUntilStep("Became present", () => topLocalRank.IsPresent);
AddAssert("Correct rank", () => topLocalRank.Rank == ScoreRank.B);
AddUntilStep("B rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.B);
AddStep("Delete score", () =>
{
scoreManager.Delete(testScoreInfo);
});
AddStep("Delete score", () => scoreManager.Delete(testScoreInfo));
AddUntilStep("Became not present", () => !topLocalRank.IsPresent);
AddUntilStep("No rank displayed", () => topLocalRank.DisplayedRank == null);
}
[Test]
public void TestRulesetChange()
{
ScoreInfo testScoreInfo;
AddStep("Add score for current user", () =>
{
testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
var testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
testScoreInfo.User = API.LocalUser.Value;
testScoreInfo.Rank = ScoreRank.B;
@ -99,25 +93,21 @@ namespace osu.Game.Tests.Visual.SongSelect
scoreManager.Import(testScoreInfo);
});
AddUntilStep("Wait for initial presence", () => topLocalRank.IsPresent);
AddUntilStep("Wait for initial display", () => topLocalRank.DisplayedRank == ScoreRank.B);
AddStep("Change ruleset", () => Ruleset.Value = rulesets.GetRuleset("fruits"));
AddUntilStep("Became not present", () => !topLocalRank.IsPresent);
AddUntilStep("No rank displayed", () => topLocalRank.DisplayedRank == null);
AddStep("Change ruleset back", () => Ruleset.Value = rulesets.GetRuleset("osu"));
AddUntilStep("Became present", () => topLocalRank.IsPresent);
AddUntilStep("B rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.B);
}
[Test]
public void TestHigherScoreSet()
{
ScoreInfo testScoreInfo = null;
AddAssert("Initially not present", () => !topLocalRank.IsPresent);
AddStep("Add score for current user", () =>
{
testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
var testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
testScoreInfo.User = API.LocalUser.Value;
testScoreInfo.Rank = ScoreRank.B;
@ -125,21 +115,58 @@ namespace osu.Game.Tests.Visual.SongSelect
scoreManager.Import(testScoreInfo);
});
AddUntilStep("Became present", () => topLocalRank.IsPresent);
AddUntilStep("Correct rank", () => topLocalRank.Rank == ScoreRank.B);
AddUntilStep("B rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.B);
AddStep("Add higher score for current user", () =>
{
var testScoreInfo2 = TestResources.CreateTestScoreInfo(importedBeatmap);
testScoreInfo2.User = API.LocalUser.Value;
testScoreInfo2.Rank = ScoreRank.S;
testScoreInfo2.TotalScore = testScoreInfo.TotalScore + 1;
testScoreInfo2.Rank = ScoreRank.X;
testScoreInfo2.TotalScore = 1000000;
testScoreInfo2.Statistics = testScoreInfo2.MaximumStatistics;
scoreManager.Import(testScoreInfo2);
});
AddUntilStep("Correct rank", () => topLocalRank.Rank == ScoreRank.S);
AddUntilStep("SS rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.X);
}
[Test]
public void TestLegacyScore()
{
ScoreInfo testScoreInfo = null!;
AddStep("Add legacy score for current user", () =>
{
testScoreInfo = TestResources.CreateTestScoreInfo(importedBeatmap);
testScoreInfo.User = API.LocalUser.Value;
testScoreInfo.Rank = ScoreRank.B;
testScoreInfo.TotalScore = scoreManager.GetTotalScore(testScoreInfo, ScoringMode.Classic);
scoreManager.Import(testScoreInfo);
});
AddUntilStep("B rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.B);
AddStep("Add higher score for current user", () =>
{
var testScoreInfo2 = TestResources.CreateTestScoreInfo(importedBeatmap);
testScoreInfo2.User = API.LocalUser.Value;
testScoreInfo2.Rank = ScoreRank.X;
testScoreInfo2.Statistics = testScoreInfo2.MaximumStatistics;
testScoreInfo2.TotalScore = scoreManager.GetTotalScore(testScoreInfo2);
// ensure second score has a total score (standardised) less than first one (classic)
// despite having better statistics, otherwise this test is pointless.
Debug.Assert(testScoreInfo2.TotalScore < testScoreInfo.TotalScore);
scoreManager.Import(testScoreInfo2);
});
AddUntilStep("SS rank displayed", () => topLocalRank.DisplayedRank == ScoreRank.X);
}
}
}

View File

@ -17,7 +17,7 @@ namespace osu.Game.Online.Leaderboards
set => Model = value;
}
public UpdateableRank(ScoreRank? rank)
public UpdateableRank(ScoreRank? rank = null)
{
Rank = rank;
}

View File

@ -1,13 +1,12 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
#nullable disable
using System;
using System.Linq;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Game.Beatmaps;
using osu.Game.Database;
using osu.Game.Models;
@ -20,27 +19,39 @@ using Realms;
namespace osu.Game.Screens.Select.Carousel
{
public class TopLocalRank : UpdateableRank
public class TopLocalRank : CompositeDrawable
{
private readonly BeatmapInfo beatmapInfo;
[Resolved]
private IBindable<RulesetInfo> ruleset { get; set; }
private IBindable<RulesetInfo> ruleset { get; set; } = null!;
[Resolved]
private RealmAccess realm { get; set; }
private RealmAccess realm { get; set; } = null!;
[Resolved]
private IAPIProvider api { get; set; }
private ScoreManager scoreManager { get; set; } = null!;
private IDisposable scoreSubscription;
[Resolved]
private IAPIProvider api { get; set; } = null!;
private IDisposable? scoreSubscription;
private readonly UpdateableRank updateable;
public ScoreRank? DisplayedRank => updateable.Rank;
public TopLocalRank(BeatmapInfo beatmapInfo)
: base(null)
{
this.beatmapInfo = beatmapInfo;
Size = new Vector2(40, 20);
AutoSizeAxes = Axes.Both;
InternalChild = updateable = new UpdateableRank
{
Size = new Vector2(40, 20),
Alpha = 0,
};
}
protected override void LoadComplete()
@ -55,23 +66,27 @@ namespace osu.Game.Screens.Select.Carousel
.Filter($"{nameof(ScoreInfo.User)}.{nameof(RealmUser.OnlineID)} == $0"
+ $" && {nameof(ScoreInfo.BeatmapInfo)}.{nameof(BeatmapInfo.ID)} == $1"
+ $" && {nameof(ScoreInfo.Ruleset)}.{nameof(RulesetInfo.ShortName)} == $2"
+ $" && {nameof(ScoreInfo.DeletePending)} == false", api.LocalUser.Value.Id, beatmapInfo.ID, ruleset.Value.ShortName)
.OrderByDescending(s => s.TotalScore),
(items, _, _) =>
{
Rank = items.FirstOrDefault()?.Rank;
// Required since presence is changed via IsPresent override
Invalidate(Invalidation.Presence);
});
+ $" && {nameof(ScoreInfo.DeletePending)} == false", api.LocalUser.Value.Id, beatmapInfo.ID, ruleset.Value.ShortName),
localScoresChanged);
}, true);
}
public override bool IsPresent => base.IsPresent && Rank != null;
void localScoresChanged(IRealmCollection<ScoreInfo> sender, ChangeSet? changes, Exception _)
{
// This subscription may fire from changes to linked beatmaps, which we don't care about.
// It's currently not possible for a score to be modified after insertion, so we can safely ignore callbacks with only modifications.
if (changes?.HasCollectionChanges() == false)
return;
ScoreInfo? topScore = scoreManager.OrderByTotalScore(sender.Detach()).FirstOrDefault();
updateable.Rank = topScore?.Rank;
updateable.Alpha = topScore != null ? 1 : 0;
}
}
protected override void Dispose(bool isDisposing)
{
base.Dispose(isDisposing);
scoreSubscription?.Dispose();
}
}