fix: address review on the notification pushes

- Force-join's RoomUpdate carries the room's role grants, so it no
  longer wipes them from the client's cached room.
- The consumable push reports IsTransferable the same as the inventory
  list does.
- A rejected private-instance join sends the owner at most one "wants
  to join" message per requester and instance every 5 minutes.
- SubscribeToPlayers ignores sets over 5,000 ids and keeps the current
  subscriptions.
- The subscriber exclusion list looks up online friends' connections in
  one pipelined Redis batch instead of one call per friend.
This commit is contained in:
Alexa
2026-10-24 20:25:51 +03:00
parent e33145307f
commit afad9bbc5a
6 changed files with 99 additions and 15 deletions
@@ -463,7 +463,9 @@ public class InventoryController(
CreatedAt = inv.AcquiredAt, CreatedAt = inv.AcquiredAt,
ActiveDurationMinutes = (int?)null, ActiveDurationMinutes = (int?)null,
inv.IsActive, inv.IsActive,
IsTransferable = true, // Same as GET api/consumables/v2/getUnlocked, so a push never flips
// what the inventory list says.
IsTransferable = false,
}; };
private static object ToConsumableWire(PlayerInventoryEntity inv, StoreItemEntity item) private static object ToConsumableWire(PlayerInventoryEntity inv, StoreItemEntity item)
@@ -3520,9 +3520,15 @@ public class AdminController(
.Where(sc => sc.RoomId == room.Id) .Where(sc => sc.RoomId == room.Id)
.OrderBy(sc => sc.OrderIndex) .OrderBy(sc => sc.OrderIndex)
.ToListAsync(); .ToListAsync();
// With every role grant: the client replaces its cached room with
// this, so a payload without them would drop everyone's roles.
var forceJoinRoles = await db.RoomRoles
.Where(r => r.RoomId == room.Id)
.ToListAsync();
await notifications.NotifyAsync(id, await notifications.NotifyAsync(id,
PushNotificationId.SubscriptionUpdateRoom, PushNotificationId.SubscriptionUpdateRoom,
DorkNet.Server.Controllers.API.Rooms.V2.RoomsController.BuildRoomServerDetails(room, forceJoinScenes)); DorkNet.Server.Controllers.API.Rooms.V2.RoomsController.BuildRoomServerDetails(
room, forceJoinScenes, roles: forceJoinRoles));
await LogAsync("force_join", "player", id, await LogAsync("force_join", "player", id,
$"room={body.RoomId} sub={subRoomId} instance={roomInstanceId} photon={photonRoomId}"); $"room={body.RoomId} sub={subRoomId} instance={roomInstanceId} photon={photonRoomId}");
@@ -880,8 +880,11 @@ public class GoToController(
callerId, playerId, theirRoom.RoomInstanceId); callerId, playerId, theirRoom.RoomInstanceId);
// Tell the OWNER someone tried to follow them in, as the // Tell the OWNER someone tried to follow them in, as the
// client's own "wants to join" message (RequestGameInvite, // client's own "wants to join" message (RequestGameInvite,
// from the caller), so they can send an invite. // from the caller), so they can send an invite. At most once
if (callerId is long requester) // per requester, owner and instance per cooldown, so retrying
// doesn't fill the owner's inbox.
if (callerId is long requester
&& JoinRequestCooldown.TryStart(requester, playerId, theirRoom.RoomInstanceId))
await systemNotifications.SendAsync(playerId, await systemNotifications.SendAsync(playerId,
SystemNotificationService.MessageType.RequestGameInvite, fromPlayerId: requester); SystemNotificationService.MessageType.RequestGameInvite, fromPlayerId: requester);
return Ok(new MatchmakingResponseDto return Ok(new MatchmakingResponseDto
@@ -1670,3 +1673,35 @@ public class RoomInstanceDto
}; };
} }
} }
/// <summary>Rate limit for "wants to join" messages: one per (requester,
/// owner, room instance) per <see cref="Window"/>. Later retries and other
/// instances still go through. Per replica, which is enough to stop a
/// retry loop from flooding an inbox.</summary>
internal static class JoinRequestCooldown
{
public static readonly TimeSpan Window = TimeSpan.FromMinutes(5);
private static readonly System.Collections.Concurrent.ConcurrentDictionary<(long, long, long), DateTime> Last = new();
/// <summary>True when no request for this key was sent within the window
/// (and records this one); false while it is cooling down.</summary>
public static bool TryStart(long requester, long owner, long roomInstanceId)
{
var now = DateTime.UtcNow;
if (Last.Count > 10_000)
foreach (var (key, at) in Last)
if (now - at > Window) Last.TryRemove(key, out _);
var k = (requester, owner, roomInstanceId);
while (true)
{
if (!Last.TryGetValue(k, out var prev))
{
if (Last.TryAdd(k, now)) return true;
continue;
}
if (now - prev < Window) return false;
if (Last.TryUpdate(k, now, prev)) return true;
}
}
}
+19 -9
View File
@@ -161,6 +161,12 @@ public class NotifyHub(
private const string SubscriptionsKey = "presence-subscriptions"; private const string SubscriptionsKey = "presence-subscriptions";
/// <summary>Most players one connection may subscribe to. The client sends
/// its relationships plus the players around it (dozens); anything far
/// past that is not a real client, and each id costs a group membership
/// on the backplane.</summary>
public const int MaxPresenceSubscriptions = 5000;
/// <summary><c>SubscribeToPlayers(PlayerSubscriptionListDTO {PlayerIds: /// <summary><c>SubscribeToPlayers(PlayerSubscriptionListDTO {PlayerIds:
/// List&lt;int&gt;})</c> (RecNet.Runtime OMHPBIEHOHN.txt:3430): the full set /// List&lt;int&gt;})</c> (RecNet.Runtime OMHPBIEHOHN.txt:3430): the full set
/// of players this client wants presence for — its relationships, the /// of players this client wants presence for — its relationships, the
@@ -187,6 +193,13 @@ public class NotifyHub(
return; return;
} }
if (me is long self) requested.Remove(self); if (me is long self) requested.Remove(self);
if (requested.Count > MaxPresenceSubscriptions)
{
logger.LogWarning(
"[hub] SubscribeToPlayers over the {Max} limit player={PlayerId} conn={ConnectionId} count={Count}; kept the current set",
MaxPresenceSubscriptions, me, Context.ConnectionId, requested.Count);
return;
}
var current = Context.Items.TryGetValue(SubscriptionsKey, out var existing) && existing is HashSet<long> set var current = Context.Items.TryGetValue(SubscriptionsKey, out var existing) && existing is HashSet<long> set
? set ? set
@@ -232,15 +245,12 @@ public class NotifyHub(
var list = payload; var list = payload;
if (payload.ValueKind == JsonValueKind.Object) if (payload.ValueKind == JsonValueKind.Object)
{ {
var found = false; var match = payload.EnumerateObject()
foreach (var prop in payload.EnumerateObject()) .Where(prop => string.Equals(prop.Name, "PlayerIds", StringComparison.OrdinalIgnoreCase))
{ .Select(prop => (JsonElement?)prop.Value)
if (!string.Equals(prop.Name, "PlayerIds", StringComparison.OrdinalIgnoreCase)) continue; .FirstOrDefault();
list = prop.Value; if (match is not JsonElement value) return false;
found = true; list = value;
break;
}
if (!found) return false;
} }
if (list.ValueKind != JsonValueKind.Array) return false; if (list.ValueKind != JsonValueKind.Array) return false;
foreach (var item in list.EnumerateArray()) foreach (var item in list.EnumerateArray())
@@ -148,6 +148,34 @@ public class OnlinePresenceService
lock (set) return set.ToArray(); lock (set) return set.ToArray();
} }
/// <summary>Connection ids of every player in <paramref name="playerIds"/>,
/// in one go: the Redis lookups are issued together, so the multiplexer
/// pipelines them into one round trip instead of one per player.</summary>
public async Task<IReadOnlyCollection<string>> GetConnectionsAsync(IEnumerable<long> playerIds)
{
var ids = playerIds.Distinct().ToArray();
if (ids.Length == 0) return Array.Empty<string>();
if (_redis is { } mux)
{
try
{
var db = mux.GetDatabase();
var sets = await Task.WhenAll(ids.Select(id => db.SetMembersAsync(ConnsKey(id))));
return sets.SelectMany(s => s).Select(v => v.ToString()).ToArray();
}
catch
{
// Redis sick → fall through to the local fallback, as
// GetConnections does.
}
}
var result = new List<string>();
foreach (var id in ids)
if (_localConns.TryGetValue(id, out var set))
lock (set) result.AddRange(set);
return result;
}
/// <summary>Snapshot of currently-online player ids across the /// <summary>Snapshot of currently-online player ids across the
/// cluster. Returns an empty array when nobody's online; never null. /// cluster. Returns an empty array when nobody's online; never null.
/// Callers typically <c>.ToHashSet()</c> the result for membership /// Callers typically <c>.ToHashSet()</c> the result for membership
@@ -143,8 +143,11 @@ public sealed class PresenceBroadcastService(
// offline frame otherwise. Friends were served above and the // offline frame otherwise. Friends were served above and the
// subject must never get a frame about themselves, so their // subject must never get a frame about themselves, so their
// connections are excluded. // connections are excluded.
var exclude = connections.GetConnections(playerId) // Only friends who are online have connections; one call for the
.Concat(friendIds.SelectMany(connections.GetConnections)) // online set, then one pipelined lookup for all of them.
var onlineIds = connections.OnlinePlayerIds().ToHashSet();
var exclude = (await connections.GetConnectionsAsync(
friendIds.Where(onlineIds.Contains).Append(playerId)))
.Distinct() .Distinct()
.ToList(); .ToList();
var publicToAll = visibility == PresenceVisibility.Public && online; var publicToAll = visibility == PresenceVisibility.Public && online;