Skip to content

Commit

Permalink
Allow unsubscribing from solo statistics updates
Browse files Browse the repository at this point in the history
This is more of a safety item. To avoid potential duplicate key in
dictionary errors (and also avoid being slightly memory-leaky), allow
`SoloStatisticsWatcher` consumers to dispose of the subscriptions they
take out.
  • Loading branch information
bdach committed Dec 28, 2022
1 parent 04f9a35 commit 3c0b8af
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 15 deletions.
24 changes: 23 additions & 1 deletion osu.Game.Tests/Visual/Online/TestSceneSoloStatisticsWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public partial class TestSceneSoloStatisticsWatcher : OsuTestScene
private Action<GetUsersRequest>? handleGetUsersRequest;
private Action<GetUserRequest>? handleGetUserRequest;

private IDisposable? subscription;

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

[SetUpSteps]
Expand Down Expand Up @@ -246,6 +248,26 @@ public void TestIgnoredScoreUpdateIsMergedIntoNextOne()
AddAssert("values after are correct", () => update!.After.TotalScore, () => Is.EqualTo(6_000_000));
}

[Test]
public void TestStatisticsUpdateNotFiredAfterSubscriptionDisposal()
{
int userId = getUserId();
setUpUser(userId);

long scoreId = getScoreId();
var ruleset = new OsuRuleset().RulesetInfo;

SoloStatisticsUpdate? update = null;
registerForUpdates(scoreId, ruleset, receivedUpdate => update = receivedUpdate);
AddStep("unsubscribe", () => subscription!.Dispose());

feignScoreProcessing(userId, ruleset, 5_000_000);

AddStep("signal score processed", () => ((ISpectatorClient)spectatorClient).UserScoreProcessed(userId, scoreId));
AddWaitStep("wait a bit", 5);
AddAssert("update not received", () => update == null);
}

private int nextUserId = 2000;
private long nextScoreId = 50000;

Expand All @@ -266,7 +288,7 @@ private void setUpUser(int userId)
}

private void registerForUpdates(long scoreId, RulesetInfo rulesetInfo, Action<SoloStatisticsUpdate> onUpdateReady) =>
AddStep("register for updates", () => watcher.RegisterForStatisticsUpdateAfter(
AddStep("register for updates", () => subscription = watcher.RegisterForStatisticsUpdateAfter(
new ScoreInfo(Beatmap.Value.BeatmapInfo, new OsuRuleset().RulesetInfo, new RealmUser())
{
Ruleset = rulesetInfo,
Expand Down
32 changes: 19 additions & 13 deletions osu.Game/Online/Solo/SoloStatisticsWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,30 @@ protected override void LoadComplete()
/// </summary>
/// <param name="score">The score to listen for the statistics update for.</param>
/// <param name="onUpdateReady">The callback to be invoked once the statistics update has been prepared.</param>
public void RegisterForStatisticsUpdateAfter(ScoreInfo score, Action<SoloStatisticsUpdate> onUpdateReady) => Schedule(() =>
/// <returns>An <see cref="IDisposable"/> representing the subscription. Disposing it is equivalent to unsubscribing from future notifications.</returns>
public IDisposable RegisterForStatisticsUpdateAfter(ScoreInfo score, Action<SoloStatisticsUpdate> onUpdateReady)
{
if (!api.IsLoggedIn)
return;
Schedule(() =>
{
if (!api.IsLoggedIn)
return;

if (!score.Ruleset.IsLegacyRuleset() || score.OnlineID <= 0)
return;
if (!score.Ruleset.IsLegacyRuleset() || score.OnlineID <= 0)
return;

var callback = new StatisticsUpdateCallback(score, onUpdateReady);
var callback = new StatisticsUpdateCallback(score, onUpdateReady);

if (lastProcessedScoreId == score.OnlineID)
{
requestStatisticsUpdate(api.LocalUser.Value.Id, callback);
return;
}
if (lastProcessedScoreId == score.OnlineID)
{
requestStatisticsUpdate(api.LocalUser.Value.Id, callback);
return;
}

callbacks.Add(score.OnlineID, callback);
});
callbacks.Add(score.OnlineID, callback);
});

return new InvokeOnDisposal(() => Schedule(() => callbacks.Remove(score.OnlineID)));
}

private void onUserChanged(APIUser? localUser) => Schedule(() =>
{
Expand Down
4 changes: 3 additions & 1 deletion osu.Game/Screens/Ranking/SoloResultsScreen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public partial class SoloResultsScreen : ResultsScreen
[Resolved]
private SoloStatisticsWatcher soloStatisticsWatcher { get; set; } = null!;

private IDisposable? statisticsSubscription;
private readonly Bindable<SoloStatisticsUpdate?> statisticsUpdate = new Bindable<SoloStatisticsUpdate?>();

public SoloResultsScreen(ScoreInfo score, bool allowRetry)
Expand All @@ -44,7 +45,7 @@ protected override void LoadComplete()
base.LoadComplete();

if (ShowUserStatistics)
soloStatisticsWatcher.RegisterForStatisticsUpdateAfter(Score, update => statisticsUpdate.Value = update);
statisticsSubscription = soloStatisticsWatcher.RegisterForStatisticsUpdateAfter(Score, update => statisticsUpdate.Value = update);
}

protected override StatisticsPanel CreateStatisticsPanel()
Expand Down Expand Up @@ -75,6 +76,7 @@ protected override void Dispose(bool isDisposing)
base.Dispose(isDisposing);

getScoreRequest?.Cancel();
statisticsSubscription?.Dispose();
}
}
}

0 comments on commit 3c0b8af

Please sign in to comment.