Worker connector updates & refactoring (#271)

Author: Jamie Brynes

CHANGELOG.md

@@ -5,6 +5,8 @@
 ### Changed
 
 - `MobileClient` now uses the `ModularKcp` networking type rather than `Kcp`. [#268](https://github.com/spatialos/gdk-for-unity-fps-starter-project/pull/268).
+- The `MapBuilder` is now using a `Task` based API instead of a coroutine based one. [#271](https://github.com/spatialos/gdk-for-unity-fps-starter-project/pull/271)
+- The `WorkerConnector` implementations have been refactored and simplified. [#271](https://github.com/spatialos/gdk-for-unity-fps-starter-project/pull/271)
 
 ### Removed
 

gdk.pinned

@@ -1 +1 @@
-develop e91ae1c92502abebe6e7d94b6047914dd43beea6
+develop 0266b65afbbd6a70ff777148b6368ec5f1d83b82

workers/unity/Assets/Fps/Scripts/Editor/WorldTiles/MapBuilderVisualisationWindow.cs

@@ -70,7 +70,7 @@ public void OnGUI()
                         SetupMapBuilder();
                     }
 
-                    UnwindCoroutine(mapBuilder.CleanAndBuild(layerCount, seed));
+                    mapBuilder.CleanAndBuild(layerCount, seed).Wait();
                 }
             }
 
@@ -99,16 +99,5 @@ private int GetTotalTilesFromLayers(int layers)
         {
             return Mathf.RoundToInt(Mathf.Pow(layers * 2, 2));
         }
-
-        private void UnwindCoroutine(IEnumerator enumerator)
-        {
-            while (enumerator.MoveNext())
-            {
-                if (enumerator.Current is IEnumerator nestedEnumerator)
-                {
-                    UnwindCoroutine(nestedEnumerator);
-                }
-            }
-        }
     }
 }

workers/unity/Assets/Fps/Scripts/SimulatedPlayer/SimulatedPlayerDriver.cs

@@ -62,7 +62,7 @@ private void Start()
             agent.updateUpAxis = false;
             agent.Warp(transform.position);
             anchorPoint = transform.position;
-            worldBounds = coordinator.GetWorldBounds();
+            worldBounds = coordinator.Bounds;
         }
 
         private void OnEnable()

workers/unity/Assets/Fps/Scripts/WorkerConnectors/ClientWorkerConnector.cs

