fix: Shutting down client or host/server and not being able to start the client or host/server within same game instance (#496)

* implement NetworkUpdateLoop

* remove AdvanceFrame() and FrameCount

* update player loop injection points and INetworkUpdateSystem API

* add xmldoc and notes

* add NetworkUpdateLoopTests

* implement NetworkUpdateLoop plain and mixed tests

* update xmldoc

* small meta fix

* replace old network update loop with new RFC-backed network update loop

* replace WaitForEndOfFrame with WaitUntil in tests

* comment 'default' for NetworkUpdateStage.Update enum value

* optimization & refactoring pass

* dictionary lookups instead of index access to arrays

* optimize array allocation

* Fix:  PostProcessing and Resetting NetworkingManager transport

This fixes two issues:
It prevents the postprocessing method ProcessScene from being run during scene transitions while in PlayMode within the editor.

This fixes the issue where the transport is no longer being polled for events upon shutting down the server,host, or client.

* fix: more robust fix for postprocessing bug

This prevents this post processing code from being executed when in PlayMode (editor and stand alone build)

* refactor: SceneSwitchProgress

This ended up not being needed in the end.
Reverting back to the original file.

* refactor: removing commented code and merge artifacts

Removing the commented out OnDisable function (the containing code was moved into the Shutdown method) and removed the Awake method (the containing code was moved into the Init method).

* refactor: merge artifacts

This hadn't fully registered the merge.

* refactor: Comments and Internal

Added some comments regarding where the RpcQueueContainer should be instantiated and shutdown.
Made the RpcQueueContainer internal, which also required me to make the QueueHistoryFrame internal and adjust some of the methods to match the same accessibility level.

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

Author: NoelStephensUnity

com.unity.multiplayer.mlapi/Editor/PostProcessScene.cs

@@ -11,14 +11,14 @@ public class NetworkScenePostProcess : MonoBehaviour
         [PostProcessScene(int.MaxValue)]
         public static void ProcessScene()
         {
-            //If we are in playmode (editor or stand alone) we do not want this to execute 
+            //If we are in playmode (editor or stand alone) we do not want this to execute
             if(Application.isPlaying)
             {
                 return;
-            }        
-        
+            }
+
             List<NetworkedObject> traverseSortedObjects = MonoBehaviour.FindObjectsOfType<NetworkedObject>().ToList();
-            
+
             traverseSortedObjects.Sort((x, y) =>
             {
                 List<int> xSiblingIndex = x.TraversedSiblingIndex();
@@ -28,38 +28,38 @@ public static void ProcessScene()
                 {
                     if (xSiblingIndex[0] < ySiblingIndex[0])
                         return -1;
-                    
+
                     if (xSiblingIndex[0] > ySiblingIndex[0])
                         return 1;
 
                     xSiblingIndex.RemoveAt(0);
                     ySiblingIndex.RemoveAt(0);
                 }
-                
+
                 return 0;
             });
 
             for (ulong i = 0; i < (ulong)traverseSortedObjects.Count; i++)
                 traverseSortedObjects[(int)i].NetworkedInstanceId = i;
         }
     }
-    
+
     internal static class PrefabHelpers
     {
         internal static List<int> TraversedSiblingIndex(this NetworkedObject networkedObject)
         {
             List<int> paths = new List<int>();
-            
+
             Transform transform = networkedObject.transform;
-            
+
             while (transform != null)
             {
                 paths.Add(transform.GetSiblingIndex());
                 transform = transform.parent;
             }
 
             paths.Reverse();
-            
+
             return paths;
         }
     }

com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs

@@ -70,7 +70,7 @@ public class NetworkingManager : MonoBehaviour, INetworkUpdateSystem
         static ProfilerMarker s_MLAPIClientSTDRPCQueued = new ProfilerMarker("MLAPIClientSTDRPCQueued");
         static ProfilerMarker s_InvokeRPC = new ProfilerMarker("InvokeRPC");
 #endif
-        public RpcQueueContainer rpcQueueContainer { get; private set; }
+        internal RpcQueueContainer rpcQueueContainer { get; private set; }
 
         /// <summary>
         /// A synchronized time, represents the time in seconds since the server application started. Is replicated across all clients
