refactor: Remove SIPTransport and replace it with UnityTransport (#1870)

* delete `SIPTransport` &  `SIPTransportTests`

* fix compile errors

* fix

This fixes the timing issues with the MessageOrdering.SpawnRpcDespawn integration test.
This also includes a minor update to NetcodeIntegrationTestHelpers (destroy as opposed to destroy immediate).

* fix

This fixes the issue with MultiClientConnectionApproval.ClientConnectedCallbackTest failing when using UTP as opposed to SIP.

* fix

Fixed issue with ConnectionApprovalHandler's prefab getting destroyed when a client was disconnected.
Applied same fix to the NetcodeIntegrationTest when creating a prefab.

Also fixing issue with NetworkManager spewing spam messages if the NetworkConfig.ConnectionApproval is set but there is no ConnectionApprovalCallback assigned.  Adjusted it to make sure it is either the Server or Host that this applies to, but not a client.

* fix

This fixes the ConnectionApprovalMismatchTest issue that caused it to fail with UTP.
Switching over to client notifications (i.e. the server detects the mismatch and disconnects the clients, we want to make sure the clients received the disconnection notification).

* fix

Global fix for teardown and NetworkObject destroying....make sure the server is the NetworkManagerOwner before destroying it.

* fix

Some NetcodeIntegrationTest adjustments to handle the switch over to UTP.

* fix

Fixes issues with NetworkObjectParentingTests (increased wait time between changes to 1 full Network Tick).
Also the recent NetcodeIntegrationTest updates helped fix some of the issues with NetworkObjectParentingTests.

* fix

Fixes the issue with NetworkSpawnManagerTests.TestConnectAndDisconnect.

* Fix - NetworkSceneManager

Switching to UnityTransport exposed a potential bug where the server disconnects a client while the client is still processing a SceneEventType.Synchronize message which by the time the client processes this it has already sent the SceneEventType.SynchronizeComplete message back to the server that *could* attempt to send a resynchronization message where the MessageSystem will crash because the m_SendQueues entry for the now disconnected client no longer exists.

* Fix - NetcodeIntegrationTest

Added some additional UnityTransport settings for payload size, send queue size, and reducing the connection attempts and time out to connect per attempt.

Added the ability to specify whether you want the server or clients to be instantiated first (see comments under NetcodeIntegrationTest.m_CreateServerFirst for further explanation).

* Fix - ClientOnlyConnectionTests

Removing UNet and just using UTP.
Reducing the timeout period so it doesn't take over 60 seconds to timeout.

* Fix - DisconnectTests

Fixes timing issues with this test that is causing it to not passing when using UTP.

* fix - NetworkObjectOwnershipTests

Fixed timing issues exposed when using UTP.

* fix - NetworkSceneManagerEventDataPoolTest

This fixes an issue when I originally switched it to be the server as the default NetworkManager to be instantiated when using NetcodeIntegrationTestHelper.Create.  This sets the, recently updated NetcodeIntegrationTest  m_CreateServerFirst, clients to instantiate before the server (needed for in-scene objects)

* fix - NetworkSceneManagerSeneVerification

This test requires the clients to be instantiated before the server.

* style

* style

* update MTT-3016

MTT-3016
Changelog entries.

* Update CHANGELOG.md

* Update CHANGELOG.md

* fix - tools

This resolves the issues with tools integration tests and switching over to UnityTransport.

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

Co-authored-by: Fatih Mar <[email protected]>

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

Co-authored-by: Fatih Mar <[email protected]>

* fix

The following fixes some edge-case timing related issues that are being exposed in Yamato where it might take longer than 1 network tick depending upon the virtual machine's current load.  Primarily, the majority of the fixes are handled by just waiting for the condition or timing out.

* fix - console - ps4

the time out was never yielding and causing issues on PS4/

* revert

Removing some code that didn't need to make it into this PR.

Co-authored-by: Fatih Mar <[email protected]>

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,10 +18,12 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- Changed `NetcodeIntegrationTestHelpers` to use `UnityTransport` (#1870)
 - Updated `UnityTransport` dependency on `com.unity.transport` to 1.0.0 (#1849)
 
 ### Removed
 
+- Removed `SIPTransport` (#1870)
 - Removed `SnapshotSystem` (#1852)
 - Removed `com.unity.modules.animation`, `com.unity.modules.physics` and `com.unity.modules.physics2d` dependencies from the package (#1812)
 - Removed `com.unity.collections` dependency from the package (#1849)

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

@@ -930,7 +930,9 @@ private bool CanStart(StartType type)
                 return false;
             }
 
-            if (NetworkConfig.ConnectionApproval)
+            // Only if it is starting as a server or host do we need to check this
+            // Clients don't invoke the ConnectionApprovalCallback
+            if (NetworkConfig.ConnectionApproval && type != StartType.Client)
             {
                 if (ConnectionApprovalCallback == null)
                 {

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -1746,7 +1746,9 @@ private void HandleServerSceneEvent(uint sceneEventId, ulong clientId)
                         // NetworkObjects
                         m_NetworkManager.InvokeOnClientConnectedCallback(clientId);
 
-                        if (sceneEventData.ClientNeedsReSynchronization() && !DisableReSynchronization)
+                        // Check to see if the client needs to resynchronize and before sending the message make sure the client is still connected to avoid
+                        // a potential crash within the MessageSystem (i.e. sending to a client that no longer exists)
+                        if (sceneEventData.ClientNeedsReSynchronization() && !DisableReSynchronization && m_NetworkManager.ConnectedClients.ContainsKey(clientId))
                         {
                             sceneEventData.SceneEventType = SceneEventType.ReSynchronize;
                             SendSceneEventData(sceneEventId, new ulong[] { clientId });

com.unity.netcode.gameobjects/TestHelpers/Runtime/Transport/MessageHooks.cs renamed to com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs

@@ -79,6 +79,15 @@ public static void DeregisterNetworkObject(ulong localClientId, ulong networkObj
         protected const uint k_DefaultTickRate = 30;
         protected abstract int NumberOfClients { get; }
 
+        /// <summary>
+        /// Set this to false to create the clients first.
+        /// Note: If you are using scene placed NetworkObjects or doing any form of scene testing and
+        /// get prefab hash id "soft synchronization" errors, then set this to false and run your test
+        /// again.  This is a work-around until we can resolve some issues with NetworkManagerOwner and
+        /// NetworkManager.Singleton.
+        /// </summary>
+        protected bool m_CreateServerFirst = true;
+
         public enum NetworkManagerInstatiationMode
         {
             PerTest,        // This will create and destroy new NetworkManagers for each test within a child derived class
@@ -108,8 +117,6 @@ public enum HostOrServer
         protected bool m_UseHost = true;
         protected int m_TargetFrameRate = 60;
 
-        protected NetcodeIntegrationTestHelpers.InstanceTransport m_NetworkTransport = NetcodeIntegrationTestHelpers.InstanceTransport.SIP;
-
         private NetworkManagerInstatiationMode m_NetworkManagerInstatiationMode;
 
         private bool m_EnableVerboseDebug;
@@ -252,7 +259,7 @@ protected void CreateServerAndClients(int numberOfClients)
             CreatePlayerPrefab();
 
             // Create multiple NetworkManager instances
-            if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_NetworkTransport))
+            if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_CreateServerFirst))
             {
                 Debug.LogError("Failed to create instances");
                 Assert.Fail("Failed to create instances");
@@ -558,6 +565,7 @@ protected void DestroySceneNetworkObjects()
                 }
                 if (CanDestroyNetworkObject(networkObject))
                 {
+                    networkObject.NetworkManagerOwner = m_ServerNetworkManager;
                     // Destroy the GameObject that holds the NetworkObject component
                     Object.DestroyImmediate(networkObject.gameObject);
                 }
@@ -668,6 +676,7 @@ protected GameObject CreateNetworkObjectPrefab(string baseName)
             var gameObject = new GameObject();
             gameObject.name = baseName;
             var networkObject = gameObject.AddComponent<NetworkObject>();
+            networkObject.NetworkManagerOwner = m_ServerNetworkManager;
             NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject);
             var networkPrefab = new NetworkPrefab() { Prefab = gameObject };
             m_ServerNetworkManager.NetworkConfig.NetworkPrefabs.Add(networkPrefab);

com.unity.netcode.gameobjects/TestHelpers/Runtime/Transport/MessageHooks.cs.meta renamed to com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooks.cs.meta

@@ -107,12 +107,6 @@ public void OnAfterHandleMessage<T>(ref T message, ref NetworkContext context) w
 
         public static List<NetworkManager> NetworkManagerInstances => s_NetworkManagerInstances;
 
-        public enum InstanceTransport
-        {
-            SIP,
-            UTP
-        }
-
         internal static IntegrationTestSceneHandler ClientSceneHandler = null;
 
         /// <summary>
@@ -162,20 +156,32 @@ public static void RegisterHandlers(NetworkManager networkManager, bool serverSi
             }
         }
 
-        /// <summary>
-        /// Create the correct NetworkTransport, attach it to the game object and return it.
-        /// Default value is SIPTransport.
-        /// </summary>
-        internal static NetworkTransport CreateInstanceTransport(InstanceTransport instanceTransport, GameObject go)
+        public static NetworkManager CreateServer()
         {
-            switch (instanceTransport)
+            // Create gameObject
+            var go = new GameObject("NetworkManager - Server");
+
+            // Create networkManager component
+            var server = go.AddComponent<NetworkManager>();
+            NetworkManagerInstances.Insert(0, server);
+
+            // Create transport
+            var unityTransport = go.AddComponent<UnityTransport>();
+            // We need to increase this buffer size for tests that spawn a bunch of things
+            unityTransport.MaxPayloadSize = 256000;
+            unityTransport.MaxSendQueueSize = 1024 * 1024;
+
+            // Allow 4 connection attempts that each will time out after 500ms
+            unityTransport.MaxConnectAttempts = 4;
+            unityTransport.ConnectTimeoutMS = 500;
+
+            // Set the NetworkConfig
+            server.NetworkConfig = new NetworkConfig()
             {
-                case InstanceTransport.SIP:
-                    return go.AddComponent<SIPTransport>();
-                default:
-                case InstanceTransport.UTP:
-                    return go.AddComponent<UnityTransport>();
-            }
+                // Set transport
+                NetworkTransport = unityTransport
+            };
+            return server;
         }
 
         /// <summary>
@@ -185,24 +191,22 @@ internal static NetworkTransport CreateInstanceTransport(InstanceTransport insta
         /// <param name="server">The server NetworkManager</param>
         /// <param name="clients">The clients NetworkManagers</param>
         /// <param name="targetFrameRate">The targetFrameRate of the Unity engine to use while the multi instance helper is running. Will be reset on shutdown.</param>
-        public static bool Create(int clientCount, out NetworkManager server, out NetworkManager[] clients, int targetFrameRate = 60, InstanceTransport instanceTransport = InstanceTransport.SIP)
+        /// <param name="serverFirst">This determines if the server or clients will be instantiated first (defaults to server first)</param>
+        public static bool Create(int clientCount, out NetworkManager server, out NetworkManager[] clients, int targetFrameRate = 60, bool serverFirst = true)
         {
             s_NetworkManagerInstances = new List<NetworkManager>();
-            CreateNewClients(clientCount, out clients, instanceTransport);
-
-            // Create gameObject
-            var go = new GameObject("NetworkManager - Server");
+            server = null;
+            if (serverFirst)
+            {
+                server = CreateServer();
+            }
 
-            // Create networkManager component
-            server = go.AddComponent<NetworkManager>();
-            NetworkManagerInstances.Insert(0, server);
+            CreateNewClients(clientCount, out clients);
 
-            // Set the NetworkConfig
-            server.NetworkConfig = new NetworkConfig()
+            if (!serverFirst)
             {
-                // Set transport
-                NetworkTransport = CreateInstanceTransport(instanceTransport, go)
-            };
+                server = CreateServer();
+            }
 
             s_OriginalTargetFrameRate = Application.targetFrameRate;
             Application.targetFrameRate = targetFrameRate;
@@ -215,7 +219,7 @@ public static bool Create(int clientCount, out NetworkManager server, out Networ
         /// </summary>
         /// <param name="clientCount">The amount of clients</param>
         /// <param name="clients"></param>
-        public static bool CreateNewClients(int clientCount, out NetworkManager[] clients, InstanceTransport instanceTransport = InstanceTransport.SIP)
+        public static bool CreateNewClients(int clientCount, out NetworkManager[] clients)
         {
             clients = new NetworkManager[clientCount];
             var activeSceneName = SceneManager.GetActiveScene().name;
@@ -226,11 +230,14 @@ public static bool CreateNewClients(int clientCount, out NetworkManager[] client
                 // Create networkManager component
                 clients[i] = go.AddComponent<NetworkManager>();
 
+                // Create transport
+                var unityTransport = go.AddComponent<UnityTransport>();
+
                 // Set the NetworkConfig
                 clients[i].NetworkConfig = new NetworkConfig()
                 {
                     // Set transport
-                    NetworkTransport = CreateInstanceTransport(instanceTransport, go)
+                    NetworkTransport = unityTransport
                 };
             }
 
@@ -273,7 +280,10 @@ public static void Destroy()
             // Destroy the network manager instances
             foreach (var networkManager in NetworkManagerInstances)
             {
-                Object.DestroyImmediate(networkManager.gameObject);
+                if (networkManager.gameObject != null)
+                {
+                    Object.Destroy(networkManager.gameObject);
+                }
             }
 
             NetworkManagerInstances.Clear();

com.unity.netcode.gameobjects/TestHelpers/Runtime/Transport/MessageHooksConditional.cs renamed to com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using UnityEngine;
 using NUnit.Framework;
+using Unity.Netcode.Transports.UTP;
 
 namespace Unity.Netcode.TestHelpers.Runtime
 {
@@ -67,11 +68,7 @@ public static bool StartNetworkManager(out NetworkManager networkManager, Networ
 
                 Debug.Log($"{nameof(NetworkManager)} Instantiated.");
 
-                // NOTE: For now we only use SIPTransport for tests until UnityTransport
-                // has been verified working in nightly builds
-                // TODO-MTT-2486: Provide support for other transports once tested and verified
-                // working on consoles.
-                var sipTransport = NetworkManagerGameObject.AddComponent<SIPTransport>();
+                var unityTransport = NetworkManagerGameObject.AddComponent<UnityTransport>();
                 if (networkConfig == null)
                 {
                     networkConfig = new NetworkConfig
@@ -81,7 +78,7 @@ public static bool StartNetworkManager(out NetworkManager networkManager, Networ
                 }
 
                 NetworkManagerObject.NetworkConfig = networkConfig;
-                NetworkManagerObject.NetworkConfig.NetworkTransport = sipTransport;
+                NetworkManagerObject.NetworkConfig.NetworkTransport = unityTransport;
 
                 // Starts the network manager in the mode specified
                 StartNetworkManagerMode(managerMode);