Skip to content

Commit

Permalink
Merge pull request #14603 from smoogipoo/score-ordering
Browse files Browse the repository at this point in the history 
Fix scores not being ordered correctly on leaderboards
  • Loading branch information
@peppy
peppy authored Sep 7, 2021
2 parents 5026485 + 3d8faea commit fa62c84
Show file tree
Hide file tree
Showing 13 changed files with 320 additions and 188 deletions.
3 changes: 3 additions & 0 deletions osu.Game.Tests/Visual/Playlists/TestScenePlaylistsResultsScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,14 @@ private void createResults(Func<ScoreInfo> getScore = null)
Ruleset = { Value = new OsuRuleset().RulesetInfo }
}));
});

AddUntilStep("wait for load", () => resultsScreen.ChildrenOfType<ScorePanelList>().FirstOrDefault()?.AllPanelsVisible == true);
}

private void waitForDisplay()
{
AddUntilStep("wait for request to complete", () => requestComplete);
AddUntilStep("wait for panels to be visible", () => resultsScreen.ChildrenOfType<ScorePanelList>().FirstOrDefault()?.AllPanelsVisible == true);
AddWaitStep("wait for display", 5);
}

Expand Down
7 changes: 4 additions & 3 deletions osu.Game.Tests/Visual/Ranking/TestSceneResultsScreen.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void TestShowHideStatisticsViaOutsideClick()
TestResultsScreen screen = null;

AddStep("load results", () => Child = new TestResultsContainer(screen = createResultsScreen()));
AddUntilStep("wait for loaded", () => screen.IsLoaded);
AddUntilStep("wait for load", () => this.ChildrenOfType<ScorePanelList>().Single().AllPanelsVisible);

AddStep("click expanded panel", () =>
{
Expand Down Expand Up @@ -138,7 +138,7 @@ public void TestShowHideStatistics()
TestResultsScreen screen = null;

AddStep("load results", () => Child = new TestResultsContainer(screen = createResultsScreen()));
AddUntilStep("wait for loaded", () => screen.IsLoaded);
AddUntilStep("wait for load", () => this.ChildrenOfType<ScorePanelList>().Single().AllPanelsVisible);

AddStep("click expanded panel", () =>
{
Expand Down Expand Up @@ -177,7 +177,7 @@ public void TestShowStatisticsAndClickOtherPanel()
TestResultsScreen screen = null;

AddStep("load results", () => Child = new TestResultsContainer(screen = createResultsScreen()));
AddUntilStep("wait for loaded", () => screen.IsLoaded);
AddUntilStep("wait for load", () => this.ChildrenOfType<ScorePanelList>().Single().AllPanelsVisible);

ScorePanel expandedPanel = null;
ScorePanel contractedPanel = null;
Expand Down Expand Up @@ -223,6 +223,7 @@ public void TestDownloadButtonInitiallyDisabled()
TestResultsScreen screen = null;

AddStep("load results", () => Child = new TestResultsContainer(screen = createResultsScreen()));
AddUntilStep("wait for load", () => this.ChildrenOfType<ScorePanelList>().Single().AllPanelsVisible);

AddAssert("download button is disabled", () => !screen.ChildrenOfType<DownloadButton>().Last().Enabled.Value);

Expand Down
21 changes: 21 additions & 0 deletions osu.Game.Tests/Visual/Ranking/TestSceneScorePanelList.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ public void TestSelectMultipleScores()
var firstScore = new TestScoreInfo(new OsuRuleset().RulesetInfo);
var secondScore = new TestScoreInfo(new OsuRuleset().RulesetInfo);

firstScore.User.Username = "A";
secondScore.User.Username = "B";

createListStep(() => new ScorePanelList());

AddStep("add scores and select first", () =>
Expand All @@ -168,6 +171,8 @@ public void TestSelectMultipleScores()
list.SelectedScore.Value = firstScore;
});

AddUntilStep("wait for load", () => list.AllPanelsVisible);

assertScoreState(firstScore, true);
assertScoreState(secondScore, false);

Expand All @@ -182,6 +187,22 @@ public void TestSelectMultipleScores()
assertExpandedPanelCentred();
}

[Test]
public void TestAddScoreImmediately()
{
var score = new TestScoreInfo(new OsuRuleset().RulesetInfo);

createListStep(() =>
{
var newList = new ScorePanelList { SelectedScore = { Value = score } };
newList.AddScore(score);
return newList;
});

assertScoreState(score, true);
assertExpandedPanelCentred();
}

private void createListStep(Func<ScorePanelList> creationFunc)
{
AddStep("create list", () => Child = list = creationFunc().With(d =>
Expand Down
2 changes: 1 addition & 1 deletion osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapLeaderboard.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl

dependencies.Cache(rulesetStore = new RulesetStore(ContextFactory));
dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesetStore, null, dependencies.Get<AudioManager>(), Resources, dependencies.Get<GameHost>(), Beatmap.Default));
dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, null, ContextFactory));
dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, null, ContextFactory, Scheduler));

