Calling NetworkShow from OnNetworkSpawn can cause a duplicate CreateObjectMessage to be sent

[J0shhT]

Description

Calling NetworkShow on a NetworkObject that is in the process of spawning (such as from OnNetworkSpawn) can cause a duplicate CreateObjectMessage to be sent to the remote client. This only applies if the target client is not already an observer of the NetworkObject (such as when SpawnWithObservers is set to false).

Reproduce Steps

1. Setup a minimal project in which a remote client can join a host.
2. Create a new network prefab with a component that calls NetworkShow on its owner in OnNetworkSpawn, like so:

using Unity.Netcode;

public class NgoBugComponent : NetworkBehaviour
{
    public override void OnNetworkSpawn()
    {
        base.OnNetworkSpawn();
        if (IsServer && !NetworkObject.IsNetworkVisibleTo(OwnerClientId))
            NetworkObject.NetworkShow(OwnerClientId);
    }
}

3. Make sure SpawnWithObservers is set to false on the network prefab created in the previous step.
4. Create a scene-placed network object with a component that dynamically spawns the network prefab with ownership set to the remote client. For example:

using System.Collections;
using Unity.Netcode;
using UnityEngine;

public class NgoBugSetup : NetworkBehaviour
{
    [SerializeField] private NetworkObject m_networkPrefab;

    public override void OnNetworkSpawn()
    {
        base.OnNetworkSpawn();
        if (!IsServer)
            return;
        NetworkManager.OnClientConnectedCallback += _onClientConnected;
        foreach (var clientId in NetworkManager.ConnectedClientsIds)
            _onClientConnected(clientId);
    }

    public override void OnNetworkDespawn()
    {
        if (IsServer)
            NetworkManager.OnClientConnectedCallback -= _onClientConnected;
        base.OnNetworkDespawn();
    }

    private void _onClientConnected(ulong clientId)
    {
        if (clientId == NetworkManager.ServerClientId)
            return;
        StartCoroutine(_waitAndThenSpawnNetworkObject(clientId));
    }

    private IEnumerator _waitAndThenSpawnNetworkObject(ulong clientId)
    {
        yield return new WaitForSeconds(1f);
        _spawnNetworkObjectForClient(clientId);
    }

    private void _spawnNetworkObjectForClient(ulong clientId)
    {
        NetworkManager.SpawnManager.InstantiateAndSpawn(m_networkPrefab, clientId, true);
    }
}

5. Point the m_networkPrefab serialized field to the network prefab that was created in step two.
6. Run the host and connect a remote client to it.
7. Observe that the remote client receives a warning about trying to spawn a network object that is already spawned.

Actual Outcome

Two CreateObjectMessage are sent to the remote client for the same NetworkObjectId, which causes a warning for trying to spawn a network object that is already spawned. Two copies of the spawned network object exist in the scene for the remote client (the original and the duplicate).

Expected Outcome

Only one CreateObjectMessage is sent to the remote client for the NetworkObjectId. No warning occurs and only one copy of the network object exists in the scene for the remote client.

Environment

• OS: Windows 11 Pro 23H2
• Unity Version: 2022.3.22f1
• Netcode Version: 1.8.1
• Netcode Commit: eb21383

Additional Context

After investigating this issue on my end by stepping through code in a debugger, I believe I identified the reason for this bug.

com.unity.netcode.gameobjects/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

Lines 773 to 781 in eb21383

	NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene);
	for (int i = 0; i < NetworkManager.ConnectedClientsList.Count; i++)
	{
	if (Observers.Contains(NetworkManager.ConnectedClientsList[i].ClientId))
	{
	NetworkManager.SpawnManager.SendSpawnCallForObject(NetworkManager.ConnectedClientsList[i].ClientId, this);
	}
	}

Line 773 will eventually lead to OnNetworkSpawn being called, which in turn will result in our call to NetworkShow. The code for NetworkShow defers creating the CreateObjectMessage until the end of the frame, but also adds the client ID to the NetworkObject.Observers list:

com.unity.netcode.gameobjects/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

Lines 558 to 559 in eb21383

	NetworkManager.SpawnManager.MarkObjectForShowingTo(this, clientId);
	Observers.Add(clientId);

The problem is, after the call to NetworkManager.SpawnManager.SpawnNetworkObjectLocally returns in NetworkObject.SpawnInternal, it will then loop through the observers and add a CreateObjectMessage for them (since NetworkShow would have added them to the observers list). Then, at the end of the frame, another CreateObjectMessage will be added due to the call to NetworkManager.SpawnManager.MarkObjectForShowingTo inside NetworkShow.

I think a potential solution to this issue would be to only call NetworkManager.SpawnManager.MarkObjectForShowingTo inside NetworkShow if the NetworkObject has finished spawning (i.e., SpawnInternal has returned). Something like this:

if (m_FinishedSpawning)
    NetworkManager.SpawnManager.MarkObjectForShowingTo(this, clientId); 
 Observers.Add(clientId); 

and then m_FinishedSpawning would be set to true at the end of SpawnInternal. A similar thing should probably be done for NetworkHide as well.

I haven't tested this fix myself - just a thought.

[NoelStephensUnity]

Hi @J0shhT,

I have attached a project which provides you with two possible ways to handle hidden object spawning. The primary difference in from your approach is that showing the object should happen while the object is being spawned but afterwards.

The primary area I adjusted:

    private void _spawnNetworkObjectForClient(ulong clientId)
    {
        if (!SpawnWithServerAsOwner)
        {
            // Option 1: You can just spawn the object with the client already set as the owner
            var instance = NetworkManager.SpawnManager.InstantiateAndSpawn(m_networkPrefab, clientId, true);
            // You then should just show the object here and not in the spawn method of the object being spawned.
            instance.NetworkShow(clientId);
        }
        else
        {
            // Option 2: You can spawn the object only on the server side with it not being visible to any client
            // This is sort of a "round about" way to handle this and is a bit more troublesome than it is worth (imo)
            var instance = Instantiate(m_networkPrefab.gameObject);
            var networkObjectInstance = instance.GetComponent<NetworkObject>();
            networkObjectInstance.Spawn();
            // The not recommended path:
            // You have to first "show" the object (i.e. send the CreateObjectMessage to the targeted client)
            networkObjectInstance.NetworkShow(clientId);
            // You then have to change the ownership to the targeted client
            networkObjectInstance.ChangeOwnership(clientId);
        }

        // Option 1: Only 1 message is sent (i.e. the CreateObjectMessage when you show it to the client)
        // Option 2: Two messages are sent (i.e. the CreateObjectMessage when it is shown and the ChangeOwnershipMessage)
    }

As well, I didn't use the NgoBugComponent you provided since you should only handle showing something after it has run through the spawn process (it just changes the "NetworkShow" from happening within the component to afterwards).
Here is the project with the modified code you provided (I renamed NgoBugSetup to HiddenObjectSpawner).
HiddenObjectSpawner.zip

As a side note, in NGO v2.0.0 there are new NetworkBehaviour virtual methods and I believe the NetworkPostSpawn would allow you to handle showing the object from a component attached to the network prefab.

Let me know if this resolves your issue?