Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce UserStatisticsProvider component and add support for respecting selected ruleset #27128

Merged
merged 28 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
91fb59e
Introduce `LocalUserStatisticsProvider` component
frenzibyte Feb 11, 2024
3ab60b7
Remove `IAPIProvider.Statistics` in favour of the new component
frenzibyte Feb 11, 2024
633d854
Update `UserRankPanel` implementation to use new component
frenzibyte Feb 11, 2024
bc2b705
Fix `ImportTest.TestOsuGameBase` having null ruleset
frenzibyte Feb 11, 2024
11b3fa8
Fix `TestSceneUserPanel` tests failing
frenzibyte Feb 11, 2024
701fb56
Merge branch 'master' into user-statistics-provider
frenzibyte Oct 25, 2024
2fd4952
Fix post-merge errors
frenzibyte Oct 25, 2024
3a57b21
Move `LocalUserStatisticsProvider` to non-base game class and make de…
frenzibyte Oct 25, 2024
44dd813
Make `UserStatisticsWatcher` fully rely on `LocalUserStatisticsProvider`
frenzibyte Oct 25, 2024
fdeb8b9
Merge branch 'master' into user-statistics-provider
frenzibyte Oct 25, 2024
663b769
Update `DiscordRichPresence` to use new statistics provider component
frenzibyte Oct 25, 2024
979065c
Reorder code slightly
frenzibyte Oct 27, 2024
0760451
Merge branch 'master' into user-statistics-provider
peppy Nov 13, 2024
6c8a900
Merge branch 'master' into user-statistics-provider
frenzibyte Nov 17, 2024
4a62828
Decouple game-wide ruleset bindable and refactor `LocalUserStatistics…
frenzibyte Nov 17, 2024
28f8740
Make `DifficultyRecommender` rely on the statistics provider
frenzibyte Nov 17, 2024
07609b6
Fix `UserRankPanel` not updating on ruleset change
frenzibyte Nov 17, 2024
1847b67
Only update user rank panel display when ruleset matches
frenzibyte Nov 17, 2024
caf56af
Fix various test failures
frenzibyte Nov 18, 2024
b106833
Fix more test / component breakage
frenzibyte Nov 18, 2024
74daf85
Replace bindable with an event
frenzibyte Nov 18, 2024
0b52080
Handle logged out user
frenzibyte Nov 18, 2024
631bfad
Replace event subscription with callback in `UserStatisticsWatcher`
frenzibyte Nov 24, 2024
aa1358b
Enable NRT and fix code
frenzibyte Nov 24, 2024
53b3906
Fix failing test
frenzibyte Nov 24, 2024
0a3f3c3
Add guard against fetching statistics for non-legacy rulesets
bdach Nov 25, 2024
b76460f
Schedule the thing
frenzibyte Nov 26, 2024
42c68ba
Add inline comment
frenzibyte Nov 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions osu.Game.Tests/ImportTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// See the LICENCE file in the repository root for full licence text.

using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using NUnit.Framework;
Expand Down Expand Up @@ -64,6 +65,10 @@ private void load()
// Beatmap must be imported before the collection manager is loaded.
if (withBeatmap)
BeatmapManager.Import(TestResources.GetTestBeatmapForImport()).WaitSafely();

// the logic for setting the initial ruleset exists in OsuGame rather than OsuGameBase.
// the ruleset bindable is not meant to be nullable, so assign any ruleset in here.
Ruleset.Value = RulesetStore.AvailableRulesets.First();
}
}
}
Expand Down
141 changes: 141 additions & 0 deletions osu.Game.Tests/Visual/Online/TestSceneLocalUserStatisticsProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System.Collections.Generic;
using NUnit.Framework;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics;
using osu.Framework.Testing;
using osu.Game.Graphics.Sprites;
using osu.Game.Online;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Rulesets.Catch;
using osu.Game.Rulesets.Mania;
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.Taiko;
using osu.Game.Users;

namespace osu.Game.Tests.Visual.Online
{
public partial class TestSceneLocalUserStatisticsProvider : OsuTestScene
{
private LocalUserStatisticsProvider statisticsProvider = null!;

private readonly Dictionary<(int userId, string rulesetName), UserStatistics> serverSideStatistics = new Dictionary<(int userId, string rulesetName), UserStatistics>();

[SetUpSteps]
public void SetUpSteps()
{
AddStep("clear statistics", () => serverSideStatistics.Clear());

setUser(1000);

AddStep("setup provider", () =>
{
OsuSpriteText text;

((DummyAPIAccess)API).HandleRequest = r =>
{
switch (r)
{
case GetUserRequest userRequest:
int userId = int.Parse(userRequest.Lookup);
string rulesetName = userRequest.Ruleset!.ShortName;
var response = new APIUser
{
Id = userId,
Statistics = tryGetStatistics(userId, rulesetName)
};

userRequest.TriggerSuccess(response);
return true;

default:
return false;
}
};

Clear();
Add(statisticsProvider = new LocalUserStatisticsProvider());
Add(text = new OsuSpriteText
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
});

statisticsProvider.Statistics.BindValueChanged(s =>
{
text.Text = s.NewValue == null
? "Statistics: (null)"
: $"Statistics: (total score: {s.NewValue.TotalScore:N0})";
});

Ruleset.Value = new OsuRuleset().RulesetInfo;
});
}

[Test]
public void TestInitialStatistics()
{
AddAssert("initial statistics populated", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(4_000_000));
}

[Test]
public void TestRulesetChanges()
{
AddAssert("statistics from osu", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(4_000_000));
AddStep("change ruleset to taiko", () => Ruleset.Value = new TaikoRuleset().RulesetInfo);
AddAssert("statistics from taiko", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(3_000_000));
AddStep("change ruleset to catch", () => Ruleset.Value = new CatchRuleset().RulesetInfo);
AddAssert("statistics from catch", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(2_000_000));
AddStep("change ruleset to mania", () => Ruleset.Value = new ManiaRuleset().RulesetInfo);
AddAssert("statistics from mania", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(1_000_000));
}

[Test]
public void TestUserChanges()
{
setUser(1001);

AddStep("update statistics for user 1000", () =>
{
serverSideStatistics[(1000, "osu")] = new UserStatistics { TotalScore = 5_000_000 };
serverSideStatistics[(1000, "taiko")] = new UserStatistics { TotalScore = 6_000_000 };
});

AddAssert("statistics matches user 1001 from osu", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(4_000_000));

AddStep("change ruleset to taiko", () => Ruleset.Value = new TaikoRuleset().RulesetInfo);
AddAssert("statistics matches user 1001 from taiko", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(3_000_000));

AddStep("change ruleset to osu", () => Ruleset.Value = new OsuRuleset().RulesetInfo);
setUser(1000, false);

AddAssert("statistics matches user 1000 from osu", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(5_000_000));

AddStep("change ruleset to osu", () => Ruleset.Value = new TaikoRuleset().RulesetInfo);
AddAssert("statistics matches user 1000 from taiko", () => statisticsProvider.Statistics.Value.AsNonNull().TotalScore, () => Is.EqualTo(6_000_000));
}

private UserStatistics tryGetStatistics(int userId, string rulesetName)
=> serverSideStatistics.TryGetValue((userId, rulesetName), out var stats) ? stats : new UserStatistics();

private void setUser(int userId, bool generateStatistics = true)
{
AddStep($"set local user to {userId}", () =>
{
if (generateStatistics)
{
serverSideStatistics[(userId, "osu")] = new UserStatistics { TotalScore = 4_000_000 };
serverSideStatistics[(userId, "taiko")] = new UserStatistics { TotalScore = 3_000_000 };
serverSideStatistics[(userId, "fruits")] = new UserStatistics { TotalScore = 2_000_000 };
serverSideStatistics[(userId, "mania")] = new UserStatistics { TotalScore = 1_000_000 };
}

((DummyAPIAccess)API).LocalUser.Value = new APIUser { Id = userId };
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using osu.Framework.Allocation;
using osu.Framework.Testing;
using osu.Game.Models;
using osu.Game.Online;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
using osu.Game.Online.API.Requests.Responses;
Expand All @@ -25,6 +26,7 @@ public partial class TestSceneSoloStatisticsWatcher : OsuTestScene
{
protected override bool UseOnlineAPI => false;

private LocalUserStatisticsProvider statisticsProvider = null!;
private SoloStatisticsWatcher watcher = null!;

[Resolved]
Expand Down Expand Up @@ -109,7 +111,9 @@ public void SetUpSteps()

AddStep("create watcher", () =>
{
Child = watcher = new SoloStatisticsWatcher();
Clear();
Add(statisticsProvider = new LocalUserStatisticsProvider());
Add(watcher = new SoloStatisticsWatcher(statisticsProvider));
});
}

Expand Down Expand Up @@ -289,7 +293,7 @@ public void TestGlobalStatisticsUpdatedAfterRegistrationAddedAndScoreProcessed()
AddStep("signal score processed", () => ((ISpectatorClient)spectatorClient).UserScoreProcessed(userId, scoreId));
AddUntilStep("update received", () => update != null);
AddAssert("local user values are correct", () => dummyAPI.LocalUser.Value.Statistics.TotalScore, () => Is.EqualTo(5_000_000));
AddAssert("statistics values are correct", () => dummyAPI.Statistics.Value!.TotalScore, () => Is.EqualTo(5_000_000));
AddAssert("statistics values are correct", () => statisticsProvider.Statistics.Value!.TotalScore, () => Is.EqualTo(5_000_000));
}

private int nextUserId = 2000;
Expand Down
23 changes: 14 additions & 9 deletions osu.Game.Tests/Visual/Online/TestSceneUserPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using osu.Framework.Graphics.Containers;
using osu.Framework.Utils;
using osu.Game.Beatmaps;
using osu.Game.Online;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Overlays;
using osu.Game.Rulesets;
Expand All @@ -32,7 +33,10 @@ public partial class TestSceneUserPanel : OsuTestScene
private TestUserListPanel boundPanel2;

[Cached]
private OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Purple);
private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Purple);

[Cached]
private readonly LocalUserStatisticsProvider statisticsProvider = new LocalUserStatisticsProvider();

[Resolved]
private IRulesetStore rulesetStore { get; set; }
Expand All @@ -43,7 +47,11 @@ public void SetUp() => Schedule(() =>
activity.Value = null;
status.Value = null;

Child = new FillFlowContainer
Remove(statisticsProvider, false);
Clear();
Add(statisticsProvider);

Add(new FillFlowContainer
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
Expand Down Expand Up @@ -109,7 +117,7 @@ public void SetUp() => Schedule(() =>
Statistics = new UserStatistics { GlobalRank = null, CountryRank = null }
}) { Width = 300 }
}
};
});

boundPanel1.Status.BindTo(status);
boundPanel1.Activity.BindTo(activity);
Expand Down Expand Up @@ -163,16 +171,13 @@ public void TestUserStatisticsChange()
{
AddStep("update statistics", () =>
{
API.UpdateStatistics(new UserStatistics
statisticsProvider.UpdateStatistics(new UserStatistics
{
GlobalRank = RNG.Next(100000),
CountryRank = RNG.Next(100000)
});
});
AddStep("set statistics to empty", () =>
{
API.UpdateStatistics(new UserStatistics());
}, Ruleset.Value);
});
AddStep("set statistics to empty", () => statisticsProvider.UpdateStatistics(new UserStatistics(), Ruleset.Value));
}

private UserActivity soloGameStatusForRuleset(int rulesetId) => new UserActivity.InSoloGame(new BeatmapInfo(), rulesetStore.GetRuleset(rulesetId)!);
Expand Down
17 changes: 1 addition & 16 deletions osu.Game/Online/API/APIAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ public partial class APIAccess : Component, IAPIProvider
public IBindable<APIUser> LocalUser => localUser;
public IBindableList<APIUser> Friends => friends;
public IBindable<UserActivity> Activity => activity;
public IBindable<UserStatistics> Statistics => statistics;

public INotificationsClient NotificationsClient { get; }

Expand All @@ -70,8 +69,6 @@ public partial class APIAccess : Component, IAPIProvider
private Bindable<UserStatus?> configStatus { get; } = new Bindable<UserStatus?>();
private Bindable<UserStatus?> localUserStatus { get; } = new Bindable<UserStatus?>();

private Bindable<UserStatistics> statistics { get; } = new Bindable<UserStatistics>();

protected bool HasLogin => authentication.Token.Value != null || (!string.IsNullOrEmpty(ProvidedUsername) && !string.IsNullOrEmpty(password));

private readonly CancellationTokenSource cancellationToken = new CancellationTokenSource();
Expand Down Expand Up @@ -595,21 +592,9 @@ public void Logout()
flushQueue();
}

public void UpdateStatistics(UserStatistics newStatistics)
{
statistics.Value = newStatistics;

if (IsLoggedIn)
localUser.Value.Statistics = newStatistics;
}

private static APIUser createGuestUser() => new GuestUser();

private void setLocalUser(APIUser user) => Scheduler.Add(() =>
{
localUser.Value = user;
statistics.Value = user.Statistics;
}, false);
private void setLocalUser(APIUser user) => Scheduler.Add(() => localUser.Value = user, false);

protected override void Dispose(bool isDisposing)
{
Expand Down
16 changes: 0 additions & 16 deletions osu.Game/Online/API/DummyAPIAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ public partial class DummyAPIAccess : Component, IAPIProvider

public Bindable<UserActivity> Activity { get; } = new Bindable<UserActivity>();

public Bindable<UserStatistics?> Statistics { get; } = new Bindable<UserStatistics?>();

public DummyNotificationsClient NotificationsClient { get; } = new DummyNotificationsClient();
INotificationsClient IAPIProvider.NotificationsClient => NotificationsClient;

Expand Down Expand Up @@ -158,11 +156,6 @@ public void AuthenticateSecondFactor(string code)
private void onSuccessfulLogin()
{
state.Value = APIState.Online;
Statistics.Value = new UserStatistics
{
GlobalRank = 1,
CountryRank = 1
};
}

public void Logout()
Expand All @@ -173,14 +166,6 @@ public void Logout()
LocalUser.Value = new GuestUser();
}

public void UpdateStatistics(UserStatistics newStatistics)
{
Statistics.Value = newStatistics;

if (IsLoggedIn)
LocalUser.Value.Statistics = newStatistics;
}

public IHubClientConnector? GetHubConnector(string clientName, string endpoint, bool preferMessagePack) => null;

public IChatClient GetChatClient() => new TestChatClientConnector(this);
Expand All @@ -196,7 +181,6 @@ public void UpdateStatistics(UserStatistics newStatistics)
IBindable<APIUser> IAPIProvider.LocalUser => LocalUser;
IBindableList<APIUser> IAPIProvider.Friends => Friends;
IBindable<UserActivity> IAPIProvider.Activity => Activity;
IBindable<UserStatistics?> IAPIProvider.Statistics => Statistics;

/// <summary>
/// Skip 2FA requirement for next login.
Expand Down
10 changes: 0 additions & 10 deletions osu.Game/Online/API/IAPIProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ public interface IAPIProvider
/// </summary>
IBindable<UserActivity> Activity { get; }

/// <summary>
/// The current user's online statistics.
/// </summary>
IBindable<UserStatistics?> Statistics { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For one, very happy to see this gone from here


/// <summary>
/// The language supplied by this provider to API requests.
/// </summary>
Expand Down Expand Up @@ -123,11 +118,6 @@ public interface IAPIProvider
/// </summary>
void Logout();

/// <summary>
/// Sets Statistics bindable.
/// </summary>
void UpdateStatistics(UserStatistics newStatistics);

/// <summary>
/// Constructs a new <see cref="IHubClientConnector"/>. May be null if not supported.
/// </summary>
Expand Down
Loading
Loading