@@ -383,6 +383,27 @@ private void Init(bool server)
                 return;
             }
 
+
+            //This should never happen, but in the event that it does there should be (at a minimum) a unity error logged.
+            if(rpcQueueContainer != null)
+            {
+                UnityEngine.Debug.LogError("Init was invoked, but rpcQueueContainer was already initialized! (destroying previous instance)");
+                rpcQueueContainer.Shutdown();
+                rpcQueueContainer = null;
+            }
+
+            //The RpcQueueContainer must be initialized within the Init method ONLY
+            //It should ONLY be shutdown and destroyed in the Shutdown method (other than just above)
+            rpcQueueContainer = new RpcQueueContainer(false);
+
+            //Note: Since frame history is not being used, this is set to 0
+            //To test frame history, increase the number to (n) where n > 0
+            rpcQueueContainer.Initialize(0);
+
+            // Register INetworkUpdateSystem (always register this after rpcQueueContainer has been instantiated)
+            this.RegisterNetworkUpdate(NetworkUpdateStage.EarlyUpdate);
+            this.RegisterNetworkUpdate(NetworkUpdateStage.PreUpdate);
+
             try
             {
                 string pfx = null;
@@ -639,14 +660,6 @@ public void SetSingleton()
                 OnSingletonReady();
         }
 
-        private void Awake()
-        {
-            rpcQueueContainer = new RpcQueueContainer(false);
-            //Note: Since frame history is not being used, this is set to 0
-            //To test frame history, increase the number to (n) where n > 0
-            rpcQueueContainer.Initialize(0);
-        }
-
         private void OnEnable()
         {
             if (Singleton != null && Singleton != this)
@@ -655,28 +668,15 @@ private void OnEnable()
                 return;
             }
 
-            // Register INetworkUpdateSystem
-            this.RegisterNetworkUpdate(NetworkUpdateStage.EarlyUpdate);
-            this.RegisterNetworkUpdate(NetworkUpdateStage.PreUpdate);
-
             SetSingleton();
             if (DontDestroy)
                 DontDestroyOnLoad(gameObject);
             if (RunInBackground)
                 Application.runInBackground = true;
         }
 
