fix: Don't let exceptions in OnNetworkSpawn/OnNetworkDespawn block processing of the next callback. [MTT-1378] (Unity-Technologies#1739)

* fix: Don't let exceptions in OnNetworkSpawn/OnNetworkDespawn block processing of the next callback.

* Standards and changelog.

* Standards fixes

* It would be good to check in the right file.

* I swear these tests passed before but I have no idea how.

Also review feedback.

* Address review feedback.

* Switched to LogAssert.Expect

* Updated tests to NetcodeIntegrationTest style

* Standards fix

* Rewrote tests according to feedback.

Also fixed merge issue with changelog.

* Test the behavior on both server and client

* style

fixing white space issue.

* Update com.unity.netcode.gameobjects/CHANGELOG.md

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

Co-authored-by: Noel Stephens <noel.stephens@unity3d.com>
Co-authored-by: Fatih Mar <mfatihmar@gmail.com>

Author: 3 people

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Fixed
 
 - Fixed RPC codegen failing to choose the correct extension methods for FastBufferReader and FastBufferWriter when the parameters were a generic type (i.e., List<int>) and extensions for multiple instantiations of that type have been defined (i.e., List<int> and List<string>) (#2142)
+- Fixed throwing an exception in OnNetworkUpdate causing other OnNetworkUpdate calls to not be executed. (#1739)
 
 ## [1.0.1] - 2022-08-23
 
@@ -20,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
 
@@ -238,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)
@@ -263,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<T>` for `NetworkTransform` (#1060)
 - Implemented `NetworkPrefabHandler` that provides support for object pooling and `NetworkPrefab` overrides (#1073, #1004, #977, #905,#749, #727)
@@ -320,7 +321,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Removed `NetworkDictionary`, `NetworkSet` (#1149)
 - Removed `NetworkVariableSettings` (#1097)
 - Removed predefined `NetworkVariable<T>` 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)
@@ -392,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).
@@ -432,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.
@@ -447,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. <!-- MTT54 -->
 - [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.
@@ -463,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.

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

@@ -440,14 +440,28 @@ internal void InternalOnNetworkSpawn()
 
         internal void VisibleOnNetworkSpawn()
         {
-            OnNetworkSpawn();
+            try
+            {
+                OnNetworkSpawn();
+            }
+            catch (Exception e)
+            {
+                Debug.LogException(e);
+            }
         }
 
         internal void InternalOnNetworkDespawn()
         {
             IsSpawned = false;
             UpdateNetworkProperties();
-            OnNetworkDespawn();
+            try
+            {
+                OnNetworkDespawn();
+            }
+            catch (Exception e)
+            {
+                Debug.LogException(e);
+            }
         }
 
         /// <summary>

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<T>(object receivedMessage) where T : INetworkMessage
         {
             return receivedMessage is T;
@@ -57,13 +59,23 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
 
         public void OnBeforeHandleMessage<T>(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<T>(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;
             }
         }
     }