@@ -11,22 +11,34 @@ namespace Fps.WorkerConnectors
 {
     public class ClientWorkerConnector : WorkerConnectorBase
     {
-        protected string deployment;
-
         private string playerName;
         private bool isReadyToSpawn;
         private bool wantsSpawn;
         private Action<PlayerCreator.CreatePlayer.ReceivedResponse> onPlayerResponse;
-        private AdvancedEntityPipeline entityPipeline;
 
         public bool HasConnected => Worker != null;
 
         public event Action OnLostPlayerEntity;
 
-        public async void Connect(string deployment = "")
+        public void Start()
+        {
+            Application.targetFrameRate = 60;
+        }
+
+        public async void Connect()
         {
-            this.deployment = deployment.Trim();
-            await AttemptConnect();
+            try
+            {
+                await Connect(GetConnectionHandlerBuilder(), new ForwardingDispatcher());
+                await LoadWorld();
+                isReadyToSpawn = true;
+            }
+            catch (Exception e)
+            {
+                Debug.LogException(e);
+                Dispose();
+                Destroy(gameObject);
+            }
         }
 
         public void SpawnPlayer(string playerName, Action<PlayerCreator.CreatePlayer.ReceivedResponse> onPlayerResponse)
@@ -36,11 +48,6 @@ public void SpawnPlayer(string playerName, Action<PlayerCreator.CreatePlayer.Rec
             wantsSpawn = true;
         }
 
-        public void DisconnectPlayer()
-        {
-            StartCoroutine(PrepareDestroy());
-        }
-
         protected virtual string GetAuthPlayerPrefabPath()
         {
             return "Prefabs/UnityClient/Authoritative/Player";
@@ -51,7 +58,7 @@ protected virtual string GetNonAuthPlayerPrefabPath()
             return "Prefabs/UnityClient/NonAuthoritative/Player";
         }
 
-        protected override IConnectionHandlerBuilder GetConnectionHandlerBuilder()
+        protected virtual IConnectionHandlerBuilder GetConnectionHandlerBuilder()
         {
             IConnectionFlow connectionFlow;
             var connectionParams = CreateConnectionParameters(WorkerUtils.UnityClient);
@@ -91,33 +98,19 @@ protected override void HandleWorkerConnectionEstablished()
             PlayerLifecycleHelper.AddClientSystems(world, autoRequestPlayerCreation: false);
             PlayerLifecycleConfig.MaxPlayerCreationRetries = 0;
 
-            entityPipeline = new AdvancedEntityPipeline(Worker, GetAuthPlayerPrefabPath(), GetNonAuthPlayerPrefabPath());
+            var entityPipeline = new AdvancedEntityPipeline(Worker, GetAuthPlayerPrefabPath(), GetNonAuthPlayerPrefabPath());
             entityPipeline.OnRemovedAuthoritativePlayer += RemovingAuthoritativePlayer;
 
             // Set the Worker gameObject to the ClientWorker so it can access PlayerCreater reader/writers
             GameObjectCreationHelper.EnableStandardGameObjectCreation(world, entityPipeline, gameObject);
-
-            base.HandleWorkerConnectionEstablished();
         }
 
         private void RemovingAuthoritativePlayer()
         {
-            Debug.LogError($"Player entity got removed while still being connected. Disconnecting...");
+            Debug.LogWarning($"Player entity got removed while still being connected. Disconnecting...");
             OnLostPlayerEntity?.Invoke();
         }
 
-        protected override void HandleWorkerConnectionFailure(string errorMessage)
-        {
-            Debug.LogError($"Connection failed: {errorMessage}");
-            Destroy(gameObject);
-        }
-
-        protected override IEnumerator LoadWorld()
-        {
-            yield return base.LoadWorld();
-            isReadyToSpawn = true;
-        }
-
         private void Update()
         {
             if (wantsSpawn && isReadyToSpawn)
@@ -127,22 +120,6 @@ private void Update()
             }
         }
 
-        public override void Dispose()
-        {
-            if (entityPipeline != null)
-            {
-                entityPipeline.OnRemovedAuthoritativePlayer -= RemovingAuthoritativePlayer;
-            }
-
-            base.Dispose();
-        }
-
-        private IEnumerator PrepareDestroy()
-        {
-            yield return DeferredDisposeWorker();
-            Destroy(gameObject);
-        }
-
         private void SendRequest()
         {
             var serializedArgs = Encoding.ASCII.GetBytes(playerName);

workers/unity/Assets/Fps/Scripts/WorkerConnectors/GameLogicWorkerConnector.cs

@@ -15,13 +15,23 @@ public class GameLogicWorkerConnector : WorkerConnectorBase
     {
         public bool DisableRenderers = true;
 
-        protected override async void Start()
+        protected async void Start()
         {
-            base.Start();
-            await AttemptConnect();
+            Application.targetFrameRate = 60;
+
+            await Connect(GetConnectionHandlerBuilder(), new ForwardingDispatcher());
+            await LoadWorld();
+
+            if (DisableRenderers)
+            {
+                foreach (var childRenderer in LevelInstance.GetComponentsInChildren<Renderer>())
+                {
+                    childRenderer.enabled = false;
+                }
+            }
         }
 
-        protected override IConnectionHandlerBuilder GetConnectionHandlerBuilder()
+        private IConnectionHandlerBuilder GetConnectionHandlerBuilder()
         {
             IConnectionFlow connectionFlow;
             ConnectionParameters connectionParameters;
@@ -61,21 +71,6 @@ protected override void HandleWorkerConnectionEstablished()
             // Health
             world.GetOrCreateSystem<ServerHealthModifierSystem>();
             world.GetOrCreateSystem<HealthRegenSystem>();
-
-            base.HandleWorkerConnectionEstablished();
-        }
-
-        protected override IEnumerator LoadWorld()
-        {
-            yield return base.LoadWorld();
-
-            if (DisableRenderers)
-            {
-                foreach (var childRenderer in LevelInstance.GetComponentsInChildren<Renderer>())
-                {
-                    childRenderer.enabled = false;
-                }
-            }
         }
     }
 }

workers/unity/Assets/Fps/Scripts/WorkerConnectors/SimulatedPlayerCoordinatorWorkerConnector.cs

@@ -30,6 +30,8 @@ public class SimulatedPlayerCoordinatorWorkerConnector : WorkerConnectorBase
         public int MaxSimulatedPlayerCount = 1;
         public int SimulatedPlayerCreationInterval = 5;
 
+        public Bounds Bounds { get; private set; }
+
         private readonly Dictionary<EntityId, List<GameObject>> proxies = new Dictionary<EntityId, List<GameObject>>();
         private readonly List<EntityId> localSimulatedPlayers = new List<EntityId>();
 
@@ -40,10 +42,11 @@ public class SimulatedPlayerCoordinatorWorkerConnector : WorkerConnectorBase
         private CancellationTokenSource tokenSource;
         private int TimeBetweenCycleSecs = 2;
 
-        protected override async void Start()
+        protected async void Start()
         {
-            var args = CommandLineArgs.FromCommandLine();
+            Application.targetFrameRate = 60;
 
+            var args = CommandLineArgs.FromCommandLine();
             var deploymentName = string.Empty;
 
             if (args.TryGetCommandLineValue(DeploymentNameFlag, ref deploymentName))
@@ -56,45 +59,22 @@ protected override async void Start()
                 connectPlayersWithDevAuth = false;
             }
 
-            Application.targetFrameRate = 60;
-            await AttemptConnect();
-        }
+            await Connect(GetConnectionHandlerBuilder(), new ForwardingDispatcher());
 
