From 6b3e1ab46df8834a60cd3c0b458640b641876d2e Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 22 Feb 2022 11:25:25 -0600 Subject: [PATCH 01/13] fix: Don't let exceptions in OnNetworkSpawn/OnNetworkDespawn block processing of the next callback. --- .../Runtime/Core/NetworkObject.cs | 18 ++- .../Runtime/OnNetworkSpawnExceptionTests.cs | 106 ++++++++++++++++++ .../OnNetworkSpawnExceptionTests.cs.meta | 3 + 3 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs create mode 100644 testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index a5d08ca3f6..46cd3ddc00 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -796,7 +796,14 @@ internal void InvokeBehaviourNetworkSpawn() for (int i = 0; i < ChildNetworkBehaviours.Count; i++) { ChildNetworkBehaviours[i].InternalOnNetworkSpawn(); - ChildNetworkBehaviours[i].OnNetworkSpawn(); + try + { + ChildNetworkBehaviours[i].OnNetworkSpawn(); + } + catch (Exception e) + { + Console.WriteLine(e); + } } } @@ -805,7 +812,14 @@ internal void InvokeBehaviourNetworkDespawn() for (int i = 0; i < ChildNetworkBehaviours.Count; i++) { ChildNetworkBehaviours[i].InternalOnNetworkDespawn(); - ChildNetworkBehaviours[i].OnNetworkDespawn(); + try + { + ChildNetworkBehaviours[i].OnNetworkDespawn(); + } + catch (Exception e) + { + Console.WriteLine(e); + } } } diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs new file mode 100644 index 0000000000..67cfcabd7f --- /dev/null +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -0,0 +1,106 @@ +using System; +using System.Collections; +using Unity.Netcode; +using Unity.Netcode.RuntimeTests; +using UnityEngine; +using UnityEngine.Assertions; +using UnityEngine.TestTools; + +namespace TestProject.RuntimeTests +{ + + public class OnNetworkSpawnThrowsExceptionComponent : NetworkBehaviour + { + public static int NumClientSpawns = 0; + public override void OnNetworkSpawn() + { + if (IsClient) + { + ++NumClientSpawns; + if (NumClientSpawns > 2) + { + throw new Exception("I'm misbehaving"); + } + } + } + } + public class OnNetworkDespawnThrowsExceptionComponent : NetworkBehaviour + { + public static int NumClientDespawns = 0; + public override void OnNetworkDespawn() + { + if (IsClient) + { + ++NumClientDespawns; + if (NumClientDespawns > 2) + { + throw new Exception("I'm misbehaving"); + } + } + } + + } + public class OnNetworkSpawnExceptionTests : BaseMultiInstanceTest + { + public GameObject m_Prefab; + public GameObject[] m_Objects = new GameObject[5]; + + [UnityTest] + public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNotPrevented() + { + //Spawning was done during setup + Assert.AreEqual(5, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); + yield return null; + } + + [UnityTest] + public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAreNotPrevented() + { + //Spawning was done during setup. Now we despawn. + for (var i = 0; i < 5; ++i) + { + m_Objects[i].GetComponent().Despawn(); + } + + var result = new MultiInstanceHelpers.CoroutineResultWrapper(); + yield return MultiInstanceHelpers.Run( + MultiInstanceHelpers.WaitForCondition( + () => OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns == 5, result)); + Assert.IsTrue(result.Result); + } + + public override IEnumerator Setup() + { + yield return StartSomeClientsAndServerWithPlayers(true, NbClients, _ => + { + m_Prefab = new GameObject(); + var networkObject = m_Prefab.AddComponent(); + m_Prefab.AddComponent(); + MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject); + + var validNetworkPrefab = new NetworkPrefab(); + validNetworkPrefab.Prefab = m_Prefab; + m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); + foreach (var client in m_ClientNetworkManagers) + { + client.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); + } + }); + + for (var i = 0; i < 5; ++i) + { + var obj = GameObject.Instantiate(m_Prefab); + m_Objects[i] = obj; + obj.GetComponent().NetworkManagerOwner = m_ServerNetworkManager; + obj.GetComponent().Spawn(); + } + + var result = new MultiInstanceHelpers.CoroutineResultWrapper(); + MultiInstanceHelpers.Run( + MultiInstanceHelpers.WaitForCondition( + () => OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns == 5, result)); + Assert.IsTrue(result.Result); + } + protected override int NbClients => 1; + } +} diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs.meta b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs.meta new file mode 100644 index 0000000000..bdab26680d --- /dev/null +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 4aed67943ab04ae68d0c137e564e1e0e +timeCreated: 1644863541 \ No newline at end of file From d502aa19af243d9a9abbb240267a383d861e88e8 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 22 Feb 2022 11:50:33 -0600 Subject: [PATCH 02/13] Standards and changelog. --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 974ea4793a..343545124b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -27,6 +27,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed NetworkList to properly call INetworkSerializable's NetworkSerialize() method (#1682) - Fixed The NetworkConfig's checksum hash includes the NetworkTick so that clients with a different tickrate than the server are identified and not allowed to connect (#1728) - Fixed OwnedObjects not being properly modified when using ChangeOwnership (#1731) +- Fixed throwing an exception in OnNetworkUpdate causing other OnNetworkUpdate calls to not be executed. (#1739) ## [1.0.0-pre.5] - 2022-01-26 diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index 67cfcabd7f..ce6a709924 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using Unity.Netcode; using Unity.Netcode.RuntimeTests; @@ -89,7 +89,7 @@ public override IEnumerator Setup() for (var i = 0; i < 5; ++i) { - var obj = GameObject.Instantiate(m_Prefab); + var obj = UnityEngine.Object.Instantiate(m_Prefab); m_Objects[i] = obj; obj.GetComponent().NetworkManagerOwner = m_ServerNetworkManager; obj.GetComponent().Spawn(); From ef344d2d9a6123a934864775c61833c6fa2e99f4 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 22 Feb 2022 14:47:29 -0600 Subject: [PATCH 03/13] Standards fixes --- .../ProjectSettings/BurstAotSettings_StandaloneWindows.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json b/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json index 2144f6dc76..e02ae33205 100644 --- a/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json +++ b/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json @@ -1,6 +1,6 @@ { "MonoBehaviour": { - "Version": 3, + "Version": 4, "EnableBurstCompilation": true, "EnableOptimisations": true, "EnableSafetyChecks": false, @@ -11,6 +11,7 @@ "CpuMinTargetX64": 0, "CpuMaxTargetX64": 0, "CpuTargetsX32": 6, - "CpuTargetsX64": 72 + "CpuTargetsX64": 72, + "OptimizeFor": 0 } } From 4749f460cc2ba83dd5d9ecf8d9f70c2ccee8fcd2 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 22 Feb 2022 15:02:13 -0600 Subject: [PATCH 04/13] It would be good to check in the right file. --- .../Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs | 4 ++-- .../ProjectSettings/BurstAotSettings_StandaloneWindows.json | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index ce6a709924..268125553c 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -42,8 +42,8 @@ public override void OnNetworkDespawn() } public class OnNetworkSpawnExceptionTests : BaseMultiInstanceTest { - public GameObject m_Prefab; - public GameObject[] m_Objects = new GameObject[5]; + private GameObject m_Prefab; + private GameObject[] m_Objects = new GameObject[5]; [UnityTest] public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNotPrevented() diff --git a/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json b/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json index e02ae33205..2144f6dc76 100644 --- a/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json +++ b/testproject/ProjectSettings/BurstAotSettings_StandaloneWindows.json @@ -1,6 +1,6 @@ { "MonoBehaviour": { - "Version": 4, + "Version": 3, "EnableBurstCompilation": true, "EnableOptimisations": true, "EnableSafetyChecks": false, @@ -11,7 +11,6 @@ "CpuMinTargetX64": 0, "CpuMaxTargetX64": 0, "CpuTargetsX32": 6, - "CpuTargetsX64": 72, - "OptimizeFor": 0 + "CpuTargetsX64": 72 } } From 23d4b1505e3866b620b6a8f2ddafa60f4ba1a949 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 1 Mar 2022 14:38:35 -0600 Subject: [PATCH 05/13] I swear these tests passed before but I have no idea how. Also review feedback. --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- .../Runtime/Core/NetworkObject.cs | 4 ++-- .../Tests/Runtime/OnNetworkSpawnExceptionTests.cs | 10 ++++++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 343545124b..962e9ec2c5 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -27,7 +27,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed NetworkList to properly call INetworkSerializable's NetworkSerialize() method (#1682) - Fixed The NetworkConfig's checksum hash includes the NetworkTick so that clients with a different tickrate than the server are identified and not allowed to connect (#1728) - Fixed OwnedObjects not being properly modified when using ChangeOwnership (#1731) -- Fixed throwing an exception in OnNetworkUpdate causing other OnNetworkUpdate calls to not be executed. (#1739) +- Fixed throwing an exception in OnNetworkSpawn/OnNetworkDespawn causing other OnNetworkSpawn/OnNetworkDespawn calls to not be executed. (#1739) ## [1.0.0-pre.5] - 2022-01-26 diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index 46cd3ddc00..e649cef115 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -802,7 +802,7 @@ internal void InvokeBehaviourNetworkSpawn() } catch (Exception e) { - Console.WriteLine(e); + NetworkLog.LogError(e.ToString()); } } } @@ -818,7 +818,7 @@ internal void InvokeBehaviourNetworkDespawn() } catch (Exception e) { - Console.WriteLine(e); + NetworkLog.LogError(e.ToString()); } } } diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index 268125553c..e9d1633681 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -48,6 +48,7 @@ public class OnNetworkSpawnExceptionTests : BaseMultiInstanceTest [UnityTest] public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNotPrevented() { + LogAssert.ignoreFailingMessages = true; //Spawning was done during setup Assert.AreEqual(5, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); yield return null; @@ -56,6 +57,7 @@ public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNot [UnityTest] public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAreNotPrevented() { + LogAssert.ignoreFailingMessages = true; //Spawning was done during setup. Now we despawn. for (var i = 0; i < 5; ++i) { @@ -71,11 +73,15 @@ public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAr public override IEnumerator Setup() { - yield return StartSomeClientsAndServerWithPlayers(true, NbClients, _ => + OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns = 0; + OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns = 0; + LogAssert.ignoreFailingMessages = true; + yield return StartSomeClientsAndServerWithPlayers(false, NbClients, _ => { m_Prefab = new GameObject(); var networkObject = m_Prefab.AddComponent(); m_Prefab.AddComponent(); + m_Prefab.AddComponent(); MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject); var validNetworkPrefab = new NetworkPrefab(); @@ -96,7 +102,7 @@ public override IEnumerator Setup() } var result = new MultiInstanceHelpers.CoroutineResultWrapper(); - MultiInstanceHelpers.Run( + yield return MultiInstanceHelpers.Run( MultiInstanceHelpers.WaitForCondition( () => OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns == 5, result)); Assert.IsTrue(result.Result); From 012aa98bf8a192299d15dddcc331f9fe23268ecf Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 29 Mar 2022 13:35:35 -0500 Subject: [PATCH 06/13] Address review feedback. --- com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index e649cef115..9b6381aac9 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -802,7 +802,7 @@ internal void InvokeBehaviourNetworkSpawn() } catch (Exception e) { - NetworkLog.LogError(e.ToString()); + Debug.LogException(e); } } } @@ -818,7 +818,7 @@ internal void InvokeBehaviourNetworkDespawn() } catch (Exception e) { - NetworkLog.LogError(e.ToString()); + Debug.LogException(e); } } } From 530e72887e11de75bddd5c51d770fd24b62d1959 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Tue, 29 Mar 2022 13:43:03 -0500 Subject: [PATCH 07/13] Switched to LogAssert.Expect --- .../Tests/Runtime/OnNetworkSpawnExceptionTests.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index e9d1633681..c46773c51f 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Text.RegularExpressions; using Unity.Netcode; using Unity.Netcode.RuntimeTests; using UnityEngine; @@ -48,7 +49,6 @@ public class OnNetworkSpawnExceptionTests : BaseMultiInstanceTest [UnityTest] public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNotPrevented() { - LogAssert.ignoreFailingMessages = true; //Spawning was done during setup Assert.AreEqual(5, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); yield return null; @@ -57,7 +57,10 @@ public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNot [UnityTest] public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAreNotPrevented() { - LogAssert.ignoreFailingMessages = true; + for (var i = 0; i < 3; ++i) + { + LogAssert.Expect(LogType.Exception, new Regex("I'm misbehaving")); + } //Spawning was done during setup. Now we despawn. for (var i = 0; i < 5; ++i) { @@ -73,9 +76,12 @@ public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAr public override IEnumerator Setup() { + for (var i = 0; i < 3; ++i) + { + LogAssert.Expect(LogType.Exception, new Regex("I'm misbehaving")); + } OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns = 0; OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns = 0; - LogAssert.ignoreFailingMessages = true; yield return StartSomeClientsAndServerWithPlayers(false, NbClients, _ => { m_Prefab = new GameObject(); From 032202baeb14be5da03d0a3032cb6d7c69c1ed38 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Mon, 9 May 2022 17:31:52 -0500 Subject: [PATCH 08/13] Updated tests to NetcodeIntegrationTest style --- .../Runtime/OnNetworkSpawnExceptionTests.cs | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index c46773c51f..3ce8121368 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -3,6 +3,7 @@ using System.Text.RegularExpressions; using Unity.Netcode; using Unity.Netcode.RuntimeTests; +using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.Assertions; using UnityEngine.TestTools; @@ -41,7 +42,7 @@ public override void OnNetworkDespawn() } } - public class OnNetworkSpawnExceptionTests : BaseMultiInstanceTest + public class OnNetworkSpawnExceptionTests : NetcodeIntegrationTest { private GameObject m_Prefab; private GameObject[] m_Objects = new GameObject[5]; @@ -67,37 +68,40 @@ public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAr m_Objects[i].GetComponent().Despawn(); } - var result = new MultiInstanceHelpers.CoroutineResultWrapper(); - yield return MultiInstanceHelpers.Run( - MultiInstanceHelpers.WaitForCondition( - () => OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns == 5, result)); - Assert.IsTrue(result.Result); + var result = new TimeoutHelper(); + yield return WaitForConditionOrTimeOut( + () => OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns == 5, result); + Assert.IsFalse(result.TimedOut); } - public override IEnumerator Setup() + protected override IEnumerator OnSetup() { + m_UseHost = false; + for (var i = 0; i < 3; ++i) { LogAssert.Expect(LogType.Exception, new Regex("I'm misbehaving")); } OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns = 0; OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns = 0; - yield return StartSomeClientsAndServerWithPlayers(false, NbClients, _ => - { - m_Prefab = new GameObject(); - var networkObject = m_Prefab.AddComponent(); - m_Prefab.AddComponent(); - m_Prefab.AddComponent(); - MultiInstanceHelpers.MakeNetworkObjectTestPrefab(networkObject); + yield return null; + } - var validNetworkPrefab = new NetworkPrefab(); - validNetworkPrefab.Prefab = m_Prefab; - m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); - foreach (var client in m_ClientNetworkManagers) - { - client.NetworkConfig.NetworkPrefabs.Add(validNetworkPrefab); - } - }); + protected override IEnumerator OnServerAndClientsConnected() + { + m_Prefab = new GameObject(); + var networkObject = m_Prefab.AddComponent(); + m_Prefab.AddComponent(); + m_Prefab.AddComponent(); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject); + + m_ServerNetworkManager.NetworkConfig.ForceSamePrefabs = false; + m_ServerNetworkManager.AddNetworkPrefab(m_Prefab); + foreach (var client in m_ClientNetworkManagers) + { + client.NetworkConfig.ForceSamePrefabs = false; + client.AddNetworkPrefab(m_Prefab); + } for (var i = 0; i < 5; ++i) { @@ -107,12 +111,12 @@ public override IEnumerator Setup() obj.GetComponent().Spawn(); } - var result = new MultiInstanceHelpers.CoroutineResultWrapper(); - yield return MultiInstanceHelpers.Run( - MultiInstanceHelpers.WaitForCondition( - () => OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns == 5, result)); - Assert.IsTrue(result.Result); + var result = new TimeoutHelper(); + yield return WaitForConditionOrTimeOut( + () => OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns == 5, result); + Assert.IsFalse(result.TimedOut); } - protected override int NbClients => 1; + + protected override int NumberOfClients => 1; } } From cede7c3c08cd5676e6c66fc9fbdb004c44444b56 Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Fri, 20 May 2022 12:53:11 -0500 Subject: [PATCH 09/13] Standards fix --- testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index 3ce8121368..da662bd527 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -2,7 +2,6 @@ using System.Collections; using System.Text.RegularExpressions; using Unity.Netcode; -using Unity.Netcode.RuntimeTests; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.Assertions; From 8a7c8b8304220d8fbbba6da5fb33f556b37b0bce Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Wed, 24 Aug 2022 12:53:12 -0500 Subject: [PATCH 10/13] Rewrote tests according to feedback. Also fixed merge issue with changelog. --- com.unity.netcode.gameobjects/CHANGELOG.md | 119 +++++++-- .../TestHelpers/Runtime/MessageHooks.cs | 16 +- .../Runtime/OnNetworkSpawnExceptionTests.cs | 248 ++++++++++++++---- 3 files changed, 308 insertions(+), 75 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 6d652c93be..3784d92dcd 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -1,3 +1,4 @@ + # Changelog All notable changes to this project will be documented in this file. @@ -6,38 +7,118 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). +## [Unreleased] + ### Fixed -- Fixed: Hosting again after failing to host now works correctly +- Fixed RPC codegen failing to choose the correct extension methods for FastBufferReader and FastBufferWriter when the parameters were a generic type (i.e., List) and extensions for multiple instantiations of that type have been defined (i.e., List and List) (#2142) +- Fixed throwing an exception in OnNetworkUpdate causing other OnNetworkUpdate calls to not be executed. (#1739) -- Fixed NetworkManager to cleanup connected client lists after stopping (#1945) -- Fixed: NetworkHide followed by NetworkShow on the same frame works correctly (#1940) -- Fixed throwing an exception in OnNetworkSpawn/OnNetworkDespawn causing other OnNetworkSpawn/OnNetworkDespawn calls to not be executed. (#1739) +## [1.0.1] - 2022-08-23 + +### Changed + +- Changed version to 1.0.1. (#2131) +- Updated dependency on `com.unity.transport` to 1.2.0. (#2129) +- When using `UnityTransport`, _reliable_ payloads are now allowed to exceed the configured 'Max Payload Size'. Unreliable payloads remain bounded by this setting. (#2081) +- Preformance improvements for cases with large number of NetworkObjects, by not iterating over all unchanged NetworkObjects + +### Fixed +- Fixed not sending all NetworkVariables to all clients when a client connects to a server. (#1987) +- Fixed an issue where reading/writing more than 8 bits at a time with BitReader/BitWriter would write/read from the wrong place, returning and incorrect result. (#2130) +- Fixed issue with the internal `NetworkTransformState.m_Bitset` flag not getting cleared upon the next tick advancement. (#2110) +- Fixed interpolation issue with `NetworkTransform.Teleport`. (#2110) +- Fixed issue where the authoritative side was interpolating its transform. (#2110) +- Fixed Owner-written NetworkVariable infinitely write themselves (#2109) +- Fixed NetworkList issue that showed when inserting at the very end of a NetworkList (#2099) +- Fixed issue where a client owner of a `NetworkVariable` with both owner read and write permissions would not update the server side when changed. (#2097) +- Fixed issue when attempting to spawn a parent `GameObject`, with `NetworkObject` component attached, that has one or more child `GameObject`s, that are inactive in the hierarchy, with `NetworkBehaviour` components it will no longer attempt to spawn the associated `NetworkBehaviour`(s) or invoke ownership changed notifications but will log a warning message. (#2096) +- Fixed an issue where destroying a NetworkBehaviour would not deregister it from the parent NetworkObject, leading to exceptions when the parent was later destroyed. (#2091) +- Fixed issue where `NetworkObject.NetworkHide` was despawning and destroying, as opposed to only despawning, in-scene placed `NetworkObject`s. (#2086) +- Fixed `NetworkAnimator` synchronizing transitions twice due to it detecting the change in animation state once a transition is started by a trigger. (#2084) +- Fixed issue where `NetworkAnimator` would not synchronize a looping animation for late joining clients if it was at the very end of its loop. (#2076) +- Fixed issue where `NetworkAnimator` was not removing its subscription from `OnClientConnectedCallback` when despawned during the shutdown sequence. (#2074) +- Fixed IsServer and IsClient being set to false before object despawn during the shutdown sequence. (#2074) +- Fixed NetworkList Value event on the server. PreviousValue is now set correctly when a new value is set through property setter. (#2067) +- Fixed NetworkLists not populating on client. NetworkList now uses the most recent list as opposed to the list at the end of previous frame, when sending full updates to dynamically spawned NetworkObject. The difference in behaviour is required as scene management spawns those objects at a different time in the frame, relative to updates. (#2062) + +## [1.0.0] - 2022-06-27 + +### Changed + +- Changed version to 1.0.0. (#2046) + +## [1.0.0-pre.10] - 2022-06-21 + +### Added + +- Added a new `OnTransportFailure` callback to `NetworkManager`. This callback is invoked when the manager's `NetworkTransport` encounters an unrecoverable error. Transport failures also cause the `NetworkManager` to shut down. Currently, this is only used by `UnityTransport` to signal a timeout of its connection to the Unity Relay servers. (#1994) +- Added `NetworkEvent.TransportFailure`, which can be used by implementations of `NetworkTransport` to signal to `NetworkManager` that an unrecoverable error was encountered. (#1994) +- Added test to ensure a warning occurs when nesting NetworkObjects in a NetworkPrefab (#1969) +- Added `NetworkManager.RemoveNetworkPrefab(...)` to remove a prefab from the prefabs list (#1950) + +### Changed + +- Updated `UnityTransport` dependency on `com.unity.transport` to 1.1.0. (#2025) +- (API Breaking) `ConnectionApprovalCallback` is no longer an `event` and will not allow more than 1 handler registered at a time. Also, `ConnectionApprovalCallback` is now an `Action<>` taking a `ConnectionApprovalRequest` and a `ConnectionApprovalResponse` that the client code must fill (#1972) (#2002) + +### Removed + +### Fixed +- Fixed issue where dynamically spawned `NetworkObject`s could throw an exception if the scene of origin handle was zero (0) and the `NetworkObject` was already spawned. (#2017) +- Fixed issue where `NetworkObject.Observers` was not being cleared when despawned. (#2009) +- Fixed `NetworkAnimator` could not run in the server authoritative mode. (#2003) +- Fixed issue where late joining clients would get a soft synchronization error if any in-scene placed NetworkObjects were parented under another `NetworkObject`. (#1985) +- Fixed issue where `NetworkBehaviourReference` would throw a type cast exception if using `NetworkBehaviourReference.TryGet` and the component type was not found. (#1984) +- Fixed `NetworkSceneManager` was not sending scene event notifications for the currently active scene and any additively loaded scenes when loading a new scene in `LoadSceneMode.Single` mode. (#1975) +- Fixed issue where one or more clients disconnecting during a scene event would cause `LoadEventCompleted` or `UnloadEventCompleted` to wait until the `NetworkConfig.LoadSceneTimeOut` period before being triggered. (#1973) +- Fixed issues when multiple `ConnectionApprovalCallback`s were registered (#1972) +- Fixed a regression in serialization support: `FixedString`, `Vector2Int`, and `Vector3Int` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper. (#1961) +- Fixed generic types that inherit from NetworkBehaviour causing crashes at compile time. (#1976) +- Fixed endless dialog boxes when adding a `NetworkBehaviour` to a `NetworkManager` or vice-versa. (#1947) +- Fixed `NetworkAnimator` issue where it was only synchronizing parameters if the layer or state changed or was transitioning between states. (#1946) +- Fixed `NetworkAnimator` issue where when it did detect a parameter had changed it would send all parameters as opposed to only the parameters that changed. (#1946) +- Fixed `NetworkAnimator` issue where it was not always disposing the `NativeArray` that is allocated when spawned. (#1946) +- Fixed `NetworkAnimator` issue where it was not taking the animation speed or state speed multiplier into consideration. (#1946) +- Fixed `NetworkAnimator` issue where it was not properly synchronizing late joining clients if they joined while `Animator` was transitioning between states. (#1946) +- Fixed `NetworkAnimator` issue where the server was not relaying changes to non-owner clients when a client was the owner. (#1946) +- Fixed issue where the `PacketLoss` metric for tools would return the packet loss over a connection lifetime instead of a single frame. (#2004) + +## [1.0.0-pre.9] - 2022-05-10 + +### Fixed + +- Fixed Hosting again after failing to host now works correctly (#1938) +- Fixed NetworkManager to cleanup connected client lists after stopping (#1945) +- Fixed NetworkHide followed by NetworkShow on the same frame works correctly (#1940) ## [1.0.0-pre.8] - 2022-04-27 ### Changed - `unmanaged` structs are no longer universally accepted as RPC parameters because some structs (i.e., structs with pointers in them, such as `NativeList`) 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`. (#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). +- 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) (#1912) ### 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) - Fixed an issue where UNetTransport.StartServer would return success even if the underlying transport failed to start (#854) - Passing generic types to RPCs no longer causes a native crash (#1901) +- Fixed a compile failure when compiling against com.unity.nuget.mono-cecil >= 1.11.4 (#1920) - Fixed an issue where calling `Shutdown` on a `NetworkManager` that was already shut down would cause an immediate shutdown the next time it was started (basically the fix makes `Shutdown` idempotent). (#1877) ## [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) @@ -158,7 +239,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - ResetTrigger function to NetworkAnimator (#1327) -### Fixed +### Fixed - Overflow exception when syncing Animator state. (#1327) - Added `try`/`catch` around RPC calls, preventing exception from causing further RPC calls to fail (#1329) @@ -183,7 +264,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Added `ClientNetworkTransform` sample to the SDK package (#1168) - Added `Bootstrap` sample to the SDK package (#1140) - Enhanced `NetworkSceneManager` implementation with additive scene loading capabilities (#1080, #955, #913) - - `NetworkSceneManager.OnSceneEvent` provides improved scene event notificaitons + - `NetworkSceneManager.OnSceneEvent` provides improved scene event notificaitons - Enhanced `NetworkTransform` implementation with per axis/component based and threshold based state replication (#1042, #1055, #1061, #1084, #1101) - Added a jitter-resistent `BufferedLinearInterpolator` for `NetworkTransform` (#1060) - Implemented `NetworkPrefabHandler` that provides support for object pooling and `NetworkPrefab` overrides (#1073, #1004, #977, #905,#749, #727) @@ -240,7 +321,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Removed `NetworkDictionary`, `NetworkSet` (#1149) - Removed `NetworkVariableSettings` (#1097) - Removed predefined `NetworkVariable` types (#1093) - - Removed `NetworkVariableBool`, `NetworkVariableByte`, `NetworkVariableSByte`, `NetworkVariableUShort`, `NetworkVariableShort`, `NetworkVariableUInt`, `NetworkVariableInt`, `NetworkVariableULong`, `NetworkVariableLong`, `NetworkVariableFloat`, `NetworkVariableDouble`, `NetworkVariableVector2`, `NetworkVariableVector3`, `NetworkVariableVector4`, `NetworkVariableColor`, `NetworkVariableColor32`, `NetworkVariableRay`, `NetworkVariableQuaternion` + - Removed `NetworkVariableBool`, `NetworkVariableByte`, `NetworkVariableSByte`, `NetworkVariableUShort`, `NetworkVariableShort`, `NetworkVariableUInt`, `NetworkVariableInt`, `NetworkVariableULong`, `NetworkVariableLong`, `NetworkVariableFloat`, `NetworkVariableDouble`, `NetworkVariableVector2`, `NetworkVariableVector3`, `NetworkVariableVector4`, `NetworkVariableColor`, `NetworkVariableColor32`, `NetworkVariableRay`, `NetworkVariableQuaternion` - Removed `NetworkChannel` and `MultiplexTransportAdapter` (#1133) - Removed ILPP backend for 2019.4, minimum required version is 2020.3+ (#895) - `NetworkManager.NetworkConfig` had the following properties removed: (#1080) @@ -312,14 +393,14 @@ This is the initial experimental Unity MLAPI Package, v0.1.0. - Integrated MLAPI with the Unity Profiler for versions 2020.2 and later: - Added new profiler modules for MLAPI that report important network data. - Attached the profiler to a remote player to view network data over the wire. -- A test project is available for building and experimenting with MLAPI features. This project is available in the MLAPI GitHub [testproject folder](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/tree/release/0.1.0/testproject). +- A test project is available for building and experimenting with MLAPI features. This project is available in the MLAPI GitHub [testproject folder](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/tree/release/0.1.0/testproject). - Added a [MLAPI Community Contributions](https://github.com/Unity-Technologies/mlapi-community-contributions/tree/master/com.mlapi.contrib.extensions) new GitHub repository to accept extensions from the MLAPI community. Current extensions include moved MLAPI features for lag compensation (useful for Server Authoritative actions) and `TrackedObject`. ### Changed - [GitHub 520](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/520): MLAPI now uses the Unity Package Manager for installation management. -- Added functionality and usability to `NetworkVariable`, previously called `NetworkVar`. Updates enhance options and fully replace the need for `SyncedVar`s. -- [GitHub 507](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/507): Reimplemented `NetworkAnimator`, which synchronizes animation states for networked objects. +- Added functionality and usability to `NetworkVariable`, previously called `NetworkVar`. Updates enhance options and fully replace the need for `SyncedVar`s. +- [GitHub 507](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/507): Reimplemented `NetworkAnimator`, which synchronizes animation states for networked objects. - GitHub [444](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/444) and [455](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/455): Channels are now represented as bytes instead of strings. For users of previous versions of MLAPI, this release renames APIs due to refactoring. All obsolete marked APIs have been removed as per [GitHub 513](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/513) and [GitHub 514](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/514). @@ -352,7 +433,7 @@ For users of previous versions of MLAPI, this release renames APIs due to refact ### Fixed -- [GitHub 460](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/460): Fixed an issue for RPC where the host-server was not receiving RPCs from the host-client and vice versa without the loopback flag set in `NetworkingManager`. +- [GitHub 460](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/460): Fixed an issue for RPC where the host-server was not receiving RPCs from the host-client and vice versa without the loopback flag set in `NetworkingManager`. - Fixed an issue where data in the Profiler was incorrectly aggregated and drawn, which caused the profiler data to increment indefinitely instead of resetting each frame. - Fixed an issue the client soft-synced causing PlayMode client-only scene transition issues, caused when running the client in the editor and the host as a release build. Users may have encountered a soft sync of `NetworkedInstanceId` issues in the `SpawnManager.ClientCollectSoftSyncSceneObjectSweep` method. - [GitHub 458](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/458): Fixed serialization issues in `NetworkList` and `NetworkDictionary` when running in Server mode. @@ -367,10 +448,10 @@ With a new release of MLAPI in Unity, some features have been removed: - SyncVars have been removed from MLAPI. Use `NetworkVariable`s in place of this functionality. - [GitHub 527](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/527): Lag compensation systems and `TrackedObject` have moved to the new [MLAPI Community Contributions](https://github.com/Unity-Technologies/mlapi-community-contributions/tree/master/com.mlapi.contrib.extensions) repo. - [GitHub 509](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/509): Encryption has been removed from MLAPI. The `Encryption` option in `NetworkConfig` on the `NetworkingManager` is not available in this release. This change will not block game creation or running. A current replacement for this functionality is not available, and may be developed in future releases. See the following changes: - - Removed `SecuritySendFlags` from all APIs. - - Removed encryption, cryptography, and certificate configurations from APIs including `NetworkManager` and `NetworkConfig`. - - Removed "hail handshake", including `NetworkManager` implementation and `NetworkConstants` entries. - - Modified `RpcQueue` and `RpcBatcher` internals to remove encryption and authentication from reading and writing. + - Removed `SecuritySendFlags` from all APIs. + - Removed encryption, cryptography, and certificate configurations from APIs including `NetworkManager` and `NetworkConfig`. + - Removed "hail handshake", including `NetworkManager` implementation and `NetworkConstants` entries. + - Modified `RpcQueue` and `RpcBatcher` internals to remove encryption and authentication from reading and writing. - Removed the previous MLAPI Profiler editor window from Unity versions 2020.2 and later. - Removed previous MLAPI Convenience and Performance RPC APIs with the new standard RPC API. See [RFC #1](https://github.com/Unity-Technologies/com.unity.multiplayer.rfcs/blob/master/text/0001-std-rpc-api.md) for details. - [GitHub 520](https://github.com/Unity-Technologies/com.unity.multiplayer.mlapi/pull/520): Removed the MLAPI Installer. @@ -383,7 +464,7 @@ With a new release of MLAPI in Unity, some features have been removed: - For `NetworkVariable`, the `NetworkDictionary` `List` and `Set` must use the `reliableSequenced` channel. - `NetworkObjects`s are supported but when spawning a prefab with nested child network objects you have to manually call spawn on them - `NetworkTransform` have the following issues: - - Replicated objects may have jitter. + - Replicated objects may have jitter. - The owner is always authoritative about the object's position. - Scale is not synchronized. - Connection Approval is not called on the host client. diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs index dceceb3fe0..1f56850558 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs @@ -4,10 +4,12 @@ namespace Unity.Netcode.TestHelpers.Runtime { internal class MessageHooks : INetworkHooks { - public bool IsWaiting; + public bool IsWaiting = true; public delegate bool MessageReceiptCheck(object receivedMessage); public MessageReceiptCheck ReceiptCheck; + public static bool CurrentMessageHasTriggerdAHook = false; + public static bool CheckForMessageOfType(object receivedMessage) where T : INetworkMessage { return receivedMessage is T; @@ -57,13 +59,23 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade public void OnBeforeHandleMessage(ref T message, ref NetworkContext context) where T : INetworkMessage { + // The way the system works, it goes through all hooks and calls OnBeforeHandleMessage, then handles the message, + // then goes thorugh all hooks and calls OnAfterHandleMessage. + // This ensures each message only manages to activate a single message hook - because we know that only + // one message will ever be handled between OnBeforeHandleMessage and OnAfterHandleMessage, + // we can reset the flag here, and then in OnAfterHandleMessage, the moment the message matches a hook, + // it'll flip this flag back on, and then other hooks will stop checking that message. + // Without this flag, waiting for 10 messages of the same type isn't possible - all 10 hooks would get + // tripped by the first message. + CurrentMessageHasTriggerdAHook = false; } public void OnAfterHandleMessage(ref T message, ref NetworkContext context) where T : INetworkMessage { - if (IsWaiting && (ReceiptCheck == null || ReceiptCheck.Invoke(message))) + if (!CurrentMessageHasTriggerdAHook && IsWaiting && (ReceiptCheck == null || ReceiptCheck.Invoke(message))) { IsWaiting = false; + CurrentMessageHasTriggerdAHook = true; } } } diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index da662bd527..50936be13f 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -1,15 +1,29 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Text.RegularExpressions; using Unity.Netcode; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; using UnityEngine.Assertions; using UnityEngine.TestTools; +using Random = UnityEngine.Random; namespace TestProject.RuntimeTests { + public class OnNetworkSpawnNoExceptionComponent : NetworkBehaviour + { + public static int NumClientSpawns = 0; + public override void OnNetworkSpawn() + { + if (IsClient) + { + ++NumClientSpawns; + } + } + } + public class OnNetworkSpawnThrowsExceptionComponent : NetworkBehaviour { public static int NumClientSpawns = 0; @@ -18,14 +32,12 @@ public override void OnNetworkSpawn() if (IsClient) { ++NumClientSpawns; - if (NumClientSpawns > 2) - { - throw new Exception("I'm misbehaving"); - } + throw new Exception("Exception thrown in OnNetworkSpawn"); } } } - public class OnNetworkDespawnThrowsExceptionComponent : NetworkBehaviour + + public class OnNetworkDespawnNoExceptionComponent : NetworkBehaviour { public static int NumClientDespawns = 0; public override void OnNetworkDespawn() @@ -33,89 +45,217 @@ public override void OnNetworkDespawn() if (IsClient) { ++NumClientDespawns; - if (NumClientDespawns > 2) - { - throw new Exception("I'm misbehaving"); - } } } + } + public class OnNetworkDespawnThrowsExceptionComponent : NetworkBehaviour + { + public static int NumClientDespawns = 0; + public override void OnNetworkDespawn() + { + if (IsClient) + { + ++NumClientDespawns; + throw new Exception("Exception thrown in OnNetworkDespawn"); + } + } } + public class OnNetworkSpawnExceptionTests : NetcodeIntegrationTest { - private GameObject m_Prefab; - private GameObject[] m_Objects = new GameObject[5]; + protected override int NumberOfClients => 1; + private GameObject m_SpawnExceptionPrefab; + private GameObject m_NoSpawnExceptionPrefab; + private GameObject m_SpawnWithAndWithoutExceptionPrefab; + private GameObject m_DespawnExceptionPrefab; + private GameObject m_NoDespawnExceptionPrefab; + private GameObject m_DespawnWithAndWithoutExceptionPrefab; + + private const int k_NumObjects = 10; [UnityTest] public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNotPrevented() { - //Spawning was done during setup - Assert.AreEqual(5, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); - yield return null; + int numExceptionsExpected = 0; + int numExceptionFreeSpawnsExpected = 0; + + var messageHookEntriesForSpawn = new List(); + + for (var i = 0; i < k_NumObjects; ++i) + { + GameObject instance; + // Randomly create some different objects, but the first will always be with exception, + // the second will always be with both an exception behaviour and a non-exception behaviour, + // and the last will always be without exception, with random choices in between. + // Random.Range with int values 0 and 3 will always return 0, 1, or 2. + var rand = Random.Range(0, 3); + if (i == 0 || (i != 1 && i != k_NumObjects - 1 && rand == 0)) + { + instance = UnityEngine.Object.Instantiate(m_SpawnExceptionPrefab); + ++numExceptionsExpected; + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); + } + else if (i == 1 || (i != k_NumObjects - 1 && rand == 1)) + { + instance = UnityEngine.Object.Instantiate(m_SpawnWithAndWithoutExceptionPrefab); + ++numExceptionsExpected; + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); + ++numExceptionFreeSpawnsExpected; + } + else + { + instance = UnityEngine.Object.Instantiate(m_NoSpawnExceptionPrefab); + ++numExceptionFreeSpawnsExpected; + } + var networkObject = instance.GetComponent(); + networkObject.NetworkManagerOwner = m_ServerNetworkManager; + networkObject.Spawn(); + + var messageHook = new MessageHookEntry(m_ClientNetworkManagers[0]); + messageHook.AssignMessageType(); + messageHookEntriesForSpawn.Add(messageHook); + } + + var condition = new MessageHooksConditional(messageHookEntriesForSpawn); + + yield return WaitForConditionOrTimeOut(condition); + + // Assert that all objects had their OnNetworkSpawn called whether they threw exceptions or not + Assert.AreEqual(numExceptionsExpected, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); + Assert.AreEqual(numExceptionFreeSpawnsExpected, OnNetworkSpawnNoExceptionComponent.NumClientSpawns); } [UnityTest] public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAreNotPrevented() { - for (var i = 0; i < 3; ++i) + int numExceptionsExpected = 0; + int numExceptionFreeDespawnsExpected = 0; + + var messageHookEntriesForSpawn = new List(); + + var allObjects = new List(); + + for (var i = 0; i < k_NumObjects; ++i) { - LogAssert.Expect(LogType.Exception, new Regex("I'm misbehaving")); + GameObject instance; + // Randomly create some different objects, but the first will always be with exception, + // the second will always be with both an exception behaviour and a non-exception behaviour, + // and the last will always be without exception, with random choices in between. + // Random.Range with int values 0 and 3 will always return 0, 1, or 2. + var rand = Random.Range(0, 3); + if (i == 0 || (i != 1 && i != k_NumObjects - 1 && rand == 0)) + { + instance = UnityEngine.Object.Instantiate(m_DespawnExceptionPrefab); + ++numExceptionsExpected; + } + else if (i == 1 || (i != k_NumObjects - 1 && rand == 1)) + { + instance = UnityEngine.Object.Instantiate(m_DespawnWithAndWithoutExceptionPrefab); + ++numExceptionsExpected; + ++numExceptionFreeDespawnsExpected; + } + else + { + instance = UnityEngine.Object.Instantiate(m_NoDespawnExceptionPrefab); + ++numExceptionFreeDespawnsExpected; + } + var networkObject = instance.GetComponent(); + networkObject.NetworkManagerOwner = m_ServerNetworkManager; + networkObject.Spawn(); + + allObjects.Add(networkObject); + + var messageHook = new MessageHookEntry(m_ClientNetworkManagers[0]); + messageHook.AssignMessageType(); + messageHookEntriesForSpawn.Add(messageHook); } - //Spawning was done during setup. Now we despawn. - for (var i = 0; i < 5; ++i) + + var spawnCondition = new MessageHooksConditional(messageHookEntriesForSpawn); + + yield return WaitForConditionOrTimeOut(spawnCondition); + + // Make sure no exceptions were thrown in the spawn + LogAssert.NoUnexpectedReceived(); + + var messageHookEntriesForDespawn = new List(); + for (var i = 0; i < numExceptionsExpected; ++i) { - m_Objects[i].GetComponent().Despawn(); + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkDespawn")); } + foreach (var networkObject in allObjects) + { + networkObject.Despawn(); + var messageHook = new MessageHookEntry(m_ClientNetworkManagers[0]); + messageHook.AssignMessageType(); + messageHookEntriesForDespawn.Add(messageHook); + } + var despawnCondition = new MessageHooksConditional(messageHookEntriesForDespawn); + + yield return WaitForConditionOrTimeOut(despawnCondition); - var result = new TimeoutHelper(); - yield return WaitForConditionOrTimeOut( - () => OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns == 5, result); - Assert.IsFalse(result.TimedOut); + // Assert that all objects had their OnNetworkSpawn called whether they threw exceptions or not + Assert.AreEqual(numExceptionsExpected, OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns); + Assert.AreEqual(numExceptionFreeDespawnsExpected, OnNetworkDespawnNoExceptionComponent.NumClientDespawns); } protected override IEnumerator OnSetup() { m_UseHost = false; - - for (var i = 0; i < 3; ++i) - { - LogAssert.Expect(LogType.Exception, new Regex("I'm misbehaving")); - } OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns = 0; OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns = 0; yield return null; } - protected override IEnumerator OnServerAndClientsConnected() + protected override void OnServerAndClientsCreated() { - m_Prefab = new GameObject(); - var networkObject = m_Prefab.AddComponent(); - m_Prefab.AddComponent(); - m_Prefab.AddComponent(); - NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject); - - m_ServerNetworkManager.NetworkConfig.ForceSamePrefabs = false; - m_ServerNetworkManager.AddNetworkPrefab(m_Prefab); - foreach (var client in m_ClientNetworkManagers) - { - client.NetworkConfig.ForceSamePrefabs = false; - client.AddNetworkPrefab(m_Prefab); - } + m_SpawnExceptionPrefab = new GameObject("Spawn Exception Object"); + m_NoSpawnExceptionPrefab = new GameObject("Spawn Normal Object"); + m_SpawnWithAndWithoutExceptionPrefab = new GameObject("Spawn Hybrid Object"); + m_DespawnExceptionPrefab = new GameObject("Despawn Exception Object"); + m_NoDespawnExceptionPrefab = new GameObject("Despawn Normal Object"); + m_DespawnWithAndWithoutExceptionPrefab = new GameObject("Despawn Hybrid Object"); + + m_SpawnExceptionPrefab.AddComponent(); + m_NoSpawnExceptionPrefab.AddComponent(); + + // Note: Unity does not actually guarantee that GetComponenetsInChildren() will return the components in the + // same order they are added, however in practice it *seems* to do so. This is an attempt to test that + // an exception thrown in one NetworkBehaviour won't prevent another NetworkBehaviour on the same object + // from executing its events... but this only works because that behaviour in Unity happens to work that way. + // If a future version of Unity changes that, then this test won't actually test that, but there doesn't + // seem to be any way to actually make this reliable, so this test is just doing the best it can with + // current Unity behavior to test this. + m_SpawnWithAndWithoutExceptionPrefab.AddComponent(); + m_SpawnWithAndWithoutExceptionPrefab.AddComponent(); + + m_DespawnExceptionPrefab.AddComponent(); + m_NoDespawnExceptionPrefab.AddComponent(); + m_DespawnWithAndWithoutExceptionPrefab.AddComponent(); + m_DespawnWithAndWithoutExceptionPrefab.AddComponent(); + + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_SpawnExceptionPrefab.AddComponent()); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_NoSpawnExceptionPrefab.AddComponent()); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_SpawnWithAndWithoutExceptionPrefab.AddComponent()); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_DespawnExceptionPrefab.AddComponent()); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_NoDespawnExceptionPrefab.AddComponent()); + NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(m_DespawnWithAndWithoutExceptionPrefab.AddComponent()); - for (var i = 0; i < 5; ++i) + m_ServerNetworkManager.AddNetworkPrefab(m_SpawnExceptionPrefab); + m_ServerNetworkManager.AddNetworkPrefab(m_NoSpawnExceptionPrefab); + m_ServerNetworkManager.AddNetworkPrefab(m_SpawnWithAndWithoutExceptionPrefab); + m_ServerNetworkManager.AddNetworkPrefab(m_DespawnExceptionPrefab); + m_ServerNetworkManager.AddNetworkPrefab(m_NoDespawnExceptionPrefab); + m_ServerNetworkManager.AddNetworkPrefab(m_DespawnWithAndWithoutExceptionPrefab); + foreach (var client in m_ClientNetworkManagers) { - var obj = UnityEngine.Object.Instantiate(m_Prefab); - m_Objects[i] = obj; - obj.GetComponent().NetworkManagerOwner = m_ServerNetworkManager; - obj.GetComponent().Spawn(); + client.AddNetworkPrefab(m_SpawnExceptionPrefab); + client.AddNetworkPrefab(m_NoSpawnExceptionPrefab); + client.AddNetworkPrefab(m_SpawnWithAndWithoutExceptionPrefab); + client.AddNetworkPrefab(m_DespawnExceptionPrefab); + client.AddNetworkPrefab(m_NoDespawnExceptionPrefab); + client.AddNetworkPrefab(m_DespawnWithAndWithoutExceptionPrefab); } - - var result = new TimeoutHelper(); - yield return WaitForConditionOrTimeOut( - () => OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns == 5, result); - Assert.IsFalse(result.TimedOut); } - - protected override int NumberOfClients => 1; } } From a70c778239ec2682a24c3b4a056df80362fcb52e Mon Sep 17 00:00:00 2001 From: Kitty Draper Date: Wed, 24 Aug 2022 13:02:02 -0500 Subject: [PATCH 11/13] Test the behavior on both server and client --- .../Runtime/OnNetworkSpawnExceptionTests.cs | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs index 50936be13f..db2886ac0d 100644 --- a/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs +++ b/testproject/Assets/Tests/Runtime/OnNetworkSpawnExceptionTests.cs @@ -15,50 +15,70 @@ namespace TestProject.RuntimeTests public class OnNetworkSpawnNoExceptionComponent : NetworkBehaviour { public static int NumClientSpawns = 0; + public static int NumServerSpawns = 0; public override void OnNetworkSpawn() { if (IsClient) { ++NumClientSpawns; } + else + { + ++NumServerSpawns; + } } } public class OnNetworkSpawnThrowsExceptionComponent : NetworkBehaviour { public static int NumClientSpawns = 0; + public static int NumServerSpawns = 0; public override void OnNetworkSpawn() { if (IsClient) { ++NumClientSpawns; - throw new Exception("Exception thrown in OnNetworkSpawn"); } + else + { + ++NumServerSpawns; + } + throw new Exception("Exception thrown in OnNetworkSpawn"); } } public class OnNetworkDespawnNoExceptionComponent : NetworkBehaviour { public static int NumClientDespawns = 0; + public static int NumServerDespawns = 0; public override void OnNetworkDespawn() { if (IsClient) { ++NumClientDespawns; } + else + { + ++NumServerDespawns; + } } } public class OnNetworkDespawnThrowsExceptionComponent : NetworkBehaviour { public static int NumClientDespawns = 0; + public static int NumServerDespawns = 0; public override void OnNetworkDespawn() { if (IsClient) { ++NumClientDespawns; - throw new Exception("Exception thrown in OnNetworkDespawn"); } + else + { + ++NumServerDespawns; + } + throw new Exception("Exception thrown in OnNetworkDespawn"); } } @@ -94,12 +114,16 @@ public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNot { instance = UnityEngine.Object.Instantiate(m_SpawnExceptionPrefab); ++numExceptionsExpected; + // One for server, one for client. + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); } else if (i == 1 || (i != k_NumObjects - 1 && rand == 1)) { instance = UnityEngine.Object.Instantiate(m_SpawnWithAndWithoutExceptionPrefab); ++numExceptionsExpected; + // One for server, one for client. + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkSpawn")); ++numExceptionFreeSpawnsExpected; } @@ -123,7 +147,9 @@ public IEnumerator WhenOnNetworkSpawnThrowsException_FutureOnNetworkSpawnsAreNot // Assert that all objects had their OnNetworkSpawn called whether they threw exceptions or not Assert.AreEqual(numExceptionsExpected, OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns); + Assert.AreEqual(numExceptionsExpected, OnNetworkSpawnThrowsExceptionComponent.NumServerSpawns); Assert.AreEqual(numExceptionFreeSpawnsExpected, OnNetworkSpawnNoExceptionComponent.NumClientSpawns); + Assert.AreEqual(numExceptionFreeSpawnsExpected, OnNetworkSpawnNoExceptionComponent.NumServerSpawns); } [UnityTest] @@ -181,6 +207,8 @@ public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAr var messageHookEntriesForDespawn = new List(); for (var i = 0; i < numExceptionsExpected; ++i) { + // One for server, one for client + LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkDespawn")); LogAssert.Expect(LogType.Exception, new Regex("Exception thrown in OnNetworkDespawn")); } foreach (var networkObject in allObjects) @@ -196,14 +224,22 @@ public IEnumerator WhenOnNetworkDespawnThrowsException_FutureOnNetworkDespawnsAr // Assert that all objects had their OnNetworkSpawn called whether they threw exceptions or not Assert.AreEqual(numExceptionsExpected, OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns); + Assert.AreEqual(numExceptionsExpected, OnNetworkDespawnThrowsExceptionComponent.NumServerDespawns); Assert.AreEqual(numExceptionFreeDespawnsExpected, OnNetworkDespawnNoExceptionComponent.NumClientDespawns); + Assert.AreEqual(numExceptionFreeDespawnsExpected, OnNetworkDespawnNoExceptionComponent.NumServerDespawns); } protected override IEnumerator OnSetup() { m_UseHost = false; OnNetworkSpawnThrowsExceptionComponent.NumClientSpawns = 0; + OnNetworkSpawnThrowsExceptionComponent.NumServerSpawns = 0; + OnNetworkSpawnNoExceptionComponent.NumClientSpawns = 0; + OnNetworkSpawnNoExceptionComponent.NumServerSpawns = 0; OnNetworkDespawnThrowsExceptionComponent.NumClientDespawns = 0; + OnNetworkDespawnThrowsExceptionComponent.NumServerDespawns = 0; + OnNetworkDespawnNoExceptionComponent.NumClientDespawns = 0; + OnNetworkDespawnNoExceptionComponent.NumServerDespawns = 0; yield return null; } From 5017321307cad44f2385c9ff44d7e970d7638e85 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 24 Aug 2022 15:38:41 -0500 Subject: [PATCH 12/13] style fixing white space issue. --- com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index ed48de8a40..945a8f834f 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -439,7 +439,7 @@ internal void InternalOnNetworkSpawn() } internal void VisibleOnNetworkSpawn() - { + { try { OnNetworkSpawn(); From 0fe647157fe365c7bea8621a34a03069b6e3bf4c Mon Sep 17 00:00:00 2001 From: Kitty Draper <284434+ShadauxCat@users.noreply.github.com> Date: Wed, 24 Aug 2022 17:37:43 -0500 Subject: [PATCH 13/13] Update com.unity.netcode.gameobjects/CHANGELOG.md Co-authored-by: Fatih Mar --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 3784d92dcd..688d039d93 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -21,7 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Changed version to 1.0.1. (#2131) - Updated dependency on `com.unity.transport` to 1.2.0. (#2129) - When using `UnityTransport`, _reliable_ payloads are now allowed to exceed the configured 'Max Payload Size'. Unreliable payloads remain bounded by this setting. (#2081) -- Preformance improvements for cases with large number of NetworkObjects, by not iterating over all unchanged NetworkObjects +- Performance improvements for cases with large number of NetworkObjects, by not iterating over all unchanged NetworkObjects ### Fixed