Better gameobject initialization (#1333)

Paul Balaji · web-flow · commit 9c0b93fddff0 · 2020-03-24T17:35:09.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,8 +12,10 @@
 - `CustomSpatialOSSendSystem` is no longer available. [#1308](https://github.com/spatialos/gdk-for-unity/pull/1308)
 - The Player Lifecycle feature module now provides an `EntityId` in its `CreatePlayerEntityTemplate` callback. [#1315](https://github.com/spatialos/gdk-for-unity/pull/1315)
     - You will have to change your callback from `(string clientWorkerId, byte[] serializedArguments)` to `(EntityId entityId, string clientWorkerId, byte[] serializedArguments)`.
-- Added the `ComponentType[] MiniumComponentTypes { get; }` property to `IEntityGameObjectCreator`. [#1330](https://github.com/spatialos/gdk-for-unity/pull/1330)
-    - You will have to define the minimum set of components required on an entity to trigger the `OnEntityCreated` method on your custom GameObject creator.
+- Added the `PopulateEntityTypeExpectations` method to `IEntityGameObjectCreator`. [#1333](https://github.com/spatialos/gdk-for-unity/pull/1333)
+    - Use this method to define the set of components expected on an entity to be able create GameObjects for a given entity type.
+- Added `string entityType` as an argument to  `IEntityGameObjectCreator.OnEntityCreated`. [#1333](https://github.com/spatialos/gdk-for-unity/pull/1333)
+    - This means that your entities must have the `Metadata` component to use the GameObject Creation Feature Module.
 
 ### Added
 
diff --git a/UPGRADE_GUIDE.md b/UPGRADE_GUIDE.md
@@ -29,21 +29,65 @@ public static EntityTemplate Player(EntityId entityId, string workerId, byte[] a
 }
 ```
 
-### The `IEntityGameObjectCreator` now requires the `ComponentType[] MiniumComponentTypes { get; }` property
+### `IEntityGameObjectCreator` changes
 
-If you have written custom GameObject creators implementing `IEntityGameObjectCreator`, you will have to define the minimum set of components required on an entity to trigger the `OnEntityCreated` method.
+#### Implement `PopulateEntityTypeExpectations` method
 
-For example, the following has been added to the `GameObjectCreatorFromMetadata` class:
+If you have written custom GameObject creators implementing `IEntityGameObjectCreator`, you will now have to implement a `void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations)` method.
+
+The `EntityTypeExpectations` class provides two public methods for defining a set of components expected on an entity to be able create GameObjects for a given entity type:
+
+- `void RegisterDefault(IEnumerable<Type> defaultComponentTypes = null)`
+- `void RegisterEntityType(string entityType, IEnumerable<Type> expectedComponentTypes = null)`
+
+For example, the `GameObjectCreatorFromMetadata` class implements the method like so:
 
 ```csharp
-public ComponentType[] MinimumComponentTypes { get; } =
+public void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations)
 {
-    ComponentType.ReadOnly<Metadata.Component>(),
-    ComponentType.ReadOnly<Position.Component>()
-};
+    entityTypeExpectations.RegisterDefault(new[]
+    {
+        typeof(Position.Component)
+    });
+}
 ```
 
-> You will need to add `using Unity.Entities;` to the top of the file and reference it in the assembly that your custom GameObject creator is in.
+The `AdvancedEntityPipeline` in the FPS Starter Project makes use of both public methods on the `EntityTypeExpectations` class. This is done in order to wait for specific components when creating "Player" GameObjects, and a different set of components for any other entity type:
+
+```csharp
+public void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations)
+{
+    entityTypeExpectations.RegisterEntityType(PlayerEntityType, new[]
+    {
+        typeof(OwningWorker.Component), typeof(ServerMovement.Component)
+    });
+
+    fallback.PopulateEntityTypeExpectations(entityTypeExpectations);
+}
+``
+
+#### Add `string entityType` as argument to `OnEntityCreated`
+
+The `EntityType` available the `Metadata` component is now provided as an argument when calling `OnEntityCreated` on your GameObject creator.
+
+> This means that your entities must have the `Metadata` component to use the GameObject Creation Feature Module.
+
+The `AdvancedEntityPipeline` in the FPS Starter Project makes use of it like so:
+
+```csharp
+public void OnEntityCreated(string entityType, SpatialOSEntity entity, EntityGameObjectLinker linker)
+{
+    switch (entityType)
+    {
+        case PlayerEntityType:
+            CreatePlayerGameObject(entity, linker);
+            break;
+        default:
+            fallback.OnEntityCreated(entityType, entity, linker);
+            break;
+    }
+}
+```
 
 ## From `0.3.2` to `0.3.3`
 
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/EntityTypeExpectations.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/EntityTypeExpectations.cs
@@ -0,0 +1,60 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Unity.Entities;
+
+namespace Improbable.Gdk.GameObjectCreation
+{
+    /// <summary>
+    ///     Holds mappings from entity types to the set of components expected before creating their GameObjects.
+    /// </summary>
+    public class EntityTypeExpectations
+    {
+        private ComponentType[] defaultExpectation = new ComponentType[] { };
+
+        private readonly Dictionary<string, ComponentType[]> entityExpectations
+            = new Dictionary<string, ComponentType[]>();
+
+        public void RegisterDefault(IEnumerable<Type> defaultComponentTypes = null)
+        {
+            if (defaultComponentTypes == null)
+            {
+                defaultExpectation = new ComponentType[] { };
+            }
+            else
+            {
+                defaultExpectation = defaultComponentTypes
+                    .Select(type => new ComponentType(type, ComponentType.AccessMode.ReadOnly))
+                    .ToArray();
+            }
+        }
+
+        public void RegisterEntityType(string entityType, IEnumerable<Type> expectedComponentTypes = null)
+        {
+            ComponentType[] expectedTypes;
+
+            if (expectedComponentTypes == null)
+            {
+                expectedTypes = new ComponentType[] { };
+            }
+            else
+            {
+                expectedTypes = expectedComponentTypes
+                    .Select(type => new ComponentType(type, ComponentType.AccessMode.ReadOnly))
+                    .ToArray();
+            }
+
+            entityExpectations.Add(entityType, expectedTypes);
+        }
+
+        internal ComponentType[] GetExpectedTypes(string entityType)
+        {
+            if (!entityExpectations.TryGetValue(entityType, out var types))
+            {
+                return defaultExpectation;
+            }
+
+            return types;
+        }
+    }
+}
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/EntityTypeExpectations.cs.meta b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/EntityTypeExpectations.cs.meta
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/GameObjectCreatorFromMetadata.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/GameObjectCreatorFromMetadata.cs
@@ -3,7 +3,6 @@
 using System.IO;
 using Improbable.Gdk.Core;
 using Improbable.Gdk.Subscriptions;
-using Unity.Entities;
 using UnityEngine;
 using Object = UnityEngine.Object;
 
@@ -17,39 +16,32 @@ private readonly Dictionary<string, GameObject> cachedPrefabs
         private readonly string workerType;
         private readonly Vector3 workerOrigin;
 
-        private readonly ILogDispatcher logger;
-
         private readonly Dictionary<EntityId, GameObject> entityIdToGameObject = new Dictionary<EntityId, GameObject>();
 
         private readonly Type[] componentsToAdd =
         {
-            typeof(Transform),
-            typeof(Rigidbody),
-            typeof(MeshRenderer)
-        };
-
-        public ComponentType[] MinimumComponentTypes { get; } =
-        {
-            ComponentType.ReadOnly<Metadata.Component>(),
-            ComponentType.ReadOnly<Position.Component>()
+            typeof(Transform), typeof(Rigidbody), typeof(MeshRenderer)
         };
 
         public GameObjectCreatorFromMetadata(string workerType, Vector3 workerOrigin, ILogDispatcher logger)
         {
             this.workerType = workerType;
             this.workerOrigin = workerOrigin;
-            this.logger = logger;
         }
 
-        public void OnEntityCreated(SpatialOSEntity entity, EntityGameObjectLinker linker)
+        public void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations)
         {
-            if (!entity.TryGetComponent<Metadata.Component>(out var metadata) ||
-                !entity.TryGetComponent<Position.Component>(out var spatialOSPosition))
+            entityTypeExpectations.RegisterDefault(new[]
             {
-                return;
-            }
+                typeof(Position.Component)
+            });
+        }
+
+        public void OnEntityCreated(string entityType, SpatialOSEntity entity, EntityGameObjectLinker linker)
+        {
+            var spatialOSPosition = entity.GetComponent<Position.Component>();
 
-            var prefabName = metadata.EntityType;
+            var prefabName = entityType;
             var position = spatialOSPosition.Coords.ToUnityVector() + workerOrigin;
 
             if (!cachedPrefabs.TryGetValue(prefabName, out var prefab))
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/GameObjectInitializationSystem.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/GameObjectInitializationSystem.cs
@@ -1,4 +1,4 @@
-using System.Linq;
+using System;
 using Improbable.Gdk.Core;
 using Improbable.Gdk.Subscriptions;
 using Unity.Entities;
@@ -26,23 +26,12 @@ internal class GameObjectInitializationSystem : ComponentSystem
         private EntityQuery newEntitiesQuery;
         private EntityQuery removedEntitiesQuery;
 
-        private ComponentType[] minimumComponentSet = new[]
-        {
-            ComponentType.ReadOnly<SpatialEntityId>()
-        };
+        private readonly EntityTypeExpectations entityTypeExpectations = new EntityTypeExpectations();
 
         public GameObjectInitializationSystem(IEntityGameObjectCreator gameObjectCreator, GameObject workerGameObject)
         {
             this.gameObjectCreator = gameObjectCreator;
             this.workerGameObject = workerGameObject;
-
-            var minCreatorComponentSet = gameObjectCreator.MinimumComponentTypes;
-            if (minCreatorComponentSet != null)
-            {
-                minimumComponentSet = minimumComponentSet
-                    .Concat(minCreatorComponentSet)
-                    .ToArray();
-            }
         }
 
         protected override void OnCreate()
@@ -58,6 +47,11 @@ protected override void OnCreate()
                 Linker.LinkGameObjectToSpatialOSEntity(new EntityId(0), workerGameObject);
             }
 
