From d90292da99f1476aa80dd768e1ca2b182b5ec3c7 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Fri, 5 Apr 2024 16:06:01 -0500 Subject: [PATCH] chore: migrate 2830 allow null references in NetworkBehaviourReference and NetworkObjectReference (#2874) * initial commit * update adding changelog entry * update and style Fixing some style issues along with property type issues. * test Adding some generalized testing to validate both NetworkBehaviourReference and NetworkObjectReference can be created using a null and serialized when null. * update Updating the changed description. --------- Co-authored-by: Simone Guggiari --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../NetworkBehaviourReference.cs | 9 ++- .../Serialization/NetworkObjectReference.cs | 21 ++++-- .../NetworkBehaviourReferenceTests.cs | 48 +++++++++--- .../NetworkObjectReferenceTests.cs | 74 ++++++++++++++----- 5 files changed, 119 insertions(+), 34 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 0b27befc3..6ba2d1734 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -32,6 +32,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +- Changed `NetworkObjectReference` and `NetworkBehaviourReference` to allow null references when constructing and serializing. (#2874) - Changed `NetworkAnimator` no longer requires the `Animator` component to exist on the same `GameObject`. (#2872) - Changed `NetworkTransform` to now use `NetworkTransformMessage` as opposed to named messages for NetworkTransformState updates. (#2810) - Changed `CustomMessageManager` so it no longer attempts to register or "unregister" a null or empty string and will log an error if this condition occurs. (#2807) diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkBehaviourReference.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkBehaviourReference.cs index 6df91b8f5..4bc978cd3 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkBehaviourReference.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkBehaviourReference.cs @@ -11,6 +11,7 @@ namespace Unity.Netcode { private NetworkObjectReference m_NetworkObjectReference; private ushort m_NetworkBehaviourId; + private static ushort s_NullId = ushort.MaxValue; /// /// Creates a new instance of the struct. @@ -21,7 +22,9 @@ namespace Unity.Netcode { if (networkBehaviour == null) { - throw new ArgumentNullException(nameof(networkBehaviour)); + m_NetworkObjectReference = new NetworkObjectReference((NetworkObject)null); + m_NetworkBehaviourId = s_NullId; + return; } if (networkBehaviour.NetworkObject == null) { @@ -60,6 +63,10 @@ namespace Unity.Netcode [MethodImpl(MethodImplOptions.AggressiveInlining)] private static NetworkBehaviour GetInternal(NetworkBehaviourReference networkBehaviourRef, NetworkManager networkManager = null) { + if (networkBehaviourRef.m_NetworkBehaviourId == s_NullId) + { + return null; + } if (networkBehaviourRef.m_NetworkObjectReference.TryGet(out NetworkObject networkObject, networkManager)) { return networkObject.GetNetworkBehaviourAtOrderIndex(networkBehaviourRef.m_NetworkBehaviourId); diff --git a/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkObjectReference.cs b/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkObjectReference.cs index dc910440e..c2f1ba46f 100644 --- a/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkObjectReference.cs +++ b/com.unity.netcode.gameobjects/Runtime/Serialization/NetworkObjectReference.cs @@ -10,6 +10,7 @@ namespace Unity.Netcode public struct NetworkObjectReference : INetworkSerializable, IEquatable { private ulong m_NetworkObjectId; + private static ulong s_NullId = ulong.MaxValue; /// /// The of the referenced . @@ -24,13 +25,13 @@ namespace Unity.Netcode /// Creates a new instance of the struct. /// /// The to reference. - /// /// public NetworkObjectReference(NetworkObject networkObject) { if (networkObject == null) { - throw new ArgumentNullException(nameof(networkObject)); + m_NetworkObjectId = s_NullId; + return; } if (networkObject.IsSpawned == false) @@ -45,16 +46,20 @@ namespace Unity.Netcode /// Creates a new instance of the struct. /// /// The GameObject from which the component will be referenced. - /// /// public NetworkObjectReference(GameObject gameObject) { if (gameObject == null) { - throw new ArgumentNullException(nameof(gameObject)); + m_NetworkObjectId = s_NullId; + return; } - var networkObject = gameObject.GetComponent() ?? throw new ArgumentException($"Cannot create {nameof(NetworkObjectReference)} from {nameof(GameObject)} without a {nameof(NetworkObject)} component."); + var networkObject = gameObject.GetComponent(); + if (!networkObject) + { + throw new ArgumentException($"Cannot create {nameof(NetworkObjectReference)} from {nameof(GameObject)} without a {nameof(NetworkObject)} component."); + } if (networkObject.IsSpawned == false) { throw new ArgumentException($"{nameof(NetworkObjectReference)} can only be created from spawned {nameof(NetworkObject)}s."); @@ -80,10 +85,14 @@ namespace Unity.Netcode /// /// The reference. /// The networkmanager. Uses to resolve if null. - /// The resolves . Returns null if the networkobject was not found + /// The resolved . Returns null if the networkobject was not found [MethodImpl(MethodImplOptions.AggressiveInlining)] private static NetworkObject Resolve(NetworkObjectReference networkObjectRef, NetworkManager networkManager = null) { + if (networkObjectRef.m_NetworkObjectId == s_NullId) + { + return null; + } networkManager = networkManager ?? NetworkManager.Singleton; networkManager.SpawnManager.SpawnedObjects.TryGetValue(networkObjectRef.m_NetworkObjectId, out NetworkObject networkObject); diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkBehaviourReferenceTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkBehaviourReferenceTests.cs index 472d6176c..8cf3edcae 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkBehaviourReferenceTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkBehaviourReferenceTests.cs @@ -17,6 +17,8 @@ namespace Unity.Netcode.RuntimeTests { private class TestNetworkBehaviour : NetworkBehaviour { + public static bool ReceivedRPC; + public NetworkVariable TestVariable = new NetworkVariable(); public TestNetworkBehaviour RpcReceivedBehaviour; @@ -25,6 +27,7 @@ namespace Unity.Netcode.RuntimeTests public void SendReferenceServerRpc(NetworkBehaviourReference value) { RpcReceivedBehaviour = (TestNetworkBehaviour)value; + ReceivedRPC = true; } } @@ -57,8 +60,43 @@ namespace Unity.Netcode.RuntimeTests Assert.AreEqual(testNetworkBehaviour, testNetworkBehaviour.RpcReceivedBehaviour); } + [UnityTest] + public IEnumerator TestSerializeNull([Values] bool initializeWithNull) + { + TestNetworkBehaviour.ReceivedRPC = false; + using var networkObjectContext = UnityObjectContext.CreateNetworkObject(); + var testNetworkBehaviour = networkObjectContext.Object.gameObject.AddComponent(); + networkObjectContext.Object.Spawn(); + using var otherObjectContext = UnityObjectContext.CreateNetworkObject(); + otherObjectContext.Object.Spawn(); + // If not initializing with null, then use the default constructor with no assigned NetworkBehaviour + if (!initializeWithNull) + { + testNetworkBehaviour.SendReferenceServerRpc(new NetworkBehaviourReference()); + } + else // Otherwise, initialize and pass in null as the reference + { + testNetworkBehaviour.SendReferenceServerRpc(new NetworkBehaviourReference(null)); + } + + // wait for rpc completion + float t = 0; + while (!TestNetworkBehaviour.ReceivedRPC) + { + t += Time.deltaTime; + if (t > 5f) + { + new AssertionException("RPC with NetworkBehaviour reference hasn't been received"); + } + + yield return null; + } + + // validate + Assert.AreEqual(null, testNetworkBehaviour.RpcReceivedBehaviour); + } [UnityTest] public IEnumerator TestRpcImplicitNetworkBehaviour() @@ -89,6 +127,7 @@ namespace Unity.Netcode.RuntimeTests Assert.AreEqual(testNetworkBehaviour, testNetworkBehaviour.RpcReceivedBehaviour); } + [Test] public void TestNetworkVariable() { @@ -131,15 +170,6 @@ namespace Unity.Netcode.RuntimeTests }); } - [Test] - public void FailSerializeNullBehaviour() - { - Assert.Throws(() => - { - NetworkBehaviourReference outReference = null; - }); - } - public void Dispose() { //Stop, shutdown, and destroy diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkObjectReferenceTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkObjectReferenceTests.cs index 8b29d17e6..ef459dea4 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkObjectReferenceTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/Serialization/NetworkObjectReferenceTests.cs @@ -19,6 +19,7 @@ namespace Unity.Netcode.RuntimeTests { private class TestNetworkBehaviour : NetworkBehaviour { + public static bool ReceivedRPC; public NetworkVariable TestVariable = new NetworkVariable(); public NetworkObject RpcReceivedNetworkObject; @@ -28,6 +29,7 @@ namespace Unity.Netcode.RuntimeTests [ServerRpc] public void SendReferenceServerRpc(NetworkObjectReference value) { + ReceivedRPC = true; RpcReceivedGameObject = value; RpcReceivedNetworkObject = value; } @@ -150,6 +152,60 @@ namespace Unity.Netcode.RuntimeTests Assert.AreEqual(networkObject, result); } + public enum NetworkObjectConstructorTypes + { + None, + NullNetworkObject, + NullGameObject + } + + [UnityTest] + public IEnumerator TestSerializeNull([Values] NetworkObjectConstructorTypes networkObjectConstructorTypes) + { + TestNetworkBehaviour.ReceivedRPC = false; + using var networkObjectContext = UnityObjectContext.CreateNetworkObject(); + var testNetworkBehaviour = networkObjectContext.Object.gameObject.AddComponent(); + networkObjectContext.Object.Spawn(); + + switch (networkObjectConstructorTypes) + { + case NetworkObjectConstructorTypes.None: + { + testNetworkBehaviour.SendReferenceServerRpc(new NetworkObjectReference()); + break; + } + case NetworkObjectConstructorTypes.NullNetworkObject: + { + testNetworkBehaviour.SendReferenceServerRpc(new NetworkObjectReference((NetworkObject)null)); + break; + } + case NetworkObjectConstructorTypes.NullGameObject: + { + testNetworkBehaviour.SendReferenceServerRpc(new NetworkObjectReference((GameObject)null)); + break; + } + } + + + // wait for rpc completion + float t = 0; + while (!TestNetworkBehaviour.ReceivedRPC) + { + + t += Time.deltaTime; + if (t > 5f) + { + new AssertionException("RPC with NetworkBehaviour reference hasn't been received"); + } + + yield return null; + } + + // validate + Assert.AreEqual(null, testNetworkBehaviour.RpcReceivedNetworkObject); + Assert.AreEqual(null, testNetworkBehaviour.RpcReceivedGameObject); + } + [UnityTest] public IEnumerator TestRpc() { @@ -305,24 +361,6 @@ namespace Unity.Netcode.RuntimeTests }); } - [Test] - public void FailSerializeNullNetworkObject() - { - Assert.Throws(() => - { - NetworkObjectReference outReference = (NetworkObject)null; - }); - } - - [Test] - public void FailSerializeNullGameObject() - { - Assert.Throws(() => - { - NetworkObjectReference outReference = (GameObject)null; - }); - } - public void Dispose() { //Stop, shutdown, and destroy