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

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

Setup a minimal project in which a remote client can join a host.
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:
```cs
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);
    }
}

Point the m_networkPrefab serialized field to the network prefab that was created in step two.
Run the host and connect a remote client to it.
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: https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/commit/eb213838233cbee2030891203bcb72d2354a5a7d

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.

https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/blob/eb213838233cbee2030891203bcb72d2354a5a7d/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs#L773-L781

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:

https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/blob/eb213838233cbee2030891203bcb72d2354a5a7d/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs#L558-L559

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.

Unity-Technologies / com.unity.netcode.gameobjects