+            var minimumComponentSet = new[]
+            {
+                ComponentType.ReadOnly<SpatialEntityId>(), ComponentType.ReadOnly<Metadata.Component>()
+            };
+
             newEntitiesQuery = GetEntityQuery(new EntityQueryDesc()
             {
                 All = minimumComponentSet,
@@ -69,6 +63,8 @@ protected override void OnCreate()
                 All = new[] { ComponentType.ReadOnly<GameObjectInitSystemStateComponent>() },
                 None = minimumComponentSet
             });
+
+            gameObjectCreator.PopulateEntityTypeExpectations(entityTypeExpectations);
         }
 
         protected override void OnDestroy()
@@ -87,24 +83,68 @@ protected override void OnDestroy()
 
         protected override void OnUpdate()
         {
-            Entities.With(newEntitiesQuery).ForEach((Entity entity, ref SpatialEntityId spatialEntityId) =>
+            if (!newEntitiesQuery.IsEmptyIgnoreFilter)
             {
-                gameObjectCreator.OnEntityCreated(new SpatialOSEntity(entity, EntityManager), Linker);
-                PostUpdateCommands.AddComponent(entity, new GameObjectInitSystemStateComponent
-                {
-                    EntityId = spatialEntityId.EntityId
-                });
-            });
+                ProcessNewEntities();
+            }
+
+            if (!removedEntitiesQuery.IsEmptyIgnoreFilter)
+            {
+                ProcessRemovedEntities();
+            }
 
