fix: NetworkRigidbody authority and ownership detection (#1891)

* fix

MTT-3374
Initial changes for this fix.

* fix

Migrating the authority mode check to NetworkTransform since this is what dictates authority anyways.

* style

updated and added comments

* test

Adding ownership tests

* update

Realized we could further simplify NetworkRigidbody and added additional testing against owners not being Kinematic and non-owners being kinematic.
Also had to have the NetworkManager relative test prefab instances ignore each others' colliders so they wouldn't skew the position testing.

* style

whitespace fixes

* style

parameter name change

* style

better clarity in a comment

* update

This fixes an issue with not being able to find ClientNetworkTransform and makes the IsServerAuhtoritative internal.

* update

realized we don't need to save the original Kinematic state.
updated a comment

* update

needed to add COM_UNITY_MODULES_PHYSICS define in runtime and exclude the test if the physics module is not included.

* style

removing 2 CR/LF

* update

didn't need all of the changes made to the asmdef file

* style

removing xml doc API reference

* Update com.unity.netcode.gameobjects/Tests/Runtime/com.unity.netcode.runtimetests.asmdef

Co-authored-by: Fatih Mar <mfatihmar@gmail.com>

* Update com.unity.netcode.runtimetests.asmdef

* fix

Had to adjust the rigid body interpolation setting to be based on authority and not NetworkTransform.Interpolate (bug from this PR)
Had to re-adjust the original rigid body test to match the NetworkRigidbody change where when it is despawned physics is not applied.

* fix

Fixing edge case scenario with object parenting test.

Co-authored-by: Fatih Mar <mfatihmar@gmail.com>

Author: NoelStephensUnity

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

@@ -5,77 +5,97 @@ namespace Unity.Netcode.Components
 {
     /// <summary>
     /// NetworkRigidbody allows for the use of <see cref="Rigidbody"/> on network objects. By controlling the kinematic
-    /// mode of the rigidbody and disabling it on all peers but the authoritative one.
+    /// mode of the <see cref="Rigidbody"/> and disabling it on all peers but the authoritative one.
     /// </summary>
     [RequireComponent(typeof(Rigidbody))]
     [RequireComponent(typeof(NetworkTransform))]
     public class NetworkRigidbody : NetworkBehaviour
     {
+        /// <summary>
+        /// Determines if we are server (true) or owner (false) authoritative
+        /// </summary>
+        private bool m_IsServerAuthoritative;
+
         private Rigidbody m_Rigidbody;
         private NetworkTransform m_NetworkTransform;
-
-        private bool m_OriginalKinematic;
         private RigidbodyInterpolation m_OriginalInterpolation;
 
-        // Used to cache the authority state of this rigidbody during the last frame
+        // Used to cache the authority state of this Rigidbody during the last frame
         private bool m_IsAuthority;
 
-        /// <summary>
-        /// Gets a bool value indicating whether this <see cref="NetworkRigidbody"/> on this peer currently holds authority.
-        /// </summary>
-        private bool HasAuthority => m_NetworkTransform.CanCommitToTransform;
-
         private void Awake()
         {
-            m_Rigidbody = GetComponent<Rigidbody>();
             m_NetworkTransform = GetComponent<NetworkTransform>();
+            m_IsServerAuthoritative = m_NetworkTransform.IsServerAuthoritative();
+
+            m_Rigidbody = GetComponent<Rigidbody>();
+            m_OriginalInterpolation = m_Rigidbody.interpolation;
+
+            // Set interpolation to none if NetworkTransform is handling interpolation, otherwise it sets it to the original value
+            m_Rigidbody.interpolation = m_NetworkTransform.Interpolate ? RigidbodyInterpolation.None : m_OriginalInterpolation;
+
+            // Turn off physics for the rigid body until spawned, otherwise
+            // clients can run fixed update before the first full
+            // NetworkTransform update
+            m_Rigidbody.isKinematic = true;
+        }
+
+        /// <summary>
+        /// For owner authoritative (i.e. ClientNetworkTransform)
+        /// we adjust our authority when we gain ownership
+        /// </summary>
+        public override void OnGainedOwnership()
+        {
+            UpdateOwnershipAuthority();
         }
 
-        private void FixedUpdate()
+        /// <summary>
+        /// For owner authoritative(i.e. ClientNetworkTransform)
+        /// we adjust our authority when we have lost ownership
+        /// </summary>
+        public override void OnLostOwnership()
         {
-            if (NetworkManager.IsListening)
-            {
-                if (HasAuthority != m_IsAuthority)
-                {
-                    m_IsAuthority = HasAuthority;
-                    UpdateRigidbodyKinematicMode();
-                }
-            }
+            UpdateOwnershipAuthority();
         }
 
-        // Puts the rigidbody in a kinematic non-interpolated mode on everyone but the server.
-        private void UpdateRigidbodyKinematicMode()
+        /// <summary>
+        /// Sets the authority differently depending upon
+        /// whether it is server or owner authoritative
+        /// </summary>
+        private void UpdateOwnershipAuthority()
         {
-            if (m_IsAuthority == false)
+            if (m_IsServerAuthoritative)
             {
-                m_OriginalKinematic = m_Rigidbody.isKinematic;
-                m_Rigidbody.isKinematic = true;
-
-                m_OriginalInterpolation = m_Rigidbody.interpolation;
-                // Set interpolation to none, the NetworkTransform component interpolates the position of the object.
-                m_Rigidbody.interpolation = RigidbodyInterpolation.None;
+                m_IsAuthority = NetworkManager.IsServer;
             }
             else
             {
-                // Resets the rigidbody back to it's non replication only state. Happens on shutdown and when authority is lost
-                m_Rigidbody.isKinematic = m_OriginalKinematic;
-                m_Rigidbody.interpolation = m_OriginalInterpolation;
+                m_IsAuthority = IsOwner;
             }
+
+            // If you have authority then you are not kinematic
+            m_Rigidbody.isKinematic = !m_IsAuthority;
+
+            // Set interpolation of the Rigidbody based on authority
+            // With authority: let local transform handle interpolation
+            // Without authority: let the NetworkTransform handle interpolation
+            m_Rigidbody.interpolation = m_IsAuthority ? m_OriginalInterpolation : RigidbodyInterpolation.None;
         }
 
         /// <inheritdoc />
         public override void OnNetworkSpawn()
         {
-            m_IsAuthority = HasAuthority;
-            m_OriginalKinematic = m_Rigidbody.isKinematic;
-            m_OriginalInterpolation = m_Rigidbody.interpolation;
-            UpdateRigidbodyKinematicMode();
+            UpdateOwnershipAuthority();
         }
 
         /// <inheritdoc />
         public override void OnNetworkDespawn()
         {
-            UpdateRigidbodyKinematicMode();
+            m_Rigidbody.interpolation = m_OriginalInterpolation;
+            // Turn off physics for the rigid body until spawned, otherwise
+            // non-owners can run fixed updates before the first full
+            // NetworkTransform update and physics will be applied (i.e. gravity, etc)
+            m_Rigidbody.isKinematic = true;
         }
     }
 }

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

@@ -960,5 +960,22 @@ public void Teleport(Vector3 newPosition, Quaternion newRotation, Vector3 newSca
             TryCommitValuesToServer(newPosition, newRotationEuler, newScale, m_CachedNetworkManager.LocalTime.Time);
             m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame = false;
         }
+
+        /// <summary>
+        /// Override this and return false to follow the owner authoritative
+        /// Otherwise, it defaults to server authoritative
+        /// </summary>
+        protected virtual bool OnIsServerAuthoritatitive()
+        {
+            return true;
+        }
+
+        /// <summary>
+        /// Used by <see cref="NetworkRigidbody"/> to determines if this is server or owner authoritative.
+        /// </summary>
+        internal bool IsServerAuthoritative()
+        {
+            return OnIsServerAuthoritatitive();
+        }
     }
 }

com.unity.netcode.gameobjects/Samples/ClientNetworkTransform/Scripts/ClientNetworkTransform.cs

@@ -36,5 +36,10 @@ protected override void Update()
                 }
             }
         }
+
+        protected override bool OnIsServerAuthoritatitive()
+        {
+            return false;
+        }
     }
 }