fix: Restarting UnityTransport after it failed to start [MTT-3452] (#2220)

* Add test for transport restart failure

* Fix restart after transport failure

* Add CHANGELOG entry

* Add PR number to CHANGELOG entry

Author: simon-lemay-unity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -21,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue #1924 where `UnityTransport` would fail to restart after a first failure (even if what caused the initial failure was addressed). (#2220)
 - Fixed issue where `NetworkTransform.SetStateServerRpc` and `NetworkTransform.SetStateClientRpc` were not honoring local vs world space settings when applying the position and rotation. (#2203)
 - Fixed ILPP `TypeLoadException` on WebGL on MacOS Editor and potentially other platforms. (#2199)
 - Implicit conversion of NetworkObjectReference to GameObject will now return null instead of throwing an exception if the referenced object could not be found (i.e., was already despawned) (#2158)

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

@@ -1292,9 +1292,9 @@ public override bool StartClient()
             }
 
             var succeeded = ClientBindAndConnect();
-            if (!succeeded)
+            if (!succeeded && m_Driver.IsCreated)
             {
-                Shutdown();
+                m_Driver.Dispose();
             }
             return succeeded;
         }
@@ -1319,16 +1319,16 @@ public override bool StartServer()
             {
                 case ProtocolType.UnityTransport:
                     succeeded = ServerBindAndListen(ConnectionData.ListenEndPoint);
-                    if (!succeeded)
+                    if (!succeeded && m_Driver.IsCreated)
                     {
-                        Shutdown();
+                        m_Driver.Dispose();
                     }
                     return succeeded;
                 case ProtocolType.RelayUnityTransport:
                     succeeded = StartRelayServer();
-                    if (!succeeded)
+                    if (!succeeded && m_Driver.IsCreated)
                     {
-                        Shutdown();
+                        m_Driver.Dispose();
                     }
                     return succeeded;
                 default:

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

@@ -1,14 +1,15 @@
 using NUnit.Framework;
 using Unity.Netcode.Transports.UTP;
 using UnityEngine;
+using UnityEngine.TestTools;
 
 namespace Unity.Netcode.EditorTests
 {
     public class UnityTransportTests
     {
         // Check that starting a server doesn't immediately result in faulted tasks.
         [Test]
-        public void BasicInitServer()
+        public void UnityTransport_BasicInitServer()
         {
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
@@ -20,7 +21,7 @@ public void BasicInitServer()
 
         // Check that starting a client doesn't immediately result in faulted tasks.
         [Test]
-        public void BasicInitClient()
+        public void UnityTransport_BasicInitClient()
         {
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
@@ -32,7 +33,7 @@ public void BasicInitClient()
 
         // Check that we can't restart a server.
         [Test]
-        public void NoRestartServer()
+        public void UnityTransport_NoRestartServer()
         {
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
@@ -45,7 +46,7 @@ public void NoRestartServer()
 
         // Check that we can't restart a client.
         [Test]
-        public void NoRestartClient()
+        public void UnityTransport_NoRestartClient()
         {
             UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
             transport.Initialize();
@@ -58,7 +59,7 @@ public void NoRestartClient()
 
         // Check that we can't start both a server and client on the same transport.
         [Test]
-        public void NotBothServerAndClient()
+        public void UnityTransport_NotBothServerAndClient()
         {
             UnityTransport transport;
 
@@ -80,5 +81,24 @@ public void NotBothServerAndClient()
 
             transport.Shutdown();
         }
+
+        // Check that restarting after failure succeeds.
+        [Test]
+        public void UnityTransport_RestartSucceedsAfterFailure()
+        {
+            UnityTransport transport = new GameObject().AddComponent<UnityTransport>();
+            transport.Initialize();
+
+            transport.SetConnectionData("127.0.0.", 4242);
+            Assert.False(transport.StartServer());
+
+            LogAssert.Expect(LogType.Error, "Invalid network endpoint: 127.0.0.:4242.");
+            LogAssert.Expect(LogType.Error, "Server failed to bind");
+
+            transport.SetConnectionData("127.0.0.1", 4242);
+            Assert.True(transport.StartServer());
+
+            transport.Shutdown();
+        }
     }
 }