+            Linker.FlushCommandBuffer();
+        }
+
+        private void ProcessRemovedEntities()
+        {
             Entities.With(removedEntitiesQuery).ForEach((ref GameObjectInitSystemStateComponent state) =>
             {
                 Linker.UnlinkAllGameObjectsFromEntityId(state.EntityId);
-                gameObjectCreator.OnEntityRemoved(state.EntityId);
-            });
 
-            Linker.FlushCommandBuffer();
+                try
+                {
+                    gameObjectCreator.OnEntityRemoved(state.EntityId);
+                }
+                catch (Exception e)
+                {
+                    Debug.LogException(e);
+                }
+            });
 
             EntityManager.RemoveComponent<GameObjectInitSystemStateComponent>(removedEntitiesQuery);
         }
+
+        private void ProcessNewEntities()
+        {
+            Entities.With(newEntitiesQuery).ForEach(
+                (Entity entity, ref SpatialEntityId spatialEntityId, ref Metadata.Component metadata) =>
+                {
+                    var entityType = metadata.EntityType;
+                    var expectedTypes = entityTypeExpectations.GetExpectedTypes(entityType);
+
+                    foreach (var expectedType in expectedTypes)
+                    {
+                        if (!EntityManager.HasComponent(entity, expectedType))
+                        {
+                            return;
+                        }
+                    }
+
+                    try
+                    {
+                        gameObjectCreator.OnEntityCreated(entityType, new SpatialOSEntity(entity, EntityManager), Linker);
+                    }
+                    catch (Exception e)
+                    {
+                        Debug.LogException(e);
+                    }
+
+                    PostUpdateCommands.AddComponent(entity, new GameObjectInitSystemStateComponent
+                    {
+                        EntityId = spatialEntityId.EntityId
+                    });
+                });
+        }
     }
 }
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/IEntityGameObjectCreator.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/IEntityGameObjectCreator.cs
@@ -11,17 +11,17 @@ namespace Improbable.Gdk.GameObjectCreation
     public interface IEntityGameObjectCreator
     {
         /// <summary>
-        ///     The minimum set of components required on an entity to create a GameObject.
+        ///     Called to register the components expected on an entity to create a GameObject for a given entity type.
         /// </summary>
-        ComponentType[] MinimumComponentTypes { get; }
+        void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations);
 
         /// <summary>
         ///     Called when a new SpatialOS Entity is checked out by the worker.
         /// </summary>
         /// <returns>
         ///     A GameObject to be linked to the entity, or null if no GameObject should be linked.
         /// </returns>
