NetworkObject CheckObjectVisibility delegate not invoked for connecting clients

[kjohnston-ooi]

Describe the bug
When spawning objects on the server and supplying a value for the CheckObjectVisibility delegate, this delegate is not being invoked when clients subsequently connect, causing the client to potentially see objects they shouldn't.

To Reproduce
Steps to reproduce the behavior:

1. Build and run a server that spawns one network object on launch, and provide a value for the CheckObjectVisibility delegate like the following: networkObject.CheckObjectVisibility = (clientId) => { return false; };
2. Start a client and connect to the server
3. Observe the spawned object from the server will be visible, despite the documentation stating that the CheckObjectVisibility callback should be invoked for each object when a new client connects

Expected outcome
The object would not be visible to any connecting clients, as the CheckObjectVisibility condition will always return false.

Additional notes
I believe I know why this issue occurs, but while I was able to make some changes that appear to fix it locally, I'm not confident there isn't some other underlying issues I haven't noticed yet. What I did notice was that in NetworkSpawnManager there is the following method:

internal void UpdateObservedNetworkObjects(ulong clientId)
{
    foreach (var sobj in SpawnedObjectsList)
    {
        if (sobj.CheckObjectVisibility == null || NetworkManager.IsServer)
        {
            if (!sobj.Observers.Contains(clientId))
            {
                sobj.Observers.Add(clientId);
            }
        }
        else
        {
            if (sobj.CheckObjectVisibility(clientId))
            {
                sobj.Observers.Add(clientId);
            }
            else if (sobj.Observers.Contains(clientId))
            {
                sobj.Observers.Remove(clientId);
            }
        }
    }
}

However because this only ever appears to be called by the server, the first condition will always return true and the CheckObjectVisibility callback will never be invoked here. After changing this from NetworkManager.IsServer to NetworkManager.ServerClientId == clientId, the callback began to be invoked as inspected, and only when it returned true would the connecting client be added to the observers list for the object.

The second issue was that, even with the above condition, the client was still actually receiving instructions from the server to instantiate the objects they shouldn't have visibility for, which caused the objects to be visible for the client but they wouldn't receive any other network events for them such as a network variable change. I was able to fix this by changing the following block in SceneEventData from:

internal void AddSpawnedNetworkObjects()
{
    m_NetworkObjectsSync = m_NetworkManager.SpawnManager.SpawnedObjectsList.ToList();
    m_NetworkObjectsSync.Sort(SortNetworkObjects);
}

to:

internal void AddSpawnedNetworkObjects()
{
    m_NetworkObjectsSync = m_NetworkManager.SpawnManager.SpawnedObjectsList.Where(x => x.Observers.Contains(TargetClientId)).ToList();
    m_NetworkObjectsSync.Sort(SortNetworkObjects);
}

so that the server filters the objects to only the ones the client is actually observing before sending them the scene event data.

As I mentioned I'm not sure this is a "proper" fix or not, but it may at least help further investigation into the issue.

Environment:

• OS: Windows 10
• Unity Version: 2021.2.0b15
• Netcode Version: develop
• Netcode Commit: 3720b3b

[will-mearns]

Added to backlog as MTT-1560