feat: Nested NetworkBehaviour Synchronization [MTT-5024] (#2298)

* feat 
This includes the changes to provide users with a way to synchronize NetworkBehaviours with custom data prior to their associated NetworkObject is spawned.

* fix and refactor
This updates NetworkTransform so that it will properly synchronize when placed on nested NetworkBehaviours (i.e. their GameObject has no NetworkObject component).
This also includes some fixes that allows for NetworkObjects to fail, NetworkVariables to fail, and NetworkBehaviour synchronization to fail without impacting the rest of the synchronization process.
Minor fix for in-scene placed parenting under a non-NetworkObject to prevent from being added to the orphaned child list.

* test
Renamed NetworkObjectSceneSerializationTests to NetworkObjectSynchronizationTests.
Added a more robust/wider range of tests, also added running host or server as well as added a basic OnSynchronize test.
Added integration test to validate nested network transforms synchronize properly with nested GameObjects' transforms.
Fixed an issue with late joined clients not registering all players in NetcodeIntegrationTest.

* test manual
This include a nested NetworkTransform manual test to visually validate the nested NetworkTransform update.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -9,17 +9,23 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ## [Unreleased]
 
 ### Added
+
+- Added protected method `NetworkBehaviour.OnSynchronize` which is invoked during the initial `NetworkObject` synchronization process. This provides users the ability to include custom serialization information that will be applied to the `NetworkBehaviour` prior to the `NetworkObject` being spawned. (#2298)
 - Added support for different versions of the SDK to talk to each other in circumstances where changes permit it. Starting with this version and into future versions, patch versions should be compatible as long as the minor version is the same. (#2290)
 - Added `NetworkObject` auto-add helper and Multiplayer Tools install reminder settings to Project Settings. (#2285)
-- Added `public string DisconnectReason` getter to `NetworkManager` and `string Reason` to `ConnectionApprovalResponse`. Allows connection approval to communicate back a reason. Also added `public void DisconnectClient(ulong clientId, string reason)` allowing setting a disconnection reason, when explicitly disconnecting a client. 
+- Added `public string DisconnectReason` getter to `NetworkManager` and `string Reason` to `ConnectionApprovalResponse`. Allows connection approval to communicate back a reason. Also added `public void DisconnectClient(ulong clientId, string reason)` allowing setting a disconnection reason, when explicitly disconnecting a client. (#2280)
 
 ### Changed
 
 - Changed 3rd-party `XXHash` (32 & 64) implementation with an in-house reimplementation (#2310)
+- When `NetworkConfig.EnsureNetworkVariableLengthSafety` is disabled `NetworkVariable` fields do not write the additional `ushort` size value (_which helps to reduce the total synchronization message size_), but when enabled it still writes the additional `ushort` value. (#2298)
 - Optimized bandwidth usage by encoding most integer fields using variable-length encoding. (#2276)
 
 ### Fixed
 
+- Fixed issue where `NetworkTransform` components nested under a parent with a `NetworkObject` component  (i.e. network prefab) would not have their associated `GameObject`'s transform synchronized. (#2298)
+- Fixed issue where `NetworkObject`s that failed to instantiate could cause the entire synchronization pipeline to be disrupted/halted for a connecting client. (#2298)
+- Fixed issue where in-scene placed `NetworkObject`s nested under a `GameObject` would be added to the orphaned children list causing continual console warning log messages. (#2298)
 - Custom messages are now properly received by the local client when they're sent while running in host mode. (#2296)
 - Fixed issue where the host would receive more than one event completed notification when loading or unloading a scene only when no clients were connected. (#2292)
 - Fixed an issue in `UnityTransport` where an error would be logged if the 'Use Encryption' flag was enabled with a Relay configuration that used a secure protocol. (#2289)

com.unity.netcode.gameobjects/Components/NetworkTransform.cs

@@ -451,6 +451,33 @@ internal NetworkTransformState GetLastSentState()
             return m_LastSentState;
         }
 
+        /// <summary>
+        /// This is invoked when a new client joins (server and client sides)
+        /// Server Side: Serializes as if we were teleporting (everything is sent via NetworkTransformState)
+        /// Client Side: Adds the interpolated state which applies the NetworkTransformState as well
+        /// </summary>
+        protected override void OnSynchronize<T>(ref BufferSerializer<T> serializer)
+        {
+            // We don't need to synchronize NetworkTransforms that are on the same
+            // GameObject as the NetworkObject.
+            if (NetworkObject.gameObject == gameObject)
+            {
+                return;
+            }
+            var synchronizationState = new NetworkTransformState();
+            if (serializer.IsWriter)
+            {
+                synchronizationState.IsTeleportingNextFrame = true;
+                ApplyTransformToNetworkStateWithInfo(ref synchronizationState, m_CachedNetworkManager.LocalTime.Time, transform);
+                synchronizationState.NetworkSerialize(serializer);
+            }
+            else
+            {
+                synchronizationState.NetworkSerialize(serializer);
+                AddInterpolatedState(synchronizationState);
+            }
+        }
+
         /// <summary>
         /// This will try to send/commit the current transform delta states (if any)
         /// </summary>

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

@@ -763,6 +763,14 @@ internal void MarkVariablesDirty(bool dirty)
             }
         }
 
+        /// <summary>
+        /// Synchronizes by setting only the NetworkVariable field values that the client has permission to read.
+        /// Note: This is only invoked when first synchronizing a NetworkBehaviour (i.e. late join or spawned NetworkObject)
+        /// </summary>
+        /// <remarks>
+        /// When NetworkConfig.EnsureNetworkVariableLengthSafety is enabled each NetworkVariable field will be preceded
+        /// by the number of bytes written for that specific field.
+        /// </remarks>
         internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClientId)
         {
             if (NetworkVariableFields.Count == 0)
@@ -772,32 +780,47 @@ internal void WriteNetworkVariableData(FastBufferWriter writer, ulong targetClie
 
             for (int j = 0; j < NetworkVariableFields.Count; j++)
             {
-                bool canClientRead = NetworkVariableFields[j].CanClientRead(targetClientId);
 
-                if (canClientRead)
+                if (NetworkVariableFields[j].CanClientRead(targetClientId))
                 {
-                    var writePos = writer.Position;
-                    // Note: This value can't be packed because we don't know how large it will be in advance
-                    // we reserve space for it, then write the data, then come back and fill in the space
-                    // to pack here, we'd have to write data to a temporary buffer and copy it in - which
-                    // isn't worth possibly saving one byte if and only if the data is less than 63 bytes long...
-                    // The way we do packing, any value > 63 in a ushort will use the full 2 bytes to represent.
-                    writer.WriteValueSafe((ushort)0);
-                    var startPos = writer.Position;
-                    NetworkVariableFields[j].WriteField(writer);
-                    var size = writer.Position - startPos;
-                    writer.Seek(writePos);
-                    writer.WriteValueSafe((ushort)size);
-                    writer.Seek(startPos + size);
+                    if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
+                    {
+                        var writePos = writer.Position;
+                        // Note: This value can't be packed because we don't know how large it will be in advance
+                        // we reserve space for it, then write the data, then come back and fill in the space
+                        // to pack here, we'd have to write data to a temporary buffer and copy it in - which
+                        // isn't worth possibly saving one byte if and only if the data is less than 63 bytes long...
+                        // The way we do packing, any value > 63 in a ushort will use the full 2 bytes to represent.
+                        writer.WriteValueSafe((ushort)0);
+                        var startPos = writer.Position;
+                        NetworkVariableFields[j].WriteField(writer);
+                        var size = writer.Position - startPos;
+                        writer.Seek(writePos);
+                        writer.WriteValueSafe((ushort)size);
+                        writer.Seek(startPos + size);
+                    }
+                    else
+                    {
+                        NetworkVariableFields[j].WriteField(writer);
+                    }
                 }
-                else
+                else // Only if EnsureNetworkVariableLengthSafety, otherwise just skip
+                if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
                 {
                     writer.WriteValueSafe((ushort)0);
                 }
             }
         }
 
-        internal void SetNetworkVariableData(FastBufferReader reader)
+        /// <summary>
+        /// Synchronizes by setting only the NetworkVariable field values that the client has permission to read.
+        /// Note: This is only invoked when first synchronizing a NetworkBehaviour (i.e. late join or spawned NetworkObject)
+        /// </summary>
+        /// <remarks>
+        /// When NetworkConfig.EnsureNetworkVariableLengthSafety is enabled each NetworkVariable field will be preceded
+        /// by the number of bytes written for that specific field.
+        /// </remarks>
+        internal void SetNetworkVariableData(FastBufferReader reader, ulong clientId)
         {
             if (NetworkVariableFields.Count == 0)
             {
@@ -806,13 +829,23 @@ internal void SetNetworkVariableData(FastBufferReader reader)
 
             for (int j = 0; j < NetworkVariableFields.Count; j++)
             {
-                reader.ReadValueSafe(out ushort varSize);
-                if (varSize == 0)
+                var varSize = (ushort)0;
+                var readStartPos = 0;
+                if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
+                {
+                    reader.ReadValueSafe(out varSize);
+                    if (varSize == 0)
+                    {
+                        continue;
+                    }
+                    readStartPos = reader.Position;
+                }
+                else // If the client cannot read this field, then skip it
+                if (!NetworkVariableFields[j].CanClientRead(clientId))
                 {
                     continue;
                 }
 
-                var readStartPos = reader.Position;
                 NetworkVariableFields[j].ReadField(reader);
 
                 if (NetworkManager.NetworkConfig.EnsureNetworkVariableLengthSafety)
@@ -849,6 +882,138 @@ protected NetworkObject GetNetworkObject(ulong networkId)
             return NetworkManager.SpawnManager.SpawnedObjects.TryGetValue(networkId, out NetworkObject networkObject) ? networkObject : null;
         }
 
+        /// <summary>
+        /// Override this method if your derived NetworkBehaviour requires custom synchronization data.
+        /// Note: Use of this method is only for the initial client synchronization of NetworkBehaviours
+        /// and will increase the payload size for client synchronization and dynamically spawned
+        /// <see cref="NetworkObject"/>s.
+        /// </summary>
+        /// <remarks>
+        /// When serializing (writing) this will be invoked during the client synchronization period and
+        /// when spawning new NetworkObjects.
+        /// When deserializing (reading), this will be invoked prior to the NetworkBehaviour's associated
+        /// NetworkObject being spawned.
+        /// </remarks>
+        /// <param name="serializer">The serializer to use to read and write the data.</param>
+        /// <typeparam name="T">
+        /// Either BufferSerializerReader or BufferSerializerWriter, depending whether the serializer
+        /// is in read mode or write mode.
+        /// </typeparam>
+        protected virtual void OnSynchronize<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter
+        {
+
+        }
+
+        /// <summary>
+        /// Internal method that determines if a NetworkBehaviour has additional synchronization data to
+        /// be synchronized when first instantiated prior to its associated NetworkObject being spawned.
+        /// </summary>
+        /// <remarks>
+        /// This includes try-catch blocks to recover from exceptions that might occur and continue to
+        /// synchronize any remaining NetworkBehaviours.
+        /// </remarks>
+        /// <returns>true if it wrote synchronization data and false if it did not</returns>
+        internal bool Synchronize<T>(ref BufferSerializer<T> serializer) where T : IReaderWriter
+        {
+            if (serializer.IsWriter)
+            {
+                // Get the writer to handle seeking and determining how many bytes were written
+                var writer = serializer.GetFastBufferWriter();
+                // Save our position before we attempt to write anything so we can seek back to it (i.e. error occurs)
+                var positionBeforeWrite = writer.Position;
+                writer.WriteValueSafe(NetworkBehaviourId);
+
+                // Save our position where we will write the final size being written so we can skip over it in the
+                // event an exception occurs when deserializing.
+                var sizePosition = writer.Position;
+                writer.WriteValueSafe((ushort)0);
+
+                // Save our position before synchronizing to determine how much was written
+                var positionBeforeSynchronize = writer.Position;
+                var threwException = false;
+                try
+                {
+                    OnSynchronize(ref serializer);
+                }
+                catch (Exception ex)
+                {
+                    threwException = true;
+                    if (NetworkManager.LogLevel <= LogLevel.Normal)
+                    {
+                        NetworkLog.LogWarning($"{name} threw an exception during synchronization serialization, this {nameof(NetworkBehaviour)} is being skipped and will not be synchronized!");
+                        if (NetworkManager.LogLevel == LogLevel.Developer)
+                        {
+                            NetworkLog.LogError($"{ex.Message}\n {ex.StackTrace}");
+                        }
+                    }
+                }
+                var finalPosition = writer.Position;
+
+                // If we wrote nothing then skip writing anything for this NetworkBehaviour
+                if (finalPosition == positionBeforeSynchronize || threwException)
+                {
+                    writer.Seek(positionBeforeWrite);
+                    return false;
+                }
+                else
+                {
+                    // Write the number of bytes serialized to handle exceptions on the deserialization side
+                    var bytesWritten = finalPosition - positionBeforeSynchronize;
+                    writer.Seek(sizePosition);
+                    writer.WriteValueSafe((ushort)bytesWritten);
+                    writer.Seek(finalPosition);
+                }
+                return true;
+            }
+            else
+            {
+                var reader = serializer.GetFastBufferReader();
+                // We will always read the expected byte count
+                reader.ReadValueSafe(out ushort expectedBytesToRead);
+
+                // Save our position before we begin synchronization deserialization
+                var positionBeforeSynchronize = reader.Position;
+                var synchronizationError = false;
+                try
+                {
+                    // Invoke synchronization
+                    OnSynchronize(ref serializer);
+                }
+                catch (Exception ex)
+                {
+                    if (NetworkManager.LogLevel <= LogLevel.Normal)
+                    {
+                        NetworkLog.LogWarning($"{name} threw an exception during synchronization deserialization, this {nameof(NetworkBehaviour)} is being skipped and will not be synchronized!");
+                        if (NetworkManager.LogLevel == LogLevel.Developer)
+                        {
+                            NetworkLog.LogError($"{ex.Message}\n {ex.StackTrace}");
+                        }
+                    }
+                    synchronizationError = true;
+                }
+
+                var totalBytesRead = reader.Position - positionBeforeSynchronize;
+                if (totalBytesRead != expectedBytesToRead)
+                {
+                    if (NetworkManager.LogLevel <= LogLevel.Normal)
+                    {
+                        NetworkLog.LogWarning($"{name} read {totalBytesRead} bytes but was expected to read {expectedBytesToRead} bytes during synchronization deserialization! This {nameof(NetworkBehaviour)} is being skipped and will not be synchronized!");
+                    }
+                    synchronizationError = true;
+                }
+
+                // Skip over the entry if deserialization fails
+                if (synchronizationError)
+                {
+                    var skipToPosition = positionBeforeSynchronize + expectedBytesToRead;
+                    reader.Seek(skipToPosition);
+                    return false;
+                }
+                return true;
+            }
+        }
+
+
         /// <summary>
         /// Invoked when the <see cref="GameObject"/> the <see cref="NetworkBehaviour"/> is attached to.
         /// NOTE:  If you override this, you will want to always invoke this base class version of this