-        protected override IConnectionHandlerBuilder GetConnectionHandlerBuilder()
-        {
-            IConnectionFlow connectionFlow;
-            ConnectionParameters connectionParameters;
-
-            var workerId = CreateNewWorkerId(WorkerUtils.SimulatedPlayerCoordinator);
-
-            if (Application.isEditor)
-            {
-                connectionFlow = new ReceptionistFlow(workerId);
-                connectionParameters = CreateConnectionParameters(WorkerUtils.SimulatedPlayerCoordinator);
-            }
-            else
+            if (SimulatedPlayerWorkerConnector == null)
             {
-                connectionFlow = new ReceptionistFlow(workerId, new CommandLineConnectionFlowInitializer());
-                connectionParameters = CreateConnectionParameters(WorkerUtils.SimulatedPlayerCoordinator,
-                    new CommandLineConnectionParameterInitializer());
+                Worker.LogDispatcher.HandleLog(LogType.Error, new LogEvent("Did not find a SimulatedPlayerWorkerConnector GameObject."));
+                return;
             }
 
-            return new SpatialOSConnectionHandlerBuilder()
-                .SetConnectionFlow(connectionFlow)
-                .SetConnectionParameters(connectionParameters);
+            Bounds = await GetWorldBounds();
+
+            await LoadWorld();
+            await ConnectSimulatedPlayers();
         }
 