return dependencies;
}
Expand Down
21 changes: 12 additions & 9 deletions osu.Game.Tests/Visual/UserInterface/TestSceneDeleteLocalScore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class TestSceneDeleteLocalScore : OsuManualInputManagerTestScene
private BeatmapManager beatmapManager;
private ScoreManager scoreManager;

private readonly List<ScoreInfo> scores = new List<ScoreInfo>();
private readonly List<ScoreInfo> importedScores = new List<ScoreInfo>();
private BeatmapInfo beatmap;

[Cached]
Expand Down Expand Up @@ -82,7 +82,7 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl

dependencies.Cache(rulesetStore = new RulesetStore(ContextFactory));
dependencies.Cache(beatmapManager = new BeatmapManager(LocalStorage, ContextFactory, rulesetStore, null, dependencies.Get<AudioManager>(), Resources, dependencies.Get<GameHost>(), Beatmap.Default));
dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, null, ContextFactory));
dependencies.Cache(scoreManager = new ScoreManager(rulesetStore, () => beatmapManager, LocalStorage, null, ContextFactory, Scheduler));

beatmap = beatmapManager.Import(new ImportTask(TestResources.GetQuickTestBeatmapForImport())).Result.Beatmaps[0];

Expand All @@ -100,11 +100,9 @@ protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnl
User = new User { Username = "TestUser" },
};

scores.Add(scoreManager.Import(score).Result);
importedScores.Add(scoreManager.Import(score).Result);
}

scores.Sort(Comparer<ScoreInfo>.Create((s1, s2) => s2.TotalScore.CompareTo(s1.TotalScore)));

return dependencies;
}

Expand Down Expand Up @@ -134,9 +132,14 @@ public void SetupSteps()
[Test]
public void TestDeleteViaRightClick()
{
ScoreInfo scoreBeingDeleted = null;
AddStep("open menu for top score", () =>
{
InputManager.MoveMouseTo(leaderboard.ChildrenOfType<LeaderboardScore>().First());
var leaderboardScore = leaderboard.ChildrenOfType<LeaderboardScore>().First();

scoreBeingDeleted = leaderboardScore.Score;

InputManager.MoveMouseTo(leaderboardScore);
InputManager.Click(MouseButton.Right);
});

Expand All @@ -158,14 +161,14 @@ public void TestDeleteViaRightClick()
InputManager.Click(MouseButton.Left);
});

AddUntilStep("score removed from leaderboard", () => leaderboard.Scores.All(s => s.OnlineScoreID != scores[0].OnlineScoreID));
AddUntilStep("score removed from leaderboard", () => leaderboard.Scores.All(s => s.OnlineScoreID != scoreBeingDeleted.OnlineScoreID));
}

