Merge branch 'develop-2.0.0' into netcode-update-after-2.9.0-release-branch-creation

Author: michalChrobot

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,6 +10,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+- The `NetworkMetricsPipelineStage` for Unity Transport is now part of the public API. This allows using it in custom implementations of `INetworkStreamDriverConstructor` that want to maintain compatibility with the multiplayer tools package. (#3853)
 
 ### Changed
 
@@ -22,6 +23,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where `NetworkManager` was not cleaning itself up if an exception was thrown while starting. (#3864)
+- Prevented a `NullReferenceException` in `UnityTransport` when using a custom `INetworkStreamDriverConstructor` that doesn't use all the default pipelines and the multiplayer tools package is installed. (#3853)
 
 ### Security
 

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

@@ -1323,7 +1323,17 @@ public bool StartServer()
             }
             ConnectionManager.LocalClient.ClientId = ServerClientId;
 
-            Initialize(true);
+            try
+            {
+                Initialize(true);
+            }
+            catch (Exception ex)
+            {
+                Debug.LogException(ex);
+                // Always shutdown to assure everything is cleaned up
+                ShutdownInternal();
+                return false;
+            }
 
             try
             {
@@ -1342,11 +1352,12 @@ public bool StartServer()
 
                 ConnectionManager.TransportFailureEventHandler(true);
             }
-            catch (Exception)
+            catch (Exception ex)
             {
-                ConnectionManager.LocalClient.SetRole(false, false);
+                Debug.LogException(ex);
+                // Always shutdown to assure everything is cleaned up
+                ShutdownInternal();
                 IsListening = false;
-                throw;
             }
 
             return IsListening;
@@ -1373,7 +1384,16 @@ public bool StartClient()
                 return false;
             }
 
-            Initialize(false);
+            try
+            {
+                Initialize(false);
+            }
+            catch (Exception ex)
+            {
+                Debug.LogException(ex);
+                ShutdownInternal();
+                return false;
+            }
 
             try
             {
@@ -1391,7 +1411,7 @@ public bool StartClient()
             catch (Exception ex)
             {
                 Debug.LogException(ex);
-                ConnectionManager.LocalClient.SetRole(false, false);
+                ShutdownInternal();
                 IsListening = false;
             }
 
@@ -1419,7 +1439,18 @@ public bool StartHost()
                 return false;
             }
 
