Merge branch 'develop-3.x.x' into fix/hybrid-prefab-component-removal

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -24,8 +24,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Issue where a client promoted to session owner could throw an exception when synchronizing a newly joining client. (#4214)
-- Issue where a client's clock kept alternating between running slightly fast and slightly slow on every frame, even after it had synchronized with the server. Interpolated objects juddered (a periodic motion artifact caused by mismatched frame rates): they moved in uneven steps instead of smoothly. Barely noticeable at the default `NetworkTimeSystem.AdjustmentRatio`, and clearly visible at higher values.
-- Issue where the IL post-processor could not resolve core library types, such as `Guid` and `List<T>`, in assemblies compiled against .NET reference assemblies, for example when targeting .NET 10. Those split the core library across `System.Runtime`, `System.Collections` and other assemblies.
+- Issue where a rejected parent change, for example one made by a client without parenting authority, restored the `NetworkObject`'s parent but left it at the moved position when the change was made with `worldPositionStays` disabled. The original position, rotation and scale are now restored. (#4202)
+- Issue where a client's clock kept alternating between running slightly fast and slightly slow on every frame, even after it had synchronized with the server. Interpolated objects juddered (a periodic motion artifact caused by mismatched frame rates): they moved in uneven steps instead of smoothly. Barely noticeable at the default `NetworkTimeSystem.AdjustmentRatio`, and clearly visible at higher values. (#4195)
+- Issue where the IL post-processor could not resolve core library types, such as `Guid` and `List<T>`, in assemblies compiled against .NET reference assemblies, for example when targeting .NET 10. Those split the core library across `System.Runtime`, `System.Collections` and other assemblies. (#4188)
 - Fixed issue where a prefab added to a `NetworkPrefabsList` while a session was running was registered more than once and logged a duplicate `GlobalObjectIdHash` error. (#4184)
 - Issue where moving a `NetworkObject` into another scene made the clients that did not observe it log "Trying to synchronize NetworkObjectId but it was not spawned". The scene migration is now only sent to the clients that observe the `NetworkObject`. (#4176)
 - Issue where a `NetworkObject` that was moved into another scene while hidden from a client spawned in that client's active scene when it was shown with `NetworkShow`, instead of the scene it is in on the server. (#4176)

com.unity.netcode.gameobjects/Runtime/Components/Helpers/NetworkObjectBridge.cs

@@ -27,12 +27,18 @@ private void OnValidate()
             {
                 return;
             }
-
+            
             var networkObject = GetComponent<NetworkObject>();
             if (networkObject == null)
             {
+                // Remove the bridge if there is no NetworkObject
                 EditorApplication.delayCall += () => DestroyImmediate(this, true);
             }
+            else
+            {
+                // NGO spawns the ghost when the NetworkObject spawns. This assures the ghost spawns in that NetworkManager's world.
+                ghostAdapter.DelaySpawning = true;
+            }
         }
 #endif
 

com.unity.netcode.gameobjects/Runtime/Configuration/NetworkPrefabs.cs

@@ -368,6 +368,7 @@ public bool Contains(NetworkPrefab prefab)
 #if UNIFIED_NETCODE
         internal const string DistributedAuthorityHybridPrefabError = "Distributed authority does not support hybrid prefabs.";
         internal const string HybridPrefabAfterStartError = "Hybrid prefabs must be registered before the NetworkManager starts.";
+        internal const string HybridSpawnWithoutHybridPrefabsError = "Hybrid prefab instances can only be spawned in a session that started with a hybrid prefab registered.";
 
         internal bool HasGhostPrefabs { get; private set; }
 

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

@@ -467,6 +467,79 @@ public void ApplyScale(Vector3 scale)
                 GhostObject.ApplyPostTransformMatrixScale(scale);
             }
         }
+
+        /// <summary>
+        /// Returns true when N4E replicates this instance's parent. NGO parenting is disabled for the instance in that case.
+        /// </summary>
+        internal bool GhostReplicatesParent()
+        {
+            return HasGhost && GhostObject.ParentReplication;
+        }
+
+        private bool WarnIfGhostReplicatesParent(string methodName)
+        {
+            if (!GhostReplicatesParent())
+            {
+                return false;
+            }
+            var networkManager = NetworkManager;
+            if (networkManager == null || networkManager.LogLevel <= LogLevel.Normal)
+            {
+                NetworkLog.LogWarning($"[{name}] {methodName} was ignored. {nameof(GhostObject)}.{nameof(GhostObject.ParentReplication)} is enabled, so " +
+                    $"Netcode for Entities replicates the parent of this hybrid instance. Use {nameof(Transform)}.{nameof(Transform.SetParent)} instead.");
+            }
+            return true;
+        }
+
+        /// <summary>
+        /// NGO parenting of a hybrid instance only supports worldPositionStays disabled.
+        /// </summary>
+        /// <remarks>
+        /// The ghost replicates its local transform. With worldPositionStays enabled, the authority rebases that local transform when it reparents.
+        /// Clients apply the ParentSyncMessage before the snapshot with the rebased values arrives, so the object pops.
+        /// </remarks>
+        /// <returns>true when the parent change is rejected</returns>
+        private bool RejectHybridWorldPositionStays(bool worldPositionStays)
+        {
+            if (!HasGhost || !worldPositionStays || GhostObject.ParentReplication)
+            {
+                return false;
+            }
+            var networkManager = NetworkManager;
+            if (networkManager == null || networkManager.LogLevel <= LogLevel.Error)
+            {
+                NetworkLog.LogError($"[{name}] Hybrid instances can only be parented with worldPositionStays disabled. Use {nameof(TrySetParent)} " +
+                    $"or {nameof(TryRemoveParent)} with worldPositionStays set to false, or enable {nameof(GhostObject)}.{nameof(GhostObject.ParentReplication)}.");
+            }
+            return true;
+        }
+
+        // Same tolerances as GhostObject. They are above the error a SetParent round trip leaves behind.
+        private const float k_LocalSpaceEpsilon = 1e-4f;
+        private const float k_LocalSpaceAngleEpsilon = 0.01f;
+
+        /// <summary>
+        /// Returns true when parenting occurred with world position stays set to false.
+        /// </summary>
+        private bool LocalTransformKeptByParentChange()
+        {
+            var currentTransform = transform;
+            return Vector3.Distance(currentTransform.localPosition, m_LocalPositionBeforeParentChange) <= k_LocalSpaceEpsilon
+                && Quaternion.Angle(currentTransform.localRotation, m_LocalRotationBeforeParentChange) <= k_LocalSpaceAngleEpsilon
+                && Vector3.Distance(currentTransform.localScale, m_LocalScaleBeforeParentChange) <= k_LocalSpaceEpsilon;
+        }
+
+        /// <summary>
+        /// Logs a warning when a hybrid instance is parented under a non-hybrid one or vice versa.
+        /// </summary>
+        private void LogHybridParentingArtifacts(NetworkObject parentObject)
+        {
+            if (parentObject != null && parentObject.HasGhost != HasGhost)
+            {
+                NetworkLog.LogWarning($"[{name}] Parented under {parentObject.name}, but only one of them is a hybrid instance. Their transforms are " +
+                    $"synchronized and interpolated on different timelines, so the child can jitter relative to its parent while either one moves.");
+            }
+        }
 #endif
         /// <summary>
         /// This is intentionally private since this is a sealed class.
@@ -2512,6 +2585,49 @@ internal void InvokeBehaviourOnNetworkObjectParentChanged(NetworkObject parentNe
         private Transform m_CachedParent; // What is our last set parent Transform reference?
         private bool m_CachedWorldPositionStays = true; // Used to preserve the world position stays parameter passed in TrySetParent
 
+        // The local transform's state before a parent change. These values are used if the parent change is reverted.
+        private Vector3 m_LocalPositionBeforeParentChange;
+        private Quaternion m_LocalRotationBeforeParentChange;
+        private Vector3 m_LocalScaleBeforeParentChange;
+        private bool m_RevertingParentChange;
+
+        private void OnBeforeTransformParentChanged()
+        {
+            // Skips the parent change RevertParentChange makes, so the values it restores are not overwritten.
+            if (m_RevertingParentChange)
+            {
+                return;
+            }
+            var currentTransform = transform;
+            m_LocalPositionBeforeParentChange = currentTransform.localPosition;
+            m_LocalRotationBeforeParentChange = currentTransform.localRotation;
+            m_LocalScaleBeforeParentChange = currentTransform.localScale;
+        }
+
+        /// <summary>
+        /// Restores the cached parent and the local transform prior to the parent change.
+        /// </summary>
+        /// <remarks>
+        /// Setting <see cref="Transform.parent"/> keeps the world position. That only restores the original pose when the change also kept the world position.
+        /// Restoring the local transform under the cached parent works for either worldPositionStays value.
+        /// </remarks>
+        private void RevertParentChange()
+        {
+            m_RevertingParentChange = true;
+            try
+            {
+                var currentTransform = transform;
+                currentTransform.SetParent(m_CachedParent, false);
+                currentTransform.localPosition = m_LocalPositionBeforeParentChange;
+                currentTransform.localRotation = m_LocalRotationBeforeParentChange;
+                currentTransform.localScale = m_LocalScaleBeforeParentChange;
+            }
+            finally
+            {
+                m_RevertingParentChange = false;
+            }
+        }
+
         /// <summary>
         /// With distributed authority, we need to have a way to determine if the parenting action is authorized.
         /// This is set when handling an incoming ParentSyncMessage and when running as a DAHost and a client has disconnected.
@@ -2567,6 +2683,12 @@ internal void ClearNetworkParenting()
         /// <returns>Whether or not re-parenting was successful.</returns>
         public bool TrySetParent(Transform parent, bool worldPositionStays = true)
         {
+#if UNIFIED_NETCODE
+            if (WarnIfGhostReplicatesParent(nameof(TrySetParent)))
+            {
+                return false;
+            }
+#endif
             // If we are removing ourself from a parent
             if (parent == null)
             {
@@ -2587,6 +2709,12 @@ public bool TrySetParent(Transform parent, bool worldPositionStays = true)
         /// <returns>Whether or not re-parenting was successful.</returns>
         public bool TrySetParent(GameObject parent, bool worldPositionStays = true)
         {
+#if UNIFIED_NETCODE
+            if (WarnIfGhostReplicatesParent(nameof(TrySetParent)))
+            {
+                return false;
+            }
+#endif
             // If we are removing ourself from a parent
             if (parent == null)
             {
@@ -2617,6 +2745,12 @@ internal bool TryRemoveParentCachedWorldPositionStays()
         /// <returns>True if the parent was successfully removed, false if the operation failed or the object was already parentless</returns>
         public bool TryRemoveParent(bool worldPositionStays = true)
         {
+#if UNIFIED_NETCODE
+            if (WarnIfGhostReplicatesParent(nameof(TryRemoveParent)))
+            {
+                return false;
+            }
+#endif
             return TrySetParent((NetworkObject)null, worldPositionStays);
         }
 
@@ -2628,6 +2762,12 @@ public bool TryRemoveParent(bool worldPositionStays = true)
         /// <returns>Whether or not reparenting was successful.</returns>
         public bool TrySetParent(NetworkObject parent, bool worldPositionStays = true)
         {
+#if UNIFIED_NETCODE
+            if (WarnIfGhostReplicatesParent(nameof(TrySetParent)) || RejectHybridWorldPositionStays(worldPositionStays))
+            {
+                return false;
+            }
+#endif
             if (!AutoObjectParentSync || !IsSpawned || !NetworkManagerOwner.IsListening)
             {
                 return false;
@@ -2669,19 +2809,27 @@ internal bool InternalTrySetParent(NetworkObject parent, bool worldPositionStays
         private void OnTransformParentChanged()
         {
             var networkManager = NetworkManager;
-            if (!AutoObjectParentSync || (networkManager != null && networkManager.ShutdownInProgress))
+            if (m_RevertingParentChange || !AutoObjectParentSync || (networkManager != null && networkManager.ShutdownInProgress))
             {
                 return;
             }
 
+#if UNIFIED_NETCODE
+            // No warning: this also runs for the parent changes N4E applies.
+            if (GhostReplicatesParent())
+            {
+                return;
+            }
+#endif
+
             if (transform.parent == m_CachedParent)
             {
                 return;
             }
 
             if (networkManager == null || !networkManager.IsListening)
             {
-                transform.parent = m_CachedParent;
+                RevertParentChange();
                 // We want to log at any LogLevel, since we may not have a network manager may here.
                 NetworkLog.LogError($"[{name}] {nameof(networkManager)} is not listening, start a server or host before re-parenting.");
                 return;
@@ -2699,7 +2847,7 @@ private void OnTransformParentChanged()
                 }
                 else
                 {
-                    transform.parent = m_CachedParent;
+                    RevertParentChange();
                     if (networkManager.LogLevel <= LogLevel.Error)
                     {
                         NetworkLog.LogErrorServer($"[{name}] {nameof(NetworkObject)} can only be re-parented after being spawned!");
@@ -2714,7 +2862,7 @@ private void OnTransformParentChanged()
             // If we are spawned and don't have authority; reset the parent back to the cached parent and exit
             if (!isParentingAuthority)
             {
-                transform.parent = m_CachedParent;
+                RevertParentChange();
                 if (networkManager.LogLevel <= LogLevel.Error)
                 {
                     if (networkManager.DistributedAuthorityMode)
@@ -2729,13 +2877,29 @@ private void OnTransformParentChanged()
                 return;
             }
 
+#if UNIFIED_NETCODE
+            // A direct Transform parent change has no worldPositionStays value. It is derived from whether the local transform was kept.
+            if (HasGhost && !AuthorityAppliedParenting && !GhostObject.ParentReplication)
+            {
+                if (LocalTransformKeptByParentChange())
+                {
+                    m_CachedWorldPositionStays = false;
+                }
+                else if (RejectHybridWorldPositionStays(true))
+                {
+                    RevertParentChange();
+                    return;
+                }
+            }
+#endif
+
             var removeParent = false;
             var parentTransform = transform.parent;
             if (parentTransform != null)
             {
                 if (!transform.parent.TryGetComponent(out NetworkObject parentObject))
                 {
-                    transform.parent = m_CachedParent;
+                    RevertParentChange();
                     AuthorityAppliedParenting = false;
                     if (networkManager.LogLevel <= LogLevel.Error)
                     {
@@ -2745,7 +2909,7 @@ private void OnTransformParentChanged()
                 }
                 if (!parentObject.IsSpawned)
                 {
-                    transform.parent = m_CachedParent;
+                    RevertParentChange();
                     AuthorityAppliedParenting = false;
                     if (networkManager.LogLevel <= LogLevel.Error)
                     {
@@ -2755,6 +2919,12 @@ private void OnTransformParentChanged()
                 }
 
                 m_LatestParent = parentObject.NetworkObjectId;
+#if UNIFIED_NETCODE
+                if (networkManager.LogLevel <= LogLevel.Developer)
+                {
+                    LogHybridParentingArtifacts(parentObject);
+                }
+#endif
             }
             else
             {
@@ -2835,6 +3005,14 @@ internal bool ApplyNetworkParenting(bool removeParent = false, bool ignoreNotSpa
                 return false;
             }
 
+#if UNIFIED_NETCODE
+            // NGO does not track this parent. Applying "no parent" here would undo the parent N4E replicated.
+            if (GhostReplicatesParent())
+            {
+                return false;
+            }
+#endif
+
             // SPECIAL CASE:
             // The ignoreNotSpawned is a special case scenario where a late joining client has joined
             // and loaded one or more scenes that contain nested in-scene placed NetworkObject children
@@ -3626,7 +3804,12 @@ internal SerializedObject SerializeSpawnedObject(ulong targetClientId = NetworkM
         {
             var obj = new SerializedObject
             {
+#if UNIFIED_NETCODE
+                // The ghost's spawn carries its own parent.
+                HasParent = transform.parent != null && !GhostReplicatesParent(),
+#else
                 HasParent = transform.parent != null,
+#endif
                 WorldPositionStays = m_CachedWorldPositionStays,
                 NetworkObjectId = NetworkObjectId,
                 OwnerClientId = OwnerClientId,