-        protected override async void HandleWorkerConnectionEstablished()
+        private async Task ConnectSimulatedPlayers()
         {
-            base.HandleWorkerConnectionEstablished();
-
-            Worker.World.GetOrCreateSystem<MetricSendSystem>();
-
-            if (SimulatedPlayerWorkerConnector == null)
-            {
-                return;
-            }
-
             tokenSource = new CancellationTokenSource();
 
             try
@@ -103,7 +83,7 @@ protected override async void HandleWorkerConnectionEstablished()
                 // to change the parameters..
                 if (connectPlayersWithDevAuth)
                 {
-                    await WaitForWorkerFlags(flagDevAuthTokenId, flagTargetDeployment, flagClientCount,
+                    await WaitForWorkerFlags(tokenSource.Token, flagDevAuthTokenId, flagTargetDeployment, flagClientCount,
                         flagCreationInterval);
 
                     var simulatedPlayerDevAuthTokenId = Worker.GetWorkerFlag(flagDevAuthTokenId);
@@ -142,30 +122,42 @@ await Monitor(MaxSimulatedPlayerCount, SimulatedPlayerCreationInterval,
             }
         }
 
-        public override void Dispose()
+        private IConnectionHandlerBuilder GetConnectionHandlerBuilder()
         {
-            tokenSource?.Cancel();
-            tokenSource?.Dispose();
-            tokenSource = null;
+            IConnectionFlow connectionFlow;
+            ConnectionParameters connectionParameters;
 
-            base.Dispose();
+            var workerId = CreateNewWorkerId(WorkerUtils.SimulatedPlayerCoordinator);
+
+            if (Application.isEditor)
+            {
+                connectionFlow = new ReceptionistFlow(workerId);
+                connectionParameters = CreateConnectionParameters(WorkerUtils.SimulatedPlayerCoordinator);
+            }
+            else
+            {
+                connectionFlow = new ReceptionistFlow(workerId, new CommandLineConnectionFlowInitializer());
+                connectionParameters = CreateConnectionParameters(WorkerUtils.SimulatedPlayerCoordinator,
+                    new CommandLineConnectionParameterInitializer());
+            }
+
+            return new SpatialOSConnectionHandlerBuilder()
+                .SetConnectionFlow(connectionFlow)
+                .SetConnectionParameters(connectionParameters);
         }
 
-        // Wait for a number of worker flags to be present for this worker. Will spin forever otherwise.
-        private async Task WaitForWorkerFlags(params string[] flagKeys)
+        protected override void HandleWorkerConnectionEstablished()
         {
-            var token = tokenSource.Token;
+            Worker.World.GetOrCreateSystem<MetricSendSystem>();
+        }
 
-            while (flagKeys.Any(key => string.IsNullOrEmpty(Worker.GetWorkerFlag(key))))
-            {
-                if (token.IsCancellationRequested)
-                {
-                    throw new TaskCanceledException();
-                }
+        public override void Dispose()
+        {
+            tokenSource?.Cancel();
+            tokenSource?.Dispose();
+            tokenSource = null;
 
-                Worker.LogDispatcher.HandleLog(LogType.Log, new LogEvent("Waiting for required worker flags.."));
-                await Task.Delay(TimeSpan.FromSeconds(TimeBetweenCycleSecs), token);
-            }
+            base.Dispose();
         }
 
         // Update worker flags if they have changed.
@@ -221,7 +213,17 @@ private async Task CreateSimPlayer(int delayInterval, Func<SimulatedPlayerWorker
             var simPlayer = Instantiate(SimulatedPlayerWorkerConnector, transform.position, transform.rotation);
             var connector = simPlayer.GetComponent<SimulatedPlayerWorkerConnector>();
 
-            await connectMethod(connector);
+            try
+            {
+                await connectMethod(connector);
+            }
+            catch (Exception e)
+            {
+                Worker.LogDispatcher.HandleLog(LogType.Exception, new LogEvent("Failed to connect simulated player").WithException(e));
+                Destroy(simPlayer);
+                return;
+            }
+
             connector.SpawnPlayer(simulatedPlayerConnectors.Count);
 
             simulatedPlayerConnectors.Add(simPlayer);
@@ -307,9 +309,9 @@ public void UnregisterLocalSimulatedPlayer(EntityId entityId, GameObject prefab)
             }
         }
 
-        public Bounds GetWorldBounds()
+        public async Task<Bounds> GetWorldBounds()
         {
-            var worldSize = GetWorldSize();
+            var worldSize = await GetWorldSize();
             return new Bounds(Worker.Origin, MapBuilder.GetWorldDimensions(worldSize));
         }