-        private void OnDisable()
-        {
-            // Unregister INetworkUpdateSystem
-            this.UnregisterAllNetworkUpdates();
-        }
-
         private void OnDestroy()
         {
-            //NSS: This is ok to leave this check here
-            rpcQueueContainer?.Shutdown();
-
             if (Singleton != null && Singleton == this)
             {
                 Singleton = null;
@@ -687,6 +687,17 @@ private void OnDestroy()
         public void Shutdown()
         {
             if (NetworkLog.CurrentLogLevel <= LogLevel.Developer) NetworkLog.LogInfo("Shutdown()");
+
+            // Unregister INetworkUpdateSystem before shutting down the RpcQueueContainer
+            this.UnregisterAllNetworkUpdates();
+
+            //If an instance of the RpcQueueContainer is still around, then shut it down and remove the reference
+            if (rpcQueueContainer != null)
+            {
+                rpcQueueContainer.Shutdown();
+                rpcQueueContainer = null;
+            }
+
             NetworkProfiler.Stop();
             IsListening = false;
             IsServer = false;
@@ -699,11 +710,7 @@ public void Shutdown()
             if (NetworkConfig != null && NetworkConfig.NetworkTransport != null)
                 NetworkConfig.NetworkTransport.Shutdown();
 
-            if (rpcQueueContainer != null)
-            {
-                rpcQueueContainer.Shutdown();
-                rpcQueueContainer = null;
-            }
+
         }
 
         // INetworkUpdateSystem
@@ -1234,7 +1241,7 @@ private static void ReceiveCallback(BitStream messageStream, RpcQueueContainer.Q
         /// Called when an inbound queued RPC is invoked
         /// </summary>
         /// <param name="queueItem">frame queue item to invoke</param>
-        public static void InvokeRpc(RpcFrameQueueItem queueItem)
+        internal static void InvokeRpc(RpcFrameQueueItem queueItem)
         {
 #if DEVELOPMENT_BUILD || UNITY_EDITOR
             s_InvokeRPC.Begin();

com.unity.multiplayer.mlapi/Runtime/Messaging/RpcQueue/QueueHistoryFrame.cs

@@ -159,7 +159,7 @@ private RpcFrameQueueItem GetCurrentQueueItem()
         /// If none are remaining, then it returns a queue item type of NONE
         /// </summary>
         /// <returns>FrameQueueItem</returns>
-        public RpcFrameQueueItem GetNextQueueItem()
+        internal RpcFrameQueueItem GetNextQueueItem()
         {
             queueStream.Position = queueItemOffsets[m_QueueItemOffsetIndex];
             m_QueueItemOffsetIndex++;
@@ -178,7 +178,7 @@ public RpcFrameQueueItem GetNextQueueItem()
         /// This will reset the frame's stream indices and add a new stream and stream writer to the m_CurrentQueueItem instance.
         /// </summary>
         /// <returns>FrameQueueItem</returns>
-        public RpcFrameQueueItem GetFirstQueueItem()
+        internal RpcFrameQueueItem GetFirstQueueItem()
         {
             if (queueStream.Position > 0)
             {

com.unity.multiplayer.mlapi/Runtime/Messaging/RpcQueue/RpcFrameQueueItem.cs

@@ -12,7 +12,7 @@ namespace  MLAPI.Messaging
     /// NOTE: This structure will change in the near future and is in a state of flux.
     /// This will include removing specific properties or changing property types
     /// </summary>
-    public struct RpcFrameQueueItem
+    internal struct RpcFrameQueueItem
     {
         public NetworkUpdateStage                        updateStage;
         public RpcQueueContainer.QueueItemType           queueItemType;
@@ -27,4 +27,4 @@ public struct RpcFrameQueueItem
         public PooledBitStream                           itemStream;
         public ArraySegment<byte>                        messageData;
     }
-}
+}

com.unity.multiplayer.mlapi/Runtime/Messaging/RpcQueue/RpcQueueContainer.cs

@@ -11,7 +11,7 @@ namespace MLAPI.Messaging
     /// RpcQueueContainer
     /// Handles the management of an Rpc Queue
     /// </summary>
-    public class RpcQueueContainer : INetworkUpdateSystem
+    internal class RpcQueueContainer : INetworkUpdateSystem
     {
         private const int k_MinQueueHistory = 2; //We need a minimum of 2 queue history buffers in order to properly handle looping back Rpcs when a host
 
@@ -571,7 +571,6 @@ public QueueHistoryFrame GetQueueHistoryFrame(QueueHistoryFrame.QueueFrameType f
             return QueueHistory[frameType][StreamBufferIndex][updateStage];
         }
 
-
         /// <summary>
         /// LoopbackSendFrame
         /// Will copy the contents of the current outbound QueueHistoryFrame to the current inbound QueueHistoryFrame
@@ -710,16 +709,16 @@ private void ClearParameters()
         /// </summary>
         public void Shutdown()
         {
-            //We need to make sure all internal messages (i.e. object destroy) are sent
-            m_RpcQueueProcessor.InternalMessagesSendAndFlush();
-
             //As long as this instance is using the pre-defined update stages
             if (!m_ProcessUpdateStagesExternally)
             {
                 //Remove ourself from the network loop update system
                 this.UnregisterAllNetworkUpdates();
             }
 
+            //We need to make sure all internal messages (i.e. object destroy) are sent
+            m_RpcQueueProcessor.InternalMessagesSendAndFlush();
+
             //Dispose of any readers and writers
             foreach (KeyValuePair<QueueHistoryFrame.QueueFrameType, Dictionary<int, Dictionary<NetworkUpdateStage, QueueHistoryFrame>>> queueHistorySection in QueueHistory)
             {

com.unity.multiplayer.mlapi/Runtime/SceneManagement/SceneSwitchProgress.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.ComponentModel;
 using UnityEngine;
@@ -55,7 +55,7 @@ public class SceneSwitchProgress
         /// The callback invoked when a client is done loading the scene.
         /// </summary>
         public event OnClientLoadedSceneDelegate OnClientLoadedScene;
-        
+
         internal Guid guid { get; } = Guid.NewGuid();
 
         private Coroutine timeOutCoroutine;
@@ -110,6 +110,5 @@ internal void SetTimedOut()
                     OnComplete.Invoke(true);
             }
         }
-
     }
 }