fix: CMB Service not disconnecting

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -77,6 +77,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- CMB service no longer waits for a timeout before disconnecting on an invalid ConnectionRequest. (#3812)
 - Initialization errors with NetworkAnimator. (#3767)
 - Multiple disconnect events from the same transport will no longer disconnect the host. (#3707)
 - Fixed NetworkTransform state synchronization issue when `NetworkTransform.SwitchTransformSpaceWhenParented` is enabled and the associated NetworkObject is parented multiple times in a single frame or within a couple of frames. (#3664)

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -492,6 +492,8 @@ internal void HandleNetworkEvent(NetworkEvent networkEvent, ulong transportClien
         /// </remarks>
         private ulong m_LocalClientTransportId;
 
+        internal ulong LocalClientTransportId => m_LocalClientTransportId;
+
         /// <summary>
         /// Handles a <see cref="NetworkEvent.Connect"/> event.
         /// </summary>
@@ -595,8 +597,10 @@ internal void DisconnectEventHandler(ulong transportClientId)
             // do not remove it just yet.
             var (clientId, isConnectedClient) = TransportIdToClientId(transportClientId);
 
-            // If the client is not registered and we are the server
-            if (!isConnectedClient && NetworkManager.IsServer)
+            // If the client is not registered and we are the server or we are connecting to
+            // the live CMB service and the client had a transport Id assigned then exit early
+            /// <see cref="DisconnectReasonMessage"/> handles disconnecting the client
+            if (!isConnectedClient && (NetworkManager.IsServer || (NetworkManager.CMBServiceConnection && m_LocalClientTransportId != 0)))
             {
                 // Then exit early
                 return;
@@ -639,8 +643,18 @@ internal void DisconnectEventHandler(ulong transportClientId)
                 // Client's clean up their transport id separately from the server.
                 TransportIdCleanUp(transportClientId);
 
-                // Notify local client of disconnection
-                InvokeOnClientDisconnectCallback(clientId);
+                try
+                {
+                    // Notify local client of disconnection
+                    InvokeOnClientDisconnectCallback(clientId);
+                }
+                catch (Exception ex)
+                {
+                    Debug.LogException(ex);
+                }
+
+                // Reset the transport ID
+                m_LocalClientTransportId = 0;
 
                 // As long as we are not in the middle of a shutdown
                 if (!NetworkManager.ShutdownInProgress)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/DisconnectReasonMessage.cs

@@ -1,3 +1,6 @@
+using System.Collections;
+using UnityEngine;
+
 namespace Unity.Netcode
 {
     internal struct DisconnectReasonMessage : INetworkMessage
@@ -37,10 +40,24 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
 
         public void Handle(ref NetworkContext context)
         {
+            var networkManager = (NetworkManager)context.SystemOwner;
             // Always apply the server-side generated disconnect reason to the server specific disconnect reason.
             // This is combined with the additional disconnect information when getting NetworkManager.DisconnectReason
             // (NetworkConnectionManager.DisconnectReason).
-            ((NetworkManager)context.SystemOwner).ConnectionManager.ServerDisconnectReason = Reason;
+            networkManager.ConnectionManager.ServerDisconnectReason = Reason;
+
+            if (networkManager.NetworkConfig.UseCMBService)
+            {
+                networkManager.StartCoroutine(HandleDisconnectAfterReason(networkManager));
+            }
+        }
+
+        private IEnumerator HandleDisconnectAfterReason(NetworkManager networkManager)
+        {
+            yield return new WaitForFixedUpdate();
+
+            var connectionManager = networkManager.ConnectionManager;
+            connectionManager.DisconnectEventHandler(connectionManager.LocalClientTransportId);
         }
     };
 }

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SessionVersionConnectionRequest.cs

@@ -9,52 +9,36 @@ internal class SessionVersionConnectionRequest : NetcodeIntegrationTest
     {
         protected override int NumberOfClients => 0;
 
-        // TODO: [CmbServiceTests] Adapt to run with the service
-        protected override bool UseCMBService()
-        {
-            return false;
-        }
+        // Use a specific version for the CMB tests
+        // The CMB service has more detailed versioning logic. Not all lower versions are invalid to connect with the higher version.
+        // This version will not connect with the version lower.
+        private const int k_ValidCMBVersion = 5;
 
         public SessionVersionConnectionRequest() : base(NetworkTopologyTypes.DistributedAuthority, HostOrServer.DAHost) { }
 
         private bool m_UseValidSessionVersion;
         private bool m_ClientWasDisconnected;
-        private NetworkManager m_ClientNetworkManager;
+        private bool m_CanStartClients;
+
+        // Don't start automatically when using the CMB Service
+        // We want to customize the SessionVersion of the session owner before they connect
+        protected override bool CanStartServerAndClients() => !m_UseCmbService || m_CanStartClients;
 
         /// <summary>
         /// Callback used to mock the scenario where a client has an invalid session version
         /// </summary>
-        /// <returns><see cref="SessionConfig"/></returns>
-        private SessionConfig GetInavlidSessionConfig()
+        private SessionConfig GetInvalidSessionConfig()
         {
             var authority = GetAuthorityNetworkManager();
             return new SessionConfig(authority.SessionConfig.SessionVersion - 1);
         }
 
-        /// <summary>
-        /// Overriding this method allows us to configure the newly instantiated client's
-        /// NetworkManager prior to it being started.
-        /// </summary>
-        /// <param name="networkManager">the newly instantiated NetworkManager</param>
-        protected override void OnNewClientCreated(NetworkManager networkManager)
-        {
-            m_ClientWasDisconnected = false;
-            m_ClientNetworkManager = networkManager;
-            m_ClientNetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
-            if (!m_UseValidSessionVersion)
-            {
-                networkManager.OnGetSessionConfig = GetInavlidSessionConfig;
-            }
-            base.OnNewClientCreated(networkManager);
-        }
-
         /// <summary>
         /// Tracks if the client was disconnected or not
         /// </summary>
         private void OnClientDisconnectCallback(ulong clientId)
         {
             m_ClientWasDisconnected = true;
-            m_ClientNetworkManager.OnClientDisconnectCallback -= OnClientDisconnectCallback;
         }
 
         /// <summary>
@@ -69,51 +53,76 @@ protected override bool ShouldWaitForNewClientToConnect(NetworkManager networkMa
         {
             return m_UseValidSessionVersion;
         }
-
-        internal enum SessionVersionType
-        {
-            Valid,
-            Invalid,
-        }
-
         /// <summary>
         /// Validates that when the client's session config version is valid a client will be
         /// allowed to connect and when it is not valid the client will be disconnected.
         /// </summary>
-        /// <remarks>
-        /// This is just a mock of the service logic to validate everything on the NGO side is
-        /// working correctly.
-        /// </remarks>
-        /// <param name="useValidSessionVersion">true = use valid session version | false = use invalid session version</param>
         [UnityTest]
-        public IEnumerator ValidateSessionVersion([Values] SessionVersionType type)
+        public IEnumerator ValidateSessionVersion()
         {
-            // Test client being disconnected due to invalid session version
-            m_UseValidSessionVersion = type == SessionVersionType.Valid;
-            yield return CreateAndStartNewClient();
-            yield return s_DefaultWaitForTick;
-            if (!m_UseValidSessionVersion)
+            if (m_UseCmbService)
             {
-                yield return WaitForConditionOrTimeOut(() => m_ClientWasDisconnected);
-                AssertOnTimeout("Client was not disconnected when it should have been!");
-                Assert.True(m_ClientNetworkManager.DisconnectReason.Contains(ConnectionRequestMessage.InvalidSessionVersionMessage), "Client did not receive the correct invalid session version message!");
+                var authority = GetAuthorityNetworkManager();
+                authority.OnGetSessionConfig = () => new SessionConfig(k_ValidCMBVersion);
+                m_CanStartClients = true;
+                yield return StartServerAndClients();
             }
-            else
+
+            /*
+             * Test client being disconnected due to invalid session version
+             */
+            m_UseValidSessionVersion = false;
+
+            // Create and setup client to use invalid session config
+            var invalidClient = CreateNewClient();
+            invalidClient.OnClientDisconnectCallback += OnClientDisconnectCallback;
+            invalidClient.OnGetSessionConfig = GetInvalidSessionConfig;
+
+            // Start client and wait for disconnect callback
+            m_ClientWasDisconnected = false;
+            yield return StartClient(invalidClient);
+            Assert.True(invalidClient.IsListening);
+            yield return s_DefaultWaitForTick;
+
+            var timeoutHelper = new TimeoutHelper(30f);
+            yield return WaitForConditionOrTimeOut(() => !invalidClient.IsListening, timeoutHelper);
+            AssertOnTimeout("Client is still listening when it should have been disconnected!", timeoutHelper);
+
+            yield return WaitForConditionOrTimeOut(() => m_ClientWasDisconnected);
+            AssertOnTimeout("Client was not disconnected when it should have been!");
+
+            var expectedReason = m_UseCmbService ? "incompatible ngo c# package versions for feature" : ConnectionRequestMessage.InvalidSessionVersionMessage;
+            Assert.That(invalidClient.DisconnectReason, Does.Contain(expectedReason), $"Client did not receive the correct invalid session version message! Received: {invalidClient.DisconnectReason}");
+
+            // Clean up invalid client
+            invalidClient.OnClientDisconnectCallback -= OnClientDisconnectCallback;
+            yield return StopOneClient(invalidClient, true);
+
+            /*
+             * Test a later client with a valid version
+             * They should connect as normal
+             */
+            m_UseValidSessionVersion = true;
+
+            // Create and setup client to use invalid session config
+            var lateJoin = CreateNewClient();
+            lateJoin.OnClientDisconnectCallback += OnClientDisconnectCallback;
+            if (m_UseCmbService)
             {
-                Assert.False(m_ClientWasDisconnected, "Client was disconnected when it was expected to connect!");
-                Assert.True(m_ClientNetworkManager.IsConnectedClient, "Client did not connect properly using the correct session version!");
+                lateJoin.OnGetSessionConfig = () => new SessionConfig(k_ValidCMBVersion);
             }
-        }
 
-        /// <summary>
-        /// Invoked at the end of each integration test pass.
-        /// Primarily used to clean up for the next pass.
-        /// </summary>
-        protected override IEnumerator OnTearDown()
-        {
-            m_ClientNetworkManager.OnClientDisconnectCallback -= OnClientDisconnectCallback;
-            m_ClientNetworkManager = null;
-            yield return base.OnTearDown();
+            // Start client and wait for disconnect callback
+            m_ClientWasDisconnected = false;
+            yield return StartClient(lateJoin);
+            yield return s_DefaultWaitForTick;
+
+            Assert.False(m_ClientWasDisconnected, "Client was disconnected when it was expected to connect!");
+            Assert.True(lateJoin.IsConnectedClient, "Client did not connect properly using the correct session version!");
+            Assert.That(GetAuthorityNetworkManager().ConnectedClientsIds, Has.Member(lateJoin.LocalClientId), "Newly joined client should be in connected list!");
+
+            // Clean up
+            lateJoin.OnClientDisconnectCallback -= OnClientDisconnectCallback;
         }
     }
 }

testproject/.vsconfig

@@ -0,0 +1,6 @@
+{
+  "version": "1.0",
+  "components": [
+    "Microsoft.VisualStudio.Workload.ManagedGame"
+  ]
+}

testproject/Packages/packages-lock.json

@@ -457,7 +457,8 @@
         "com.unity.modules.ui": "1.0.0",
         "com.unity.modules.imgui": "1.0.0",
         "com.unity.modules.jsonserialize": "1.0.0",
-        "com.unity.modules.hierarchycore": "1.0.0"
+        "com.unity.modules.hierarchycore": "1.0.0",
+        "com.unity.modules.physics": "1.0.0"
       }
     },
     "com.unity.modules.umbra": {

testproject/ProjectSettings/ProjectSettings.asset

@@ -70,6 +70,7 @@ PlayerSettings:
   androidStartInFullscreen: 1
   androidRenderOutsideSafeArea: 1
   androidUseSwappy: 1
+  androidDisplayOptions: 1
   androidBlitType: 0
   androidResizeableActivity: 0
   androidDefaultWindowWidth: 1920
@@ -86,6 +87,7 @@ PlayerSettings:
   muteOtherAudioSources: 0
   Prepare IOS For Recording: 0
   Force IOS Speakers When Recording: 0
+  audioSpatialExperience: 0
   deferSystemGesturesMode: 0
   hideHomeButton: 0
   submitAnalytics: 1
@@ -132,6 +134,7 @@ PlayerSettings:
   switchNVNMaxPublicSamplerIDCount: 0
   switchMaxWorkerMultiple: 8
   switchNVNGraphicsFirmwareMemory: 32
+  switchGraphicsJobsSyncAfterKick: 1
   vulkanNumSwapchainBuffers: 3
   vulkanEnableSetSRGBWrite: 0
   vulkanEnablePreTransform: 0
@@ -271,6 +274,9 @@ PlayerSettings:
   AndroidBuildApkPerCpuArchitecture: 0
   AndroidTVCompatibility: 0
   AndroidIsGame: 1
+  androidAppCategory: 3
+  useAndroidAppCategory: 1
+  androidAppCategoryOther: 
   AndroidEnableTango: 0
   androidEnableBanner: 1
   androidUseLowAccuracyLocation: 0
@@ -442,6 +448,9 @@ PlayerSettings:
   - m_BuildTarget: WebGLSupport
     m_APIs: 0b000000
     m_Automatic: 1
+  - m_BuildTarget: WindowsStandaloneSupport
+    m_APIs: 0200000012000000
+    m_Automatic: 0
   m_BuildTargetVRSettings:
   - m_BuildTarget: Standalone
     m_Enabled: 0
@@ -723,12 +732,12 @@ PlayerSettings:
   webGLMemoryLinearGrowthStep: 16
   webGLMemoryGeometricGrowthStep: 0.2
   webGLMemoryGeometricGrowthCap: 96
-  webGLEnableWebGPU: 0
   webGLPowerPreference: 2
   webGLWebAssemblyTable: 0
   webGLWebAssemblyBigInt: 0
   webGLCloseOnQuit: 0
   webWasm2023: 0
+  webEnableSubmoduleStrippingCompatibility: 0
   scriptingDefineSymbols:
     Standalone: UNITY_NETCODE_NATIVE_COLLECTION_SUPPORT
   additionalCompilerArguments:
@@ -866,3 +875,5 @@ PlayerSettings:
   insecureHttpOption: 0
   androidVulkanDenyFilterList: []
   androidVulkanAllowFilterList: []
+  androidVulkanDeviceFilterListAsset: {fileID: 0}
+  d3d12DeviceFilterListAsset: {fileID: 0}

testproject/ProjectSettings/ProjectVersion.txt

@@ -1,2 +1,2 @@
-m_EditorVersion: 6000.0.61f1
-m_EditorVersionWithRevision: 6000.0.61f1 (74a0adb02c31)
+m_EditorVersion: 6000.2.12f1
+m_EditorVersionWithRevision: 6000.2.12f1 (e89d5df0e333)