[Test]
public void TestDeleteViaDatabase()
{
AddStep("delete top score", () => scoreManager.Delete(scores[0]));
AddUntilStep("score removed from leaderboard", () => leaderboard.Scores.All(s => s.OnlineScoreID != scores[0].OnlineScoreID));
AddStep("delete top score", () => scoreManager.Delete(importedScores[0]));
AddUntilStep("score removed from leaderboard", () => leaderboard.Scores.All(s => s.OnlineScoreID != importedScores[0].OnlineScoreID));
}
}
}
28 changes: 15 additions & 13 deletions osu.Game/Online/Leaderboards/LeaderboardScore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@ public class LeaderboardScore : OsuClickableContainer, IHasContextMenu
{
public const float HEIGHT = 60;

public readonly ScoreInfo Score;

private const float corner_radius = 5;
private const float edge_margin = 5;
private const float background_alpha = 0.25f;
private const float rank_width = 35;

protected Container RankContainer { get; private set; }

private readonly ScoreInfo score;
private readonly int? rank;
private readonly bool allowHighlight;

Expand All @@ -67,7 +68,8 @@ public class LeaderboardScore : OsuClickableContainer, IHasContextMenu

public LeaderboardScore(ScoreInfo score, int? rank, bool allowHighlight = true)
{
this.score = score;
Score = score;

this.rank = rank;
this.allowHighlight = allowHighlight;

Expand All @@ -78,9 +80,9 @@ public LeaderboardScore(ScoreInfo score, int? rank, bool allowHighlight = true)
[BackgroundDependencyLoader]
private void load(IAPIProvider api, OsuColour colour, ScoreManager scoreManager)
{
var user = score.User;
var user = Score.User;

statisticsLabels = GetStatistics(score).Select(s => new ScoreComponentLabel(s)).ToList();
statisticsLabels = GetStatistics(Score).Select(s => new ScoreComponentLabel(s)).ToList();

ClickableAvatar innerAvatar;

Expand Down Expand Up @@ -198,15 +200,15 @@ private void load(IAPIProvider api, OsuColour colour, ScoreManager scoreManager)
{
TextColour = Color4.White,
GlowColour = Color4Extensions.FromHex(@"83ccfa"),
Current = scoreManager.GetBindableTotalScoreString(score),
Current = scoreManager.GetBindableTotalScoreString(Score),
Font = OsuFont.Numeric.With(size: 23),
},
RankContainer = new Container
{
Size = new Vector2(40f, 20f),
Children = new[]
{
scoreRank = new UpdateableRank(score.Rank)
scoreRank = new UpdateableRank(Score.Rank)
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
Expand All @@ -223,7 +225,7 @@ private void load(IAPIProvider api, OsuColour colour, ScoreManager scoreManager)
AutoSizeAxes = Axes.Both,
Direction = FillDirection.Horizontal,
Spacing = new Vector2(1),
ChildrenEnumerable = score.Mods.Select(mod => new ModIcon(mod) { Scale = new Vector2(0.375f) })
ChildrenEnumerable = Score.Mods.Select(mod => new ModIcon(mod) { Scale = new Vector2(0.375f) })
},
},
},
Expand Down Expand Up @@ -389,14 +391,14 @@ public MenuItem[] ContextMenuItems
{
List<MenuItem> items = new List<MenuItem>();

if (score.Mods.Length > 0 && modsContainer.Any(s => s.IsHovered) && songSelect != null)
items.Add(new OsuMenuItem("Use these mods", MenuItemType.Highlighted, () => songSelect.Mods.Value = score.Mods));
if (Score.Mods.Length > 0 && modsContainer.Any(s => s.IsHovered) && songSelect != null)
items.Add(new OsuMenuItem("Use these mods", MenuItemType.Highlighted, () => songSelect.Mods.Value = Score.Mods));

if (score.Files?.Count > 0)
items.Add(new OsuMenuItem("Export", MenuItemType.Standard, () => scoreManager.Export(score)));
if (Score.Files?.Count > 0)
items.Add(new OsuMenuItem("Export", MenuItemType.Standard, () => scoreManager.Export(Score)));

if (score.ID != 0)
items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => dialogOverlay?.Push(new LocalScoreDeleteDialog(score))));
if (Score.ID != 0)
items.Add(new OsuMenuItem("Delete", MenuItemType.Destructive, () => dialogOverlay?.Push(new LocalScoreDeleteDialog(Score))));

return items.ToArray();
}
Expand Down
2 changes: 1 addition & 1 deletion osu.Game/OsuGameBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ private void load()
dependencies.Cache(fileStore = new FileStore(contextFactory, Storage));