-            Initialize(true);
+            try
+            {
+                Initialize(true);
+            }
+            catch (Exception ex)
+            {
+                Debug.LogException(ex);
+                // Always shutdown to assure everything is cleaned up
+                ShutdownInternal();
+                return false;
+            }
+
             try
             {
                 IsListening = NetworkConfig.NetworkTransport.StartServer();
@@ -1437,7 +1468,8 @@ public bool StartHost()
             catch (Exception ex)
             {
                 Debug.LogException(ex);
-                ConnectionManager.LocalClient.SetRole(false, false);
+                // Always shutdown to assure everything is cleaned up
+                ShutdownInternal();
                 IsListening = false;
             }
 

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkPrefabHandler.cs

@@ -287,7 +287,15 @@ internal NetworkObject HandleNetworkPrefabSpawn(uint networkPrefabAssetHash, ulo
             {
                 if (m_PrefabAssetToPrefabHandlerWithData.TryGetValue(networkPrefabAssetHash, out var prefabInstanceHandler))
                 {
-                    networkObjectInstance = prefabInstanceHandler.Instantiate(ownerClientId, position, rotation, instantiationData);
+                    try
+                    {
+                        networkObjectInstance = prefabInstanceHandler.Instantiate(ownerClientId, position, rotation, instantiationData);
+                    }
+                    catch (Exception ex)
+                    {
+                        Debug.LogException(ex);
+                        return null;
+                    }
                 }
                 else
                 {
@@ -297,7 +305,15 @@ internal NetworkObject HandleNetworkPrefabSpawn(uint networkPrefabAssetHash, ulo
             }
             else if (m_PrefabAssetToPrefabHandler.TryGetValue(networkPrefabAssetHash, out var prefabInstanceHandler))
             {
-                networkObjectInstance = prefabInstanceHandler.Instantiate(ownerClientId, position, rotation);
+                try
+                {
+                    networkObjectInstance = prefabInstanceHandler.Instantiate(ownerClientId, position, rotation);
+                }
+                catch (Exception ex)
+                {
+                    Debug.LogException(ex);
+                    return null;
+                }
             }
 
             // Now we must make sure this alternate PrefabAsset spawned in place of the prefab asset with the networkPrefabAssetHash (GlobalObjectIdHash)

com.unity.netcode.gameobjects/Runtime/Transports/UTP/INetworkStreamDriverConstructor.cs

@@ -36,6 +36,8 @@ namespace Unity.Netcode.Transports.UTP
     ///             var settings = transport.GetDefaultNetworkSettings();
     ///             driver = NetworkDriver.Create(new IPCNetworkInterface(), settings);
     ///
+    ///             driver.RegisterPipelineStage(new NetworkMetricsPipelineStage());
+    ///
     ///             transport.GetDefaultPipelineConfigurations(
     ///                 out var unreliableFragmentedPipelineStages,
     ///                 out var unreliableSequencedFragmentedPipelineStages,

com.unity.netcode.gameobjects/Runtime/Transports/UTP/NetworkMetricsPipelineStage.cs

@@ -7,27 +7,32 @@
 
 namespace Unity.Netcode.Transports.UTP
 {
+    /// <summary>
+    /// A pipeline stage that tracks some internal metrics that are then used by the multiplayer
+    /// tools package. This should only be used when creating a custom <see cref="NetworkDriver"/>
+    /// with <see cref="INetworkStreamDriverConstructor"/> if compatibility with the multiplayer
+    /// tools package is desired. In that situation, this stage needs to be registered with the
+    /// constructed driver with <see cref="NetworkDriver.RegisterPipelineStage{T}"/>.
+    /// </summary>
     [BurstCompile]
-    internal unsafe struct NetworkMetricsPipelineStage : INetworkPipelineStage
+    public unsafe struct NetworkMetricsPipelineStage : INetworkPipelineStage
     {
-        private static TransportFunctionPointer<NetworkPipelineStage.ReceiveDelegate> s_ReceiveFunction = new TransportFunctionPointer<NetworkPipelineStage.ReceiveDelegate>(Receive);
-        private static TransportFunctionPointer<NetworkPipelineStage.SendDelegate> s_SendFunction = new TransportFunctionPointer<NetworkPipelineStage.SendDelegate>(Send);
-        private static TransportFunctionPointer<NetworkPipelineStage.InitializeConnectionDelegate> s_InitializeConnectionFunction = new TransportFunctionPointer<NetworkPipelineStage.InitializeConnectionDelegate>(InitializeConnection);
-
+        /// <inheritdoc/>
         public NetworkPipelineStage StaticInitialize(byte* staticInstanceBuffer,
             int staticInstanceBufferLength,
             NetworkSettings settings)
         {
             return new NetworkPipelineStage(
-                s_ReceiveFunction,
-                s_SendFunction,
-                s_InitializeConnectionFunction,
+                new TransportFunctionPointer<NetworkPipelineStage.ReceiveDelegate>(Receive),
+                new TransportFunctionPointer<NetworkPipelineStage.SendDelegate>(Send),
+                new TransportFunctionPointer<NetworkPipelineStage.InitializeConnectionDelegate>(InitializeConnection),
                 ReceiveCapacity: 0,
                 SendCapacity: 0,
                 HeaderCapacity: 0,
                 SharedStateCapacity: UnsafeUtility.SizeOf<NetworkMetricsContext>());
         }
 
+        /// <inheritdoc/>
         public int StaticSize => 0;
 
         [BurstCompile(DisableDirectCall = true)]

com.unity.netcode.gameobjects/Runtime/Transports/UTP/UnityTransport.cs

@@ -1234,13 +1234,30 @@ private void ExtractNetworkMetricsFromPipeline(NetworkPipeline pipeline, Network
                 return;
             }
 
-            //Don't need to dispose of the buffers, they are filled with data pointers.
-            m_Driver.GetPipelineBuffers(pipeline,
-                NetworkPipelineStageId.Get<NetworkMetricsPipelineStage>(),
-                networkConnection,
-                out _,
-                out _,
-                out var sharedBuffer);
+            var sharedBuffer = default(NativeArray<byte>);
+
+            try
+            {
+                // Don't need to dispose of the buffers, they are filled with data pointers.
+                m_Driver.GetPipelineBuffers(pipeline,
+                    NetworkPipelineStageId.Get<NetworkMetricsPipelineStage>(),
+                    networkConnection,
+                    out _,
+                    out _,
+                    out sharedBuffer);
+            }
+            catch (InvalidOperationException)
+            {
+                // Can happen if using a custom driver that isn't configured with the metrics stage.
+                return;
+            }
+
+            // That InvalidOperationException above is only thrown in the editor. In runtime builds
+            // we instead get default return values when the pipeline stage is invalid.
+            if (sharedBuffer == default)
+            {
+                return;
+            }
 
             unsafe
             {

com.unity.netcode.gameobjects/Tests/Editor/Transports/UnityTransportTests.cs

@@ -1,5 +1,6 @@
 using NUnit.Framework;
 using Unity.Netcode.Transports.UTP;
+using Unity.Networking.Transport;
 using UnityEngine;
 using UnityEngine.TestTools;
 
@@ -154,21 +155,21 @@ public void UnityTransport_StartServerWithoutAddresses()
         [Test]
         public void UnityTransport_EmptySecurityStringsShouldThrow([Values("", null)] string cert, [Values("", null)] string secret)
         {
-            var supportingGO = new GameObject();
+            var supportingGo = new GameObject();
             try
             {
-                var networkManager = supportingGO.AddComponent<NetworkManager>(); // NM is required for UTP to work with certificates.
+                var networkManager = supportingGo.AddComponent<NetworkManager>(); // NM is required for UTP to work with certificates.
                 networkManager.NetworkConfig = new NetworkConfig();
-                UnityTransport transport = supportingGO.AddComponent<UnityTransport>();
+                UnityTransport transport = supportingGo.AddComponent<UnityTransport>();
                 networkManager.NetworkConfig.NetworkTransport = transport;
-                transport.Initialize();
+                transport.Initialize(networkManager);
                 transport.SetServerSecrets(serverCertificate: cert, serverPrivateKey: secret);
 
                 // Use encryption, but don't set certificate and check for exception
                 transport.UseEncryption = true;
                 Assert.Throws<System.Exception>(() =>
                 {
-                    networkManager.StartServer();
+                    transport.StartServer();
                 });
                 // Make sure StartServer failed
                 Assert.False(transport.GetNetworkDriver().IsCreated);
@@ -177,9 +178,9 @@ public void UnityTransport_EmptySecurityStringsShouldThrow([Values("", null)] st
             }
             finally
             {
-                if (supportingGO != null)
+                if (supportingGo != null)
                 {
-                    Object.DestroyImmediate(supportingGO);
+                    Object.DestroyImmediate(supportingGo);
                 }
             }
         }
@@ -229,5 +230,46 @@ public void UnityTransport_HostnameValidation((string, bool) testCase)
             transport.Shutdown();
         }
 #endif
+
+        private class IPCDriverConstructor : INetworkStreamDriverConstructor
+        {
+            public void CreateDriver(
+                UnityTransport transport,
+                out NetworkDriver driver,
+                out NetworkPipeline unreliableFragmentedPipeline,
+                out NetworkPipeline unreliableSequencedFragmentedPipeline,
+                out NetworkPipeline reliableSequencedPipeline)
+            {
+                var settings = transport.GetDefaultNetworkSettings();
+                driver = NetworkDriver.Create(new IPCNetworkInterface(), settings);
+
+#if MULTIPLAYER_TOOLS
+                driver.RegisterPipelineStage(new NetworkMetricsPipelineStage());
+#endif
+
+                transport.GetDefaultPipelineConfigurations(
+                    out var unreliableFragmentedPipelineStages,
+                    out var unreliableSequencedFragmentedPipelineStages,
+                    out var reliableSequencedPipelineStages);
+
+                unreliableFragmentedPipeline = driver.CreatePipeline(unreliableFragmentedPipelineStages);
+                unreliableSequencedFragmentedPipeline = driver.CreatePipeline(unreliableSequencedFragmentedPipelineStages);
+                reliableSequencedPipeline = driver.CreatePipeline(reliableSequencedPipelineStages);
+            }
+        }
+
+        [Test]
+        public void UnityTransport_CustomDriverConstructorWithDefaultPipelines()
+        {
+            UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
+            UnityTransport.s_DriverConstructor = new IPCDriverConstructor();
+            transport.Initialize();
+
+            Assert.True(transport.StartServer());
+
+            transport.Shutdown();
+
+            UnityTransport.s_DriverConstructor = null;
+        }
     }
 }