mirror of
https://github.com/ppy/osu.git
synced 2025-01-30 01:32:55 +08:00
Add separate path for friend presence notifications
It proved to be too difficult to deal with the flow that clears user states on stopping the watching of global presence updates. It's not helped in the least that friends are updated via the API, so there's a third flow to consider (and the timings therein - both server-spectator and friends are updated concurrently). Simplest is to separate the friends flow, though this does mean some logic and state duplication.
This commit is contained in:
parent
f4d83fe685
commit
7268b2e077
@ -56,16 +56,16 @@ namespace osu.Game.Tests.Visual.Components
|
||||
[Test]
|
||||
public void TestNotifications()
|
||||
{
|
||||
AddStep("bring friend 1 online", () => metadataClient.UserPresenceUpdated(1, new UserPresence { Status = UserStatus.Online }));
|
||||
AddStep("bring friend 1 online", () => metadataClient.FriendPresenceUpdated(1, new UserPresence { Status = UserStatus.Online }));
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1));
|
||||
AddStep("bring friend 1 offline", () => metadataClient.UserPresenceUpdated(1, null));
|
||||
AddStep("bring friend 1 offline", () => metadataClient.FriendPresenceUpdated(1, null));
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(2));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void TestSingleUserNotificationOpensChat()
|
||||
{
|
||||
AddStep("bring friend 1 online", () => metadataClient.UserPresenceUpdated(1, new UserPresence { Status = UserStatus.Online }));
|
||||
AddStep("bring friend 1 online", () => metadataClient.FriendPresenceUpdated(1, new UserPresence { Status = UserStatus.Online }));
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1));
|
||||
|
||||
AddStep("click notification", () =>
|
||||
@ -83,8 +83,8 @@ namespace osu.Game.Tests.Visual.Components
|
||||
{
|
||||
AddStep("bring friends 1 & 2 online", () =>
|
||||
{
|
||||
metadataClient.UserPresenceUpdated(1, new UserPresence { Status = UserStatus.Online });
|
||||
metadataClient.UserPresenceUpdated(2, new UserPresence { Status = UserStatus.Online });
|
||||
metadataClient.FriendPresenceUpdated(1, new UserPresence { Status = UserStatus.Online });
|
||||
metadataClient.FriendPresenceUpdated(2, new UserPresence { Status = UserStatus.Online });
|
||||
});
|
||||
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1));
|
||||
@ -112,7 +112,7 @@ namespace osu.Game.Tests.Visual.Components
|
||||
AddStep("bring friends 1-10 online", () =>
|
||||
{
|
||||
for (int i = 1; i <= 10; i++)
|
||||
metadataClient.UserPresenceUpdated(i, new UserPresence { Status = UserStatus.Online });
|
||||
metadataClient.FriendPresenceUpdated(i, new UserPresence { Status = UserStatus.Online });
|
||||
});
|
||||
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(1));
|
||||
@ -120,7 +120,7 @@ namespace osu.Game.Tests.Visual.Components
|
||||
AddStep("bring friends 1-10 offline", () =>
|
||||
{
|
||||
for (int i = 1; i <= 10; i++)
|
||||
metadataClient.UserPresenceUpdated(i, null);
|
||||
metadataClient.FriendPresenceUpdated(i, null);
|
||||
});
|
||||
|
||||
AddUntilStep("wait for notification", () => notificationOverlay.AllNotifications.Count(), () => Is.EqualTo(2));
|
||||
|
@ -6,6 +6,7 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Diagnostics;
|
||||
using System.Linq;
|
||||
using System.Net;
|
||||
using System.Net.Http;
|
||||
using System.Net.Sockets;
|
||||
@ -18,6 +19,7 @@ using osu.Framework.Extensions.ObjectExtensions;
|
||||
using osu.Framework.Graphics;
|
||||
using osu.Framework.Logging;
|
||||
using osu.Game.Configuration;
|
||||
using osu.Game.Extensions;
|
||||
using osu.Game.Localisation;
|
||||
using osu.Game.Online.API.Requests;
|
||||
using osu.Game.Online.API.Requests.Responses;
|
||||
@ -75,7 +77,6 @@ namespace osu.Game.Online.API
|
||||
|
||||
protected bool HasLogin => authentication.Token.Value != null || (!string.IsNullOrEmpty(ProvidedUsername) && !string.IsNullOrEmpty(password));
|
||||
|
||||
private readonly Dictionary<int, APIRelation> friendsMapping = new Dictionary<int, APIRelation>();
|
||||
private readonly CancellationTokenSource cancellationToken = new CancellationTokenSource();
|
||||
|
||||
private readonly Logger log;
|
||||
@ -404,8 +405,6 @@ namespace osu.Game.Online.API
|
||||
|
||||
public IChatClient GetChatClient() => new WebSocketChatClient(this);
|
||||
|
||||
public APIRelation GetFriend(int userId) => friendsMapping.GetValueOrDefault(userId);
|
||||
|
||||
public RegistrationRequest.RegistrationRequestErrors CreateAccount(string email, string username, string password)
|
||||
{
|
||||
Debug.Assert(State.Value == APIState.Offline);
|
||||
@ -597,8 +596,6 @@ namespace osu.Game.Online.API
|
||||
Schedule(() =>
|
||||
{
|
||||
setLocalUser(createGuestUser());
|
||||
|
||||
friendsMapping.Clear();
|
||||
friends.Clear();
|
||||
});
|
||||
|
||||
@ -615,12 +612,14 @@ namespace osu.Game.Online.API
|
||||
friendsReq.Failure += _ => state.Value = APIState.Failing;
|
||||
friendsReq.Success += res =>
|
||||
{
|
||||
friendsMapping.Clear();
|
||||
friends.Clear();
|
||||
// Add new friends into local list.
|
||||
HashSet<int> friendsSet = friends.Select(f => f.TargetID).ToHashSet();
|
||||
friends.AddRange(res.Where(f => !friendsSet.Contains(f.TargetID)));
|
||||
|
||||
foreach (var u in res)
|
||||
friendsMapping[u.TargetID] = u;
|
||||
friends.AddRange(res);
|
||||
// Remove non-friends from local lists.
|
||||
friendsSet.Clear();
|
||||
friendsSet.AddRange(res.Select(f => f.TargetID));
|
||||
friends.RemoveAll(f => !friendsSet.Contains(f.TargetID));
|
||||
};
|
||||
|
||||
Queue(friendsReq);
|
||||
|
@ -2,7 +2,6 @@
|
||||
// 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 osu.Framework.Bindables;
|
||||
@ -195,8 +194,6 @@ namespace osu.Game.Online.API
|
||||
|
||||
public IChatClient GetChatClient() => new TestChatClientConnector(this);
|
||||
|
||||
public APIRelation? GetFriend(int userId) => Friends.FirstOrDefault(r => r.TargetID == userId);
|
||||
|
||||
public RegistrationRequest.RegistrationRequestErrors? CreateAccount(string email, string username, string password)
|
||||
{
|
||||
Thread.Sleep(200);
|
||||
|
@ -152,13 +152,6 @@ namespace osu.Game.Online.API
|
||||
/// </summary>
|
||||
IChatClient GetChatClient();
|
||||
|
||||
/// <summary>
|
||||
/// Retrieves a friend from a given user ID.
|
||||
/// </summary>
|
||||
/// <param name="userId">The friend's user ID.</param>
|
||||
/// <returns>The <see cref="APIRelation"/> object representing the friend, if any.</returns>
|
||||
APIRelation? GetFriend(int userId);
|
||||
|
||||
/// <summary>
|
||||
/// Create a new user account. This is a blocking operation.
|
||||
/// </summary>
|
||||
|
@ -2,6 +2,7 @@
|
||||
// See the LICENCE file in the repository root for full licence text.
|
||||
|
||||
using System.Collections.Generic;
|
||||
using System.Collections.Specialized;
|
||||
using System.Linq;
|
||||
using osu.Framework.Allocation;
|
||||
using osu.Framework.Bindables;
|
||||
@ -43,7 +44,10 @@ namespace osu.Game.Online
|
||||
private OsuConfigManager config { get; set; } = null!;
|
||||
|
||||
private readonly Bindable<bool> notifyOnFriendPresenceChange = new BindableBool();
|
||||
private readonly IBindableDictionary<int, UserPresence> userStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
private readonly IBindableList<APIRelation> friends = new BindableList<APIRelation>();
|
||||
private readonly IBindableDictionary<int, UserPresence> friendStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
private readonly HashSet<APIUser> onlineAlertQueue = new HashSet<APIUser>();
|
||||
private readonly HashSet<APIUser> offlineAlertQueue = new HashSet<APIUser>();
|
||||
|
||||
@ -56,42 +60,11 @@ namespace osu.Game.Online
|
||||
|
||||
config.BindWith(OsuSetting.NotifyOnFriendPresenceChange, notifyOnFriendPresenceChange);
|
||||
|
||||
userStates.BindTo(metadataClient.UserStates);
|
||||
userStates.BindCollectionChanged((_, args) =>
|
||||
{
|
||||
switch (args.Action)
|
||||
{
|
||||
case NotifyDictionaryChangedAction.Add:
|
||||
foreach ((int userId, var _) in args.NewItems!)
|
||||
{
|
||||
if (api.GetFriend(userId)?.TargetUser is APIUser user)
|
||||
{
|
||||
if (!offlineAlertQueue.Remove(user))
|
||||
{
|
||||
onlineAlertQueue.Add(user);
|
||||
lastOnlineAlertTime ??= Time.Current;
|
||||
}
|
||||
}
|
||||
}
|
||||
friends.BindTo(api.Friends);
|
||||
friends.BindCollectionChanged(onFriendsChanged, true);
|
||||
|
||||
break;
|
||||
|
||||
case NotifyDictionaryChangedAction.Remove:
|
||||
foreach ((int userId, var _) in args.OldItems!)
|
||||
{
|
||||
if (api.GetFriend(userId)?.TargetUser is APIUser user)
|
||||
{
|
||||
if (!onlineAlertQueue.Remove(user))
|
||||
{
|
||||
offlineAlertQueue.Add(user);
|
||||
lastOfflineAlertTime ??= Time.Current;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
});
|
||||
friendStates.BindTo(metadataClient.FriendStates);
|
||||
friendStates.BindCollectionChanged(onFriendStatesChanged, true);
|
||||
}
|
||||
|
||||
protected override void Update()
|
||||
@ -102,6 +75,82 @@ namespace osu.Game.Online
|
||||
alertOfflineUsers();
|
||||
}
|
||||
|
||||
private void onFriendsChanged(object? sender, NotifyCollectionChangedEventArgs e)
|
||||
{
|
||||
switch (e.Action)
|
||||
{
|
||||
case NotifyCollectionChangedAction.Add:
|
||||
foreach (APIRelation friend in e.NewItems!.Cast<APIRelation>())
|
||||
{
|
||||
if (friend.TargetUser is not APIUser user)
|
||||
continue;
|
||||
|
||||
if (friendStates.TryGetValue(friend.TargetID, out _))
|
||||
markUserOnline(user);
|
||||
}
|
||||
|
||||
break;
|
||||
|
||||
case NotifyCollectionChangedAction.Remove:
|
||||
foreach (APIRelation friend in e.OldItems!.Cast<APIRelation>())
|
||||
{
|
||||
if (friend.TargetUser is not APIUser user)
|
||||
continue;
|
||||
|
||||
onlineAlertQueue.Remove(user);
|
||||
offlineAlertQueue.Remove(user);
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
private void onFriendStatesChanged(object? sender, NotifyDictionaryChangedEventArgs<int, UserPresence> e)
|
||||
{
|
||||
switch (e.Action)
|
||||
{
|
||||
case NotifyDictionaryChangedAction.Add:
|
||||
foreach ((int friendId, _) in e.NewItems!)
|
||||
{
|
||||
APIRelation? friend = friends.FirstOrDefault(f => f.TargetID == friendId);
|
||||
|
||||
if (friend?.TargetUser is APIUser user)
|
||||
markUserOnline(user);
|
||||
}
|
||||
|
||||
break;
|
||||
|
||||
case NotifyDictionaryChangedAction.Remove:
|
||||
foreach ((int friendId, _) in e.OldItems!)
|
||||
{
|
||||
APIRelation? friend = friends.FirstOrDefault(f => f.TargetID == friendId);
|
||||
|
||||
if (friend?.TargetUser is APIUser user)
|
||||
markUserOffline(user);
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
private void markUserOnline(APIUser user)
|
||||
{
|
||||
if (!offlineAlertQueue.Remove(user))
|
||||
{
|
||||
onlineAlertQueue.Add(user);
|
||||
lastOnlineAlertTime ??= Time.Current;
|
||||
}
|
||||
}
|
||||
|
||||
private void markUserOffline(APIUser user)
|
||||
{
|
||||
if (!onlineAlertQueue.Remove(user))
|
||||
{
|
||||
offlineAlertQueue.Add(user);
|
||||
lastOfflineAlertTime ??= Time.Current;
|
||||
}
|
||||
}
|
||||
|
||||
private void alertOnlineUsers()
|
||||
{
|
||||
if (onlineAlertQueue.Count == 0)
|
||||
|
@ -21,6 +21,11 @@ namespace osu.Game.Online.Metadata
|
||||
/// </summary>
|
||||
Task UserPresenceUpdated(int userId, UserPresence? status);
|
||||
|
||||
/// <summary>
|
||||
/// Delivers and update of the <see cref="UserPresence"/> of a friend with the supplied <paramref name="userId"/>.
|
||||
/// </summary>
|
||||
Task FriendPresenceUpdated(int userId, UserPresence? presence);
|
||||
|
||||
/// <summary>
|
||||
/// Delivers an update of the current "daily challenge" status.
|
||||
/// Null value means there is no "daily challenge" currently active.
|
||||
|
@ -42,6 +42,11 @@ namespace osu.Game.Online.Metadata
|
||||
/// </summary>
|
||||
public abstract IBindableDictionary<int, UserPresence> UserStates { get; }
|
||||
|
||||
/// <summary>
|
||||
/// Dictionary keyed by user ID containing all of the <see cref="UserPresence"/> information about currently online friends received from the server.
|
||||
/// </summary>
|
||||
public abstract IBindableDictionary<int, UserPresence> FriendStates { get; }
|
||||
|
||||
/// <inheritdoc/>
|
||||
public abstract Task UpdateActivity(UserActivity? activity);
|
||||
|
||||
@ -57,6 +62,9 @@ namespace osu.Game.Online.Metadata
|
||||
/// <inheritdoc/>
|
||||
public abstract Task UserPresenceUpdated(int userId, UserPresence? presence);
|
||||
|
||||
/// <inheritdoc/>
|
||||
public abstract Task FriendPresenceUpdated(int userId, UserPresence? presence);
|
||||
|
||||
#endregion
|
||||
|
||||
#region Daily Challenge
|
||||
|
@ -3,7 +3,6 @@
|
||||
|
||||
using System;
|
||||
using System.Diagnostics;
|
||||
using System.Linq;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.AspNetCore.SignalR.Client;
|
||||
@ -27,14 +26,14 @@ namespace osu.Game.Online.Metadata
|
||||
public override IBindableDictionary<int, UserPresence> UserStates => userStates;
|
||||
private readonly BindableDictionary<int, UserPresence> userStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
public override IBindableDictionary<int, UserPresence> FriendStates => friendStates;
|
||||
private readonly BindableDictionary<int, UserPresence> friendStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
public override IBindable<DailyChallengeInfo?> DailyChallengeInfo => dailyChallengeInfo;
|
||||
private readonly Bindable<DailyChallengeInfo?> dailyChallengeInfo = new Bindable<DailyChallengeInfo?>();
|
||||
|
||||
private readonly string endpoint;
|
||||
|
||||
[Resolved]
|
||||
private IAPIProvider api { get; set; } = null!;
|
||||
|
||||
private IHubClientConnector? connector;
|
||||
private Bindable<int> lastQueueId = null!;
|
||||
private IBindable<APIUser> localUser = null!;
|
||||
@ -49,7 +48,7 @@ namespace osu.Game.Online.Metadata
|
||||
}
|
||||
|
||||
[BackgroundDependencyLoader]
|
||||
private void load(OsuConfigManager config)
|
||||
private void load(IAPIProvider api, OsuConfigManager config)
|
||||
{
|
||||
// Importantly, we are intentionally not using MessagePack here to correctly support derived class serialization.
|
||||
// More information on the limitations / reasoning can be found in osu-server-spectator's initialisation code.
|
||||
@ -63,6 +62,7 @@ namespace osu.Game.Online.Metadata
|
||||
// https://github.com/dotnet/aspnetcore/issues/15198
|
||||
connection.On<BeatmapUpdates>(nameof(IMetadataClient.BeatmapSetsUpdated), ((IMetadataClient)this).BeatmapSetsUpdated);
|
||||
connection.On<int, UserPresence?>(nameof(IMetadataClient.UserPresenceUpdated), ((IMetadataClient)this).UserPresenceUpdated);
|
||||
connection.On<int, UserPresence?>(nameof(IMetadataClient.FriendPresenceUpdated), ((IMetadataClient)this).FriendPresenceUpdated);
|
||||
connection.On<DailyChallengeInfo?>(nameof(IMetadataClient.DailyChallengeUpdated), ((IMetadataClient)this).DailyChallengeUpdated);
|
||||
connection.On<MultiplayerRoomScoreSetEvent>(nameof(IMetadataClient.MultiplayerRoomScoreSet), ((IMetadataClient)this).MultiplayerRoomScoreSet);
|
||||
connection.On(nameof(IStatefulUserHubClient.DisconnectRequested), ((IMetadataClient)this).DisconnectRequested);
|
||||
@ -108,6 +108,7 @@ namespace osu.Game.Online.Metadata
|
||||
{
|
||||
isWatchingUserPresence.Value = false;
|
||||
userStates.Clear();
|
||||
friendStates.Clear();
|
||||
dailyChallengeInfo.Value = null;
|
||||
});
|
||||
return;
|
||||
@ -209,6 +210,19 @@ namespace osu.Game.Online.Metadata
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
public override Task FriendPresenceUpdated(int userId, UserPresence? presence)
|
||||
{
|
||||
Schedule(() =>
|
||||
{
|
||||
if (presence?.Status != null)
|
||||
friendStates[userId] = presence.Value;
|
||||
else
|
||||
friendStates.Remove(userId);
|
||||
});
|
||||
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
public override async Task BeginWatchingUserPresence()
|
||||
{
|
||||
if (connector?.IsConnected.Value != true)
|
||||
@ -228,15 +242,7 @@ namespace osu.Game.Online.Metadata
|
||||
throw new OperationCanceledException();
|
||||
|
||||
// must be scheduled before any remote calls to avoid mis-ordering.
|
||||
Schedule(() =>
|
||||
{
|
||||
foreach (int userId in userStates.Keys.ToArray())
|
||||
{
|
||||
if (api.GetFriend(userId) == null)
|
||||
userStates.Remove(userId);
|
||||
}
|
||||
});
|
||||
|
||||
Schedule(() => userStates.Clear());
|
||||
Debug.Assert(connection != null);
|
||||
await connection.InvokeAsync(nameof(IMetadataServer.EndWatchingUserPresence)).ConfigureAwait(false);
|
||||
Logger.Log($@"{nameof(OnlineMetadataClient)} stopped watching user presence", LoggingTarget.Network);
|
||||
|
@ -2,7 +2,6 @@
|
||||
// See the LICENCE file in the repository root for full licence text.
|
||||
|
||||
using System;
|
||||
using System.Linq;
|
||||
using System.Threading.Tasks;
|
||||
using osu.Framework.Allocation;
|
||||
using osu.Framework.Bindables;
|
||||
@ -23,6 +22,9 @@ namespace osu.Game.Tests.Visual.Metadata
|
||||
public override IBindableDictionary<int, UserPresence> UserStates => userStates;
|
||||
private readonly BindableDictionary<int, UserPresence> userStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
public override IBindableDictionary<int, UserPresence> FriendStates => friendStates;
|
||||
private readonly BindableDictionary<int, UserPresence> friendStates = new BindableDictionary<int, UserPresence>();
|
||||
|
||||
public override Bindable<DailyChallengeInfo?> DailyChallengeInfo => dailyChallengeInfo;
|
||||
private readonly Bindable<DailyChallengeInfo?> dailyChallengeInfo = new Bindable<DailyChallengeInfo?>();
|
||||
|
||||
@ -67,7 +69,7 @@ namespace osu.Game.Tests.Visual.Metadata
|
||||
|
||||
public override Task UserPresenceUpdated(int userId, UserPresence? presence)
|
||||
{
|
||||
if (isWatchingUserPresence.Value || api.Friends.Any(f => f.TargetID == userId))
|
||||
if (isWatchingUserPresence.Value)
|
||||
{
|
||||
if (presence.HasValue)
|
||||
userStates[userId] = presence.Value;
|
||||
@ -78,6 +80,16 @@ namespace osu.Game.Tests.Visual.Metadata
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
public override Task FriendPresenceUpdated(int userId, UserPresence? presence)
|
||||
{
|
||||
if (presence.HasValue)
|
||||
friendStates[userId] = presence.Value;
|
||||
else
|
||||
friendStates.Remove(userId);
|
||||
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
public override Task<BeatmapUpdates> GetChangesSince(int queueId)
|
||||
=> Task.FromResult(new BeatmapUpdates(Array.Empty<int>(), queueId));
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user