// ordering is important here to ensure foreign keys rules are not broken in ModelStore.Cleanup()
dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Host, () => difficultyCache, LocalConfig));
dependencies.Cache(ScoreManager = new ScoreManager(RulesetStore, () => BeatmapManager, Storage, API, contextFactory, Scheduler, Host, () => difficultyCache, LocalConfig));
dependencies.Cache(BeatmapManager = new BeatmapManager(Storage, contextFactory, RulesetStore, API, Audio, Resources, Host, defaultBeatmap, true));

// this should likely be moved to ArchiveModelManager when another case appears where it is necessary
Expand Down
41 changes: 28 additions & 13 deletions osu.Game/Overlays/BeatmapSet/Scores/ScoresContainer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
using osu.Framework.Graphics.Shapes;
using osuTK;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Beatmaps;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
using osu.Framework.Bindables;
using osu.Game.Graphics.UserInterface;
using osu.Game.Rulesets;
using osu.Game.Scoring;
using osu.Game.Screens.Select.Leaderboards;
using osu.Game.Users;

Expand Down Expand Up @@ -42,34 +45,46 @@ public class ScoresContainer : BeatmapSetLayoutSection
[Resolved]
private RulesetStore rulesets { get; set; }

[Resolved]
private ScoreManager scoreManager { get; set; }

private GetScoresRequest getScoresRequest;

private CancellationTokenSource loadCancellationSource;

protected APILegacyScores Scores
{
set => Schedule(() =>
{
loadCancellationSource?.Cancel();
loadCancellationSource = new CancellationTokenSource();

topScoresContainer.Clear();
scoreTable.ClearScores();
scoreTable.Hide();

if (value?.Scores.Any() != true)
{
scoreTable.ClearScores();
scoreTable.Hide();
return;
}

var scoreInfos = value.Scores.Select(s => s.CreateScoreInfo(rulesets)).ToList();
var topScore = scoreInfos.First();
scoreManager.OrderByTotalScoreAsync(value.Scores.Select(s => s.CreateScoreInfo(rulesets)).ToArray(), loadCancellationSource.Token)
.ContinueWith(ordered => Schedule(() =>
{
if (loadCancellationSource.IsCancellationRequested)
return;

var topScore = ordered.Result.First();

scoreTable.DisplayScores(scoreInfos, topScore.Beatmap?.Status.GrantsPerformancePoints() == true);
scoreTable.Show();
scoreTable.DisplayScores(ordered.Result, topScore.Beatmap?.Status.GrantsPerformancePoints() == true);
scoreTable.Show();

var userScore = value.UserScore;
var userScoreInfo = userScore?.Score.CreateScoreInfo(rulesets);
var userScore = value.UserScore;
var userScoreInfo = userScore?.Score.CreateScoreInfo(rulesets);

topScoresContainer.Add(new DrawableTopScore(topScore));
topScoresContainer.Add(new DrawableTopScore(topScore));

if (userScoreInfo != null && userScoreInfo.OnlineScoreID != topScore.OnlineScoreID)
topScoresContainer.Add(new DrawableTopScore(userScoreInfo, userScore.Position));
if (userScoreInfo != null && userScoreInfo.OnlineScoreID != topScore.OnlineScoreID)
topScoresContainer.Add(new DrawableTopScore(userScoreInfo, userScore.Position));
}), TaskContinuationOptions.OnlyOnRanToCompletion);
});
}

Expand Down
Loading

0 comments on commit fa62c84

Please sign in to comment.