Subscription cleanup (#1200)

* Fix filename * Fix folder name * Split Callbacks.cs into separate files per class * Remove unused EntitySubscriptions.cs Folder rename metafiles * AggregateSubscriptions are now made by the SubscriptionSystem * Don't clear empty lists Named tuples * Formatting to style guide * Remove GuardedAuthorityCallbackManagerSet as its a duplicated of the GuardedCallbackManagerSet * Callback arguments are passed as ref readonly (in) * Remove unused HashSet in WorkerFlagSubscriptionManager * Have subscriptions use Option<T> instead of re-implementing optional values * Changelog
spatialos · Oct 25, 2019 · 82c49b5 · 82c49b5
1 parent 8710dd5
commit 82c49b5
Show file tree

Hide file tree

Showing 52 changed files with 259 additions and 292 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,13 @@
 
 - Generated Worker ID's for local development are now smaller and easier to read for debugging. [#1197](https://github.com/spatialos/gdk-for-unity/pull/1197)
 
+### Internal
+
+- Cleaned up Subscriptions and Callbacks. [#1200](https://github.com/spatialos/gdk-for-unity/pull/1200)
+  - Replaced usage of `GuardedAuthorityCallbackManagerSet` with more generic `GuardedCallbackManagerSet`.
+  - Removed unused `EntitySubscriptions` class.
+  - Formatting pass on all Subscriptions and Callbacks files.
+
 ## `0.2.10` - 2019-10-14
 
 ### Breaking Changes

diff --git a/...core/Subscriptions/CalllbackManagers.meta → ....core/Subscriptions/CallbackManagers.meta b/...core/Subscriptions/CalllbackManagers.meta → ....core/Subscriptions/CallbackManagers.meta
diff --git a/...ers/AuthorityConstraintCallbackManager.cs → ...ers/AuthorityConstraintCallbackManager.cs b/...ers/AuthorityConstraintCallbackManager.cs → ...ers/AuthorityConstraintCallbackManager.cs
@@ -22,7 +22,7 @@ public AuthorityConstraintCallbackManager(uint componentId, World world)
         public void InvokeCallbacks()
         {
             var changes = componentUpdateSystem.GetAuthorityChangesReceived(componentId);
-            for (int i = 0; i < changes.Count; ++i)
+            for (var i = 0; i < changes.Count; ++i)
             {
                 if (changes[i].Authority == Authority.Authoritative)
                 {
@@ -38,7 +38,7 @@ public void InvokeCallbacks()
         public void InvokeLossImminentCallbacks()
         {
             var changes = componentUpdateSystem.GetAuthorityChangesReceived(componentId);
-            for (int i = 0; i < changes.Count; ++i)
+            for (var i = 0; i < changes.Count; ++i)
             {
                 if (changes[i].Authority == Authority.AuthorityLossImminent)
                 {

diff --git a/...uthorityConstraintCallbackManager.cs.meta → ...uthorityConstraintCallbackManager.cs.meta b/...uthorityConstraintCallbackManager.cs.meta → ...uthorityConstraintCallbackManager.cs.meta
diff --git a/...Managers/CommandRequestCallbackManager.cs → ...Managers/CommandRequestCallbackManager.cs b/...Managers/CommandRequestCallbackManager.cs → ...Managers/CommandRequestCallbackManager.cs
@@ -20,7 +20,7 @@ public CommandRequestCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var requests = commandSystem.GetRequests<T>();
-            for (int i = 0; i < requests.Count; ++i)
+            for (var i = 0; i < requests.Count; ++i)
             {
                 ref readonly var request = ref requests[i];
                 callbacks.InvokeAll(request.GetEntityId().Id, request);

diff --git a/...ers/CommandRequestCallbackManager.cs.meta → ...ers/CommandRequestCallbackManager.cs.meta b/...ers/CommandRequestCallbackManager.cs.meta → ...ers/CommandRequestCallbackManager.cs.meta
diff --git a/...anagers/CommandResponseCallbackManager.cs → ...anagers/CommandResponseCallbackManager.cs b/...anagers/CommandResponseCallbackManager.cs → ...anagers/CommandResponseCallbackManager.cs
@@ -20,7 +20,7 @@ public CommandResponseCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var responses = commandSystem.GetResponses<T>();
-            for (int i = 0; i < responses.Count; ++i)
+            for (var i = 0; i < responses.Count; ++i)
             {
                 ref readonly var response = ref responses[i];
                 callbacks.InvokeAll(response.GetRequestId(), response);

diff --git a/...rs/CommandResponseCallbackManager.cs.meta → ...rs/CommandResponseCallbackManager.cs.meta b/...rs/CommandResponseCallbackManager.cs.meta → ...rs/CommandResponseCallbackManager.cs.meta
diff --git a/...Managers/ComponentAddedCallbackManager.cs → ...Managers/ComponentAddedCallbackManager.cs b/...Managers/ComponentAddedCallbackManager.cs → ...Managers/ComponentAddedCallbackManager.cs
@@ -1,6 +1,5 @@
 using System;
 using Improbable.Gdk.Core;
-using Improbable.Worker.CInterop;
 using Unity.Entities;
 
 namespace Improbable.Gdk.Subscriptions
@@ -23,9 +22,9 @@ public ComponentAddedCallbackManager(uint componentId, World world)
         public void InvokeCallbacks()
         {
             var entities = componentUpdateSystem.GetComponentsAdded(componentId);
-            for (int i = 0; i < entities.Count; ++i)
+            foreach (var entityId in entities)
             {
-                callbacks.InvokeAll(entities[i]);
+                callbacks.InvokeAll(entityId);
             }
         }
 

diff --git a/...ers/ComponentAddedCallbackManager.cs.meta → ...ers/ComponentAddedCallbackManager.cs.meta b/...ers/ComponentAddedCallbackManager.cs.meta → ...ers/ComponentAddedCallbackManager.cs.meta
diff --git a/...gers/ComponentAuthorityCallbackManager.cs → ...gers/ComponentAuthorityCallbackManager.cs b/...gers/ComponentAuthorityCallbackManager.cs → ...gers/ComponentAuthorityCallbackManager.cs
@@ -22,23 +22,24 @@ public ComponentAuthorityCallbackManager(uint componentId, World world)
         public void InvokeCallbacks()
         {
             var changes = componentUpdateSystem.GetAuthorityChangesReceived(componentId);
-            for (int i = 0; i < changes.Count; ++i)
+            for (var i = 0; i < changes.Count; ++i)
             {
-                if (changes[i].Authority == Authority.Authoritative)
+                switch (changes[i].Authority)
                 {
-                    callbacks.InvokeAll(changes[i].EntityId.Id, changes[i].Authority);
-                }
-                else if (changes[i].Authority == Authority.NotAuthoritative)
-                {
-                    callbacks.InvokeAllReverse(changes[i].EntityId.Id, changes[i].Authority);
+                    case Authority.Authoritative:
+                        callbacks.InvokeAll(changes[i].EntityId.Id, changes[i].Authority);
+                        break;
+                    case Authority.NotAuthoritative:
+                        callbacks.InvokeAllReverse(changes[i].EntityId.Id, changes[i].Authority);
+                        break;
                 }
             }
         }
 
         public void InvokeLossImminentCallbacks()
         {
             var changes = componentUpdateSystem.GetAuthorityChangesReceived(componentId);
-            for (int i = 0; i < changes.Count; ++i)
+            for (var i = 0; i < changes.Count; ++i)
             {
                 if (changes[i].Authority == Authority.AuthorityLossImminent)
                 {

diff --git a/...ComponentAuthorityCallbackManager.cs.meta → ...ComponentAuthorityCallbackManager.cs.meta b/...ComponentAuthorityCallbackManager.cs.meta → ...ComponentAuthorityCallbackManager.cs.meta
diff --git a/...Managers/ComponentEventCallbackManager.cs → ...Managers/ComponentEventCallbackManager.cs b/...Managers/ComponentEventCallbackManager.cs → ...Managers/ComponentEventCallbackManager.cs
@@ -4,7 +4,7 @@
 
 namespace Improbable.Gdk.Subscriptions
 {
-    internal class ComponentEventCallbackManager<T> : ICallbackManager where T : IEvent
+    internal class ComponentEventCallbackManager<T> : ICallbackManager where T : struct, IEvent
     {
         private readonly IndexedCallbacks<T> callbacks = new IndexedCallbacks<T>();
         private readonly ComponentUpdateSystem componentUpdateSystem;
@@ -19,10 +19,10 @@ public ComponentEventCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var updates = componentUpdateSystem.GetEventsReceived<T>();
-            for (int i = 0; i < updates.Count; ++i)
+            for (var i = 0; i < updates.Count; ++i)
             {
                 ref readonly var update = ref updates[i];
-                callbacks.InvokeAll(update.EntityId.Id, update.Event);
+                callbacks.InvokeAll(update.EntityId.Id, in update.Event);
             }
         }
 

diff --git a/...ers/ComponentEventCallbackManager.cs.meta → ...ers/ComponentEventCallbackManager.cs.meta b/...ers/ComponentEventCallbackManager.cs.meta → ...ers/ComponentEventCallbackManager.cs.meta
diff --git a/...nagers/ComponentRemovedCallbackManager.cs → ...nagers/ComponentRemovedCallbackManager.cs b/...nagers/ComponentRemovedCallbackManager.cs → ...nagers/ComponentRemovedCallbackManager.cs
@@ -23,9 +23,9 @@ public void InvokeCallbacks()
         {
             // todo like entity stuff this should also be temporarily removed components
             var entities = componentUpdateSystem.GetComponentsRemoved(componentId);
-            for (int i = 0; i < entities.Count; ++i)
+            foreach (var entityId in entities)
             {
-                callbacks.InvokeAllReverse(entities[i]);
+                callbacks.InvokeAllReverse(entityId);
             }
         }
 

diff --git a/...s/ComponentRemovedCallbackManager.cs.meta → ...s/ComponentRemovedCallbackManager.cs.meta b/...s/ComponentRemovedCallbackManager.cs.meta → ...s/ComponentRemovedCallbackManager.cs.meta
diff --git a/...anagers/ComponentUdpateCallbackManager.cs → ...anagers/ComponentUpdateCallbackManager.cs b/...anagers/ComponentUdpateCallbackManager.cs → ...anagers/ComponentUpdateCallbackManager.cs
@@ -4,7 +4,7 @@
 
 namespace Improbable.Gdk.Subscriptions
 {
-    internal class ComponentUpdateCallbackManager<T> : ICallbackManager where T : ISpatialComponentUpdate
+    internal class ComponentUpdateCallbackManager<T> : ICallbackManager where T : struct, ISpatialComponentUpdate
     {
         private readonly IndexedCallbacks<T> callbacks = new IndexedCallbacks<T>();
         private readonly ComponentUpdateSystem componentUpdateSystem;
@@ -19,10 +19,10 @@ public ComponentUpdateCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var updates = componentUpdateSystem.GetComponentUpdatesReceived<T>();
-            for (int i = 0; i < updates.Count; ++i)
+            for (var i = 0; i < updates.Count; ++i)
             {
                 ref readonly var update = ref updates[i];
-                callbacks.InvokeAll(update.EntityId.Id, update.Update);
+                callbacks.InvokeAll(update.EntityId.Id, in update.Update);
             }
         }
 

diff --git a/...uardedAuthorityCallbackManagerSet.cs.meta → ...rs/ComponentUpdateCallbackManager.cs.meta b/...uardedAuthorityCallbackManagerSet.cs.meta → ...rs/ComponentUpdateCallbackManager.cs.meta
diff --git a/...ackManagers/EntityAddedCallbackManager.cs → ...ackManagers/EntityAddedCallbackManager.cs b/...ackManagers/EntityAddedCallbackManager.cs → ...ackManagers/EntityAddedCallbackManager.cs
@@ -19,9 +19,9 @@ public EntityAddedCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var entities = entitySystem.GetEntitiesAdded();
-            for (int i = 0; i < entities.Count; ++i)
+            foreach (var entityId in entities)
             {
-                callbacks.InvokeAll(entities[i]);
+                callbacks.InvokeAll(entityId);
             }
         }
 

diff --git a/...nagers/EntityAddedCallbackManager.cs.meta → ...nagers/EntityAddedCallbackManager.cs.meta b/...nagers/EntityAddedCallbackManager.cs.meta → ...nagers/EntityAddedCallbackManager.cs.meta
diff --git a/...kManagers/EntityRemovedCallbackManager.cs → ...kManagers/EntityRemovedCallbackManager.cs b/...kManagers/EntityRemovedCallbackManager.cs → ...kManagers/EntityRemovedCallbackManager.cs
@@ -19,9 +19,9 @@ public EntityRemovedCallbackManager(World world)
         public void InvokeCallbacks()
         {
             var entities = entitySystem.GetEntitiesRemoved();
-            for (int i = 0; i < entities.Count; ++i)
+            foreach (var entityId in entities)
             {
-                callbacks.InvokeAllReverse(entities[i]);
+                callbacks.InvokeAllReverse(entityId);
             }
         }
 

diff --git a/...gers/EntityRemovedCallbackManager.cs.meta → ...gers/EntityRemovedCallbackManager.cs.meta b/...gers/EntityRemovedCallbackManager.cs.meta → ...gers/EntityRemovedCallbackManager.cs.meta
diff --git a/...backManagers/GuardedCallbackManagerSet.cs → ...backManagers/GuardedCallbackManagerSet.cs b/...backManagers/GuardedCallbackManagerSet.cs → ...backManagers/GuardedCallbackManagerSet.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 
 namespace Improbable.Gdk.Subscriptions
@@ -22,23 +23,25 @@ public void AddCallbackManager(TIndex index, TManager manager)
             indexesToAdd.Add(index);
         }
 
-        public List<TManager> GetManagers()
+        private void UpdateManagers()
         {
-            foreach (var indexToAdd in indexesToAdd)
+            if (indexesToAdd.Count > 0)
             {
-                callbackManagers.Add(indexToCallbackManager[indexToAdd]);
-            }
-
-            indexesToAdd.Clear();
+                foreach (var indexToAdd in indexesToAdd)
+                {
+                    callbackManagers.Add(indexToCallbackManager[indexToAdd]);
+                }
 
-            return callbackManagers;
+                indexesToAdd.Clear();
+            }
         }
 
-        public void InvokeCallbacks()
+        public void InvokeEach(Action<TManager> callback)
         {
-            foreach (var manager in GetManagers())
+            UpdateManagers();
+            foreach (var manager in callbackManagers)
             {
-                manager.InvokeCallbacks();
+                callback(manager);
             }
         }
     }

diff --git a/...anagers/GuardedCallbackManagerSet.cs.meta → ...anagers/GuardedCallbackManagerSet.cs.meta b/...anagers/GuardedCallbackManagerSet.cs.meta → ...anagers/GuardedCallbackManagerSet.cs.meta
diff --git a/...backManagers/IAuthorityCallbackManager.cs → ...backManagers/IAuthorityCallbackManager.cs b/...backManagers/IAuthorityCallbackManager.cs → ...backManagers/IAuthorityCallbackManager.cs
diff --git a/...anagers/IAuthorityCallbackManager.cs.meta → ...anagers/IAuthorityCallbackManager.cs.meta b/...anagers/IAuthorityCallbackManager.cs.meta → ...anagers/IAuthorityCallbackManager.cs.meta
diff --git a/...ons/CalllbackManagers/ICallbackManager.cs → ...ions/CallbackManagers/ICallbackManager.cs b/...ons/CalllbackManagers/ICallbackManager.cs → ...ions/CallbackManagers/ICallbackManager.cs
diff --git a/...alllbackManagers/ICallbackManager.cs.meta → ...CallbackManagers/ICallbackManager.cs.meta b/...alllbackManagers/ICallbackManager.cs.meta → ...CallbackManagers/ICallbackManager.cs.meta
diff --git a/...backManagers/WorkerFlagCallbackManager.cs → ...backManagers/WorkerFlagCallbackManager.cs b/...backManagers/WorkerFlagCallbackManager.cs → ...backManagers/WorkerFlagCallbackManager.cs
@@ -30,7 +30,7 @@ public bool UnregisterCallback(ulong callbackKey)
         public void InvokeCallbacks()
         {
             var workerFlagChanges = workerSystem.Diff.GetWorkerFlagChanges();
-            for (int i = 0; i < workerFlagChanges.Count; ++i)
+            for (var i = 0; i < workerFlagChanges.Count; ++i)
             {
                 var pair = workerFlagChanges[i];
                 callbacks.InvokeAll(pair);

diff --git a/...anagers/WorkerFlagCallbackManager.cs.meta → ...anagers/WorkerFlagCallbackManager.cs.meta b/...anagers/WorkerFlagCallbackManager.cs.meta → ...anagers/WorkerFlagCallbackManager.cs.meta
diff --git a/workers/unity/Packages/io.improbable.gdk.core/Subscriptions/Callbacks.meta b/workers/unity/Packages/io.improbable.gdk.core/Subscriptions/Callbacks.meta