-        void OnEntityCreated(SpatialOSEntity entity, EntityGameObjectLinker linker);
+        void OnEntityCreated(string entityType, SpatialOSEntity entity, EntityGameObjectLinker linker);
 
         /// <summary>
         ///     Called when a SpatialOS Entity is removed from the worker's view.
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/Tests/Editmode/GameObjectCreationTests.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/Tests/Editmode/GameObjectCreationTests.cs
@@ -115,18 +115,20 @@ private class TestGameObjectCreator : IEntityGameObjectCreator
 
             private readonly Dictionary<EntityId, GameObject> entityIdToGameObject = new Dictionary<EntityId, GameObject>();
 
-            public ComponentType[] MinimumComponentTypes { get; } =
-            {
-                ComponentType.ReadOnly<Position.Component>(),
-                ComponentType.ReadOnly<Metadata.Component>()
-            };
-
             public TestGameObjectCreator(string workerType)
             {
                 this.workerType = workerType;
             }
 
-            public void OnEntityCreated(SpatialOSEntity entity, EntityGameObjectLinker linker)
+            public void PopulateEntityTypeExpectations(EntityTypeExpectations entityTypeExpectations)
+            {
+                entityTypeExpectations.RegisterDefault(new[]
+                {
+                    typeof(Metadata.Component), typeof(Position.Component)
+                });
+            }
+
+            public void OnEntityCreated(string entityType, SpatialOSEntity entity, EntityGameObjectLinker linker)
             {
                 var gameObject = new GameObject();
                 gameObject.transform.position = Vector3.one;
diff --git a/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/Tests/Editmode/MockGameObjectCreator.cs b/workers/unity/Packages/io.improbable.gdk.gameobjectcreation/Tests/Editmode/MockGameObjectCreator.cs
