Skip to content
Open
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions UdpHosts/GameServer/Systems/EntityManager/EntityManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1434,20 +1434,19 @@ public void ScopeIn(INetworkPlayer player, IEntity entity)

public void ScopeOut(INetworkPlayer player, IEntity entity)
{
_scopedPlayersByEntity[entity.EntityId].Remove(player);
// Remove the tracking even if the player can't receive the scope out.
// The entity can already be removed. Refer to the same guard in ScopeIn.
if (_scopedPlayersByEntity.TryGetValue(entity.EntityId, out var scopedPlayers))
Comment on lines +1437 to +1439

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As far as I can tell, this can't really throw in the current code. ScopeOut is only called from the scope check in Tick, right after _scopedPlayersByEntity[entity.EntityId] was already indexed for the same entity, and from OnRemovedEntity, before the key gets removed. SpectateServerCommand calls it as well, but the command isn't registered.

So the guard is fine as a defensive measure, but can you adjust the commit message and PR description, so it doesn't read like a crash we actually hit? Or maybe remove the comment all together.

{
scopedPlayers.Remove(player);
}

// Avoid sending messages if player is not in the appropriate state
if (!player.CanReceiveGSS)
{
return;
}

// The entity can already be removed. Refer to the same guard in ScopeIn.
if (_scopedPlayersByEntity.TryGetValue(entity.EntityId, out var scopedPlayers))
{
scopedPlayers.Remove(player);
}

if (entity is CharacterEntity character)
{
if (character.IsPlayerControlled && character.Player == player)
Expand Down