fix: NetworkSceneManager does not synchronize despawned in-scene placed NetworkObjects [MTT-2924] (#1898)

* fix

MTT-2924
This is the first pass at solving the issue where late joining clients are not synchronized properly with despawned in-scene NetworkObjects.

* Update CHANGELOG.md

* Update CHANGELOG.md

* fix

This allows for respawning of in-scene placed NetworkObjects without having to register them with the NetworkManager.

* Update CHANGELOG.md

* style

* Update CHANGELOG.md

* style

whitespace fixes.

* fix

Passing the entire SceneObject structure as opposed to adding more parameters to the NetworkSpawnManager.HasPrefab method in order to be able to extract the NetworkSceneHandle and include the in-scene placed NetworkObjects as part of the "prefabs to search for".

* update

This is some work towards integration tests where clients might be able to actually load scenes as opposed to fake loading them

* fix

Fixed some minor issues with tests.
Updated HiddenVariableTests as it needed some timing related adjustments.

* Update NetworkSceneManagerEventDataPoolTest.cs

* style

* style

* test - integration test clients now load scenes

This update allows for clients to load scenes and synchronize in-scene placed NetworkObjects during integration testing.

* test - fix

Now that clients are loading scenes, needed to make some minor adjustments.

* test - fix

Realized I don't need to simulate client loading delay unless the client is blocked from loading.
Also had to adjust whether the metrics tests waited for the client or not when loading.

* fix

Fixes an issue that can occur where this becomes enabled in test runner and will throw an exception if target is null.

* fix - NetworkSceneManager.GetSceneRelativeInSceneNetworkObject

GetSceneRelativeInSceneNetworkObject needed to get the sceneHandle before checking for it existing in the ScenePlacedObjects.

* fix - AddDespawnedInSceneNetworkObjects

AddDespawnedInSceneNetworkObjects needed to only return despawned NetworkObjects belonging to the NetworkManager.

* test - fix - Integration test client scene loading

This fixes some of the last edge case scenarios such as:
- Not loading scenes for clients when a non-NetcodeIntegrationTest based integration test is running since this does require some additional management and cleanup.
- Assuring that in-scene NetworkObjects have the right NetworkManager assigned when loaded
- Added a check to assure that it didn't time out while waiting for all clients to connect
- Increased the NetcodeIntegrationTestHelpers default timeout to 2s vs 1s (clients loading scenes can slow down when things happen)
- NetcodeIntegrationTests now unload all scenes loaded (but the test runner scene) during teardown and onetimeteardown.

* test update

Just adding timeout checks in both NetworkVariableTests and TimeMultiInstanceTest

* test assets

Updating some of the assets used for integration testing now that clients are loading scenes too.

* test fix

Changing the scenes being loaded to avoid integration test specific SDK architecture related issues.

* test fix

An early shutdown was not needed here

* test fix

Disabling the audio listener in the EmptyScene to reduce console spam during integration tests.

* test fix

Fixing edge case timing issue with MessageOrdering.

* test InScenePlacedNetworkObjects

This is the integration test for this PR.
It tests late joining client synchronization with despawned NetworkObjects and tests that in-scene placed NetworkObjects do not need to be registered with the NetworkManager to be spawned.

* fix

Don't throw an exception when calling GetObservers or IsNetworkVisibleTo when the NetworkObject is not spawned.

* fix

fixing typo/mistake for the scene to be unloaded.

* style

* Update ObjectNameIdentifier.cs

* style

* style

updating comments and removing LFs and some commented out code.

* update and style

updating comments and minor final tweaks.

* update

minor changes suggested by Jeffrey R.

* update

removing scene management specific code from places where scene management is disabled.

* style

updated some comments

* fix

Fixing an issue where CheckValueOnClient was crashing.  Integration test timing did change slightly now that clients load scenes, so it was possible that the key would not exist by the time it was being checked.

* fix  consoles

reducing the number of clients from 9 to 4 in some of the NetworkSceneManager related tests as I think it impacted performance enough to cause clients to take longer than the default timeout of 2 seconds.

* fix

Hopefully this will fix the issue with Xbox Scarlett

* test - fix

Removing m_CanCheckLoadUnloadEvents as that was a sanity check while I was testing clients loading scenes.  This could possibly fix an edge case scenario issue where a message might get ignored.

* update

removing NetworkSceneHandle from NetworkObject as it was not needed.

* Update CHANGELOG.md

* update

Removing wait for clients to unload scenes and unloading them via SceneManager as the test has already completed and this is more clean up than anything else.

* test update

Removed the crazy code in ObjectNameIdentifier for adding tag information and simplified it by storing the original name and using that each time it needs to regenerate the tag information.
Moved where the NetworkManager name and tag information is applied to get the right client id.
Added additional debug information.

* style

* test

Testing with verbose mode to see why this passes on Windows 10 in a stand alone build and in the editor but fails on Yamato desktops.

* test

adding more debug info.

* test fix

Looks like the scene isn't really loaded when it says it is loaded.
Moving when the NetworkSceneManager's callback is invoked and adding delay before invoking.

* test fix

Making some additional modifications to see if it was when the NetworkSceneManager scene Load/Unload event action is invoked and reducing the number of clients to 1.

* Test Fix

Minor tweaks and merging develop into this branch.

* test fix

Found a bug in some recent changes that would cause a crash under specific scenarios.

* fix

* test and style

Went back through some of the changed files and removed some code no longer needed, extended some of the verbose debug info for any future debugging purposes, and updated some of the comments.

* style

removing commented out code.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,14 +18,15 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Changed
 
 - `unmanaged` structs are no longer universally accepted as RPC parameters because some structs (i.e., structs with pointers in them, such as `NativeList<T>`) can't be supported by the default memcpy struct serializer. Structs that are intended to be serialized across the network must add `INetworkSerializeByMemcpy` to the interface list (i.e., `struct Foo : INetworkSerializeByMemcpy`). This interface is empty and just serves to mark the struct as compatible with memcpy serialization. For external structs you can't edit, you can pass them to RPCs by wrapping them in `ForceNetworkSerializeByMemcpy<T>`. (#1901)
+- Changed requirement to register in-scene placed NetworkObjects with `NetworkManager` in order to respawn them.  In-scene placed NetworkObjects are now automatically tracked during runtime and no longer need to be registered as a NetworkPrefab.  (#1898)
 
 ### Removed
 - Removed `SIPTransport` (#1870)
 
 - Removed `ClientNetworkTransform` from the package samples and moved to Boss Room's Utilities package which can be found [here](https://github.com/Unity-Technologies/com.unity.multiplayer.samples.coop/blob/main/Packages/com.unity.multiplayer.samples.coop/Utilities/Net/ClientAuthority/ClientNetworkTransform.cs).
 
 ### Fixed
-
+- Fixed issue where `NetworkSceneManager` did not synchronize despawned in-scene placed NetworkObjects. (#1898)
 - Fixed `NetworkTransform` generating false positive rotation delta checks when rolling over between 0 and 360 degrees. (#1890)
 - Fixed client throwing an exception if it has messages in the outbound queue when processing the `NetworkEvent.Disconnect` event and is using UTP. (#1884)
 - Fixed issue during client synchronization if 'ValidateSceneBeforeLoading' returned false it would halt the client synchronization process resulting in a client that was approved but not synchronized or fully connected with the server. (#1883)
@@ -36,6 +37,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ## [1.0.0-pre.7] - 2022-04-06
 
 ### Added
+
 - Added editor only check prior to entering into play mode if the currently open and active scene is in the build list and if not displays a dialog box asking the user if they would like to automatically add it prior to entering into play mode. (#1828)
 - Added `UnityTransport` implementation and `com.unity.transport` package dependency (#1823)
 - Added `NetworkVariableWritePermission` to `NetworkVariableBase` and implemented `Owner` client writable netvars. (#1762)

com.unity.netcode.gameobjects/Editor/NetworkBehaviourEditor.cs

@@ -218,6 +218,11 @@ public override void OnInspectorGUI()
         /// </summary>
         private void OnEnable()
         {
+            // This can be null and throw an exception when running test runner in the editor
+            if (target == null)
+            {
+                return;
+            }
             // When we first add a NetworkBehaviour this editor will be enabled
             // so we go ahead and check for an already existing NetworkObject here
             CheckForNetworkObject((target as NetworkBehaviour).gameObject);

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

@@ -1838,7 +1838,7 @@ internal void HandleApproval(ulong ownerClientId, bool createPlayerObject, uint?
 
                 if (createPlayerObject)
                 {
-                    var networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, position, rotation);
+                    var networkObject = SpawnManager.CreateLocalNetworkObject(false, playerPrefabHash ?? NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash, ownerClientId, null, null, position, rotation);
                     SpawnManager.SpawnNetworkObjectLocally(networkObject, SpawnManager.GetNetworkObjectId(), false, true, ownerClientId, false);
 
                     ConnectedClients[ownerClientId].PlayerObject = networkObject;

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

@@ -151,6 +151,7 @@ internal string GetNameForMetrics()
 #endif
         }
 
+        private readonly HashSet<ulong> m_EmptyULongHashSet = new HashSet<ulong>();
         /// <summary>
         /// Returns Observers enumerator
         /// </summary>
@@ -159,7 +160,7 @@ public HashSet<ulong>.Enumerator GetObservers()
         {
             if (!IsSpawned)
             {
-                throw new SpawnStateException("Object is not spawned");
+                return m_EmptyULongHashSet.GetEnumerator();
             }
 
             return Observers.GetEnumerator();
@@ -174,9 +175,8 @@ public bool IsNetworkVisibleTo(ulong clientId)
         {
             if (!IsSpawned)
             {
-                throw new SpawnStateException("Object is not spawned");
+                return false;
             }
-
             return Observers.Contains(clientId);
         }
 
@@ -372,7 +372,7 @@ private void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool play
                 throw new NotServerException($"Only server can spawn {nameof(NetworkObject)}s");
             }
 
-            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), false, playerObject, ownerClientId, destroyWithScene);
+            NetworkManager.SpawnManager.SpawnNetworkObjectLocally(this, NetworkManager.SpawnManager.GetNetworkObjectId(), IsSceneObject.HasValue && IsSceneObject.Value, playerObject, ownerClientId, destroyWithScene);
 
             for (int i = 0; i < NetworkManager.ConnectedClientsList.Count; i++)
             {
@@ -865,16 +865,17 @@ public struct TransformData : INetworkSerializeByMemcpy
             public NetworkObject OwnerObject;
             public ulong TargetClientId;
 
+            public int NetworkSceneHandle;
+
             public unsafe void Serialize(FastBufferWriter writer)
             {
-                if (!writer.TryBeginWrite(
-                    sizeof(HeaderData) +
-                    (Header.HasParent ? FastBufferWriter.GetWriteSize(ParentObjectId) : 0) +
-                    (Header.HasTransform ? FastBufferWriter.GetWriteSize(Transform) : 0) +
-                    (Header.IsReparented
-                        ? FastBufferWriter.GetWriteSize(IsLatestParentSet) +
-                          (IsLatestParentSet ? FastBufferWriter.GetWriteSize<ulong>() : 0)
-                        : 0)))
+                var writeSize = sizeof(HeaderData);
+                writeSize += Header.HasParent ? FastBufferWriter.GetWriteSize(ParentObjectId) : 0;
+                writeSize += Header.HasTransform ? FastBufferWriter.GetWriteSize(Transform) : 0;
+                writeSize += Header.IsReparented ? FastBufferWriter.GetWriteSize(IsLatestParentSet) + (IsLatestParentSet ? FastBufferWriter.GetWriteSize<ulong>() : 0) : 0;
+                writeSize += Header.IsSceneObject ? FastBufferWriter.GetWriteSize<int>() : 0;
+
+                if (!writer.TryBeginWrite(writeSize))
                 {
                     throw new OverflowException("Could not serialize SceneObject: Out of buffer space.");
                 }
@@ -900,6 +901,16 @@ public unsafe void Serialize(FastBufferWriter writer)
                     }
                 }
 
+                // In-Scene NetworkObjects are uniquely identified NetworkPrefabs defined by their
+                // NetworkSceneHandle and GlobalObjectIdHash. Since each loaded scene has a unique
+                // handle, it provides us with a unique and persistent "scene prefab asset" instance.
+                // This is only set on in-scene placed NetworkObjects to reduce the over-all packet
+                // sizes for dynamically spawned NetworkObjects.
+                if (Header.IsSceneObject)
+                {
+                    writer.WriteValue(OwnerObject.gameObject.scene.handle);
+                }
+
                 OwnerObject.WriteNetworkVariableData(writer, TargetClientId);
             }
 
@@ -910,10 +921,12 @@ public unsafe void Deserialize(FastBufferReader reader)
                     throw new OverflowException("Could not deserialize SceneObject: Out of buffer space.");
                 }
                 reader.ReadValue(out Header);
-                if (!reader.TryBeginRead(
-                    (Header.HasParent ? FastBufferWriter.GetWriteSize(ParentObjectId) : 0) +
-                    (Header.HasTransform ? FastBufferWriter.GetWriteSize(Transform) : 0) +
-                    (Header.IsReparented ? FastBufferWriter.GetWriteSize(IsLatestParentSet) : 0)))
+                var readSize = Header.HasParent ? FastBufferWriter.GetWriteSize(ParentObjectId) : 0;
+                readSize += Header.HasTransform ? FastBufferWriter.GetWriteSize(Transform) : 0;
+                readSize += Header.IsReparented ? FastBufferWriter.GetWriteSize(IsLatestParentSet) + (IsLatestParentSet ? FastBufferWriter.GetWriteSize<ulong>() : 0) : 0;
+                readSize += Header.IsSceneObject ? FastBufferWriter.GetWriteSize<int>() : 0;
+
+                if (!reader.TryBeginRead(readSize))
                 {
                     throw new OverflowException("Could not deserialize SceneObject: Out of buffer space.");
                 }
@@ -937,6 +950,16 @@ public unsafe void Deserialize(FastBufferReader reader)
                         LatestParent = latestParent;
                     }
                 }
+
+                // In-Scene NetworkObjects are uniquely identified NetworkPrefabs defined by their
+                // NetworkSceneHandle and GlobalObjectIdHash. Since each loaded scene has a unique
+                // handle, it provides us with a unique and persistent "scene prefab asset" instance.
+                // Client-side NetworkSceneManagers use this to locate their local instance of the
+                // NetworkObject instance.
+                if (Header.IsSceneObject)
+                {
+                    reader.ReadValue(out NetworkSceneHandle);
+                }
             }
         }
 
@@ -1006,6 +1029,7 @@ internal static NetworkObject AddSceneObject(in SceneObject sceneObject, FastBuf
             Vector3? position = null;
             Quaternion? rotation = null;
             ulong? parentNetworkId = null;
+            int? networkSceneHandle = null;
 
             if (sceneObject.Header.HasTransform)
             {
@@ -1018,10 +1042,15 @@ internal static NetworkObject AddSceneObject(in SceneObject sceneObject, FastBuf
                 parentNetworkId = sceneObject.ParentObjectId;
             }
 
+            if (sceneObject.Header.IsSceneObject)
+            {
+                networkSceneHandle = sceneObject.NetworkSceneHandle;
+            }
+
             //Attempt to create a local NetworkObject
             var networkObject = networkManager.SpawnManager.CreateLocalNetworkObject(
                 sceneObject.Header.IsSceneObject, sceneObject.Header.Hash,
-                sceneObject.Header.OwnerClientId, parentNetworkId, position, rotation, sceneObject.Header.IsReparented);
+                sceneObject.Header.OwnerClientId, parentNetworkId, networkSceneHandle, position, rotation, sceneObject.Header.IsReparented);
 
             networkObject?.SetNetworkParenting(sceneObject.Header.IsReparented, sceneObject.LatestParent);
 

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/CreateObjectMessage.cs

@@ -19,7 +19,7 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context)
             }
 
             ObjectInfo.Deserialize(reader);
-            if (!networkManager.NetworkConfig.ForceSamePrefabs && !networkManager.SpawnManager.HasPrefab(ObjectInfo.Header.IsSceneObject, ObjectInfo.Header.Hash))
+            if (!networkManager.NetworkConfig.ForceSamePrefabs && !networkManager.SpawnManager.HasPrefab(ObjectInfo))
             {
                 networkManager.DeferredMessageManager.DeferMessage(IDeferredMessageManager.TriggerType.OnAddPrefab, ObjectInfo.Header.Hash, reader, ref context);
                 return false;

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DestroyObjectMessage.cs

@@ -3,6 +3,7 @@ namespace Unity.Netcode
     internal struct DestroyObjectMessage : INetworkMessage, INetworkSerializeByMemcpy
     {
         public ulong NetworkObjectId;
+        public bool DestroyGameObject;
 
         public void Serialize(FastBufferWriter writer)
         {
@@ -37,7 +38,7 @@ public void Handle(ref NetworkContext context)
             }
 
             networkManager.NetworkMetrics.TrackObjectDestroyReceived(context.SenderId, networkObject, context.MessageSize);
-            networkManager.SpawnManager.OnDespawnObject(networkObject, true);
+            networkManager.SpawnManager.OnDespawnObject(networkObject, DestroyGameObject);
         }
     }
 }

com.unity.netcode.gameobjects/Runtime/SceneManagement/ISceneManagerHandler.cs

@@ -15,8 +15,14 @@ struct SceneEventAction
         {
             internal uint SceneEventId;
             internal Action<uint> EventAction;
+            /// <summary>
+            /// Used server-side for integration testing in order to
+            /// invoke the SceneEventProgress once done loading
+            /// </summary>
+            internal Action Completed;
             internal void Invoke()
             {
+                Completed?.Invoke();
                 EventAction.Invoke(SceneEventId);
             }
         }