Typed command ids (#1372)

* Command request ID's are now typed and split between internal and normal
Moved some files around to the right namespaces and locations

* Made CommandRequestIds sortable

* Changelog + Upgrade Guide

Author: zeroZshadow

CHANGELOG.md

@@ -7,6 +7,7 @@
 - `WorkerConnector.HandleWorkerConnectionFailure` has been removed and `WorkerConnector.Connect` now throws exceptions for connection errors instead. [#1365](https://github.com/spatialos/gdk-for-unity/pull/1365)
 - `WorkerConnector` no longer destroys itself in `Dispose`. [#1365](https://github.com/spatialos/gdk-for-unity/pull/1365)
 - `MultiThreadedSpatialOSConnectionHandler` and `SpatialOSConnectionHandlerBuilder.SetThreadingMode` have been removed. [#1367](https://github.com/spatialos/gdk-for-unity/pull/1367)
+- Command request IDs are now typed as `CommandRequestID` instead of `long`. [#1372](https://github.com/spatialos/gdk-for-unity/pull/1372)
 
 ### Added
 

UPGRADE_GUIDE.md

@@ -2,6 +2,13 @@
 
 ## From `0.3.5` to `0.3.6`
 
+### Command Request ID's are now typed
+
+The return value of `CommandSystem.SendCommand` has changed from `long` to `CommandRequestID`.
+A similar change has been made to all APIs that take request IDs as a parameter.
+
+Anywhere you store request IDs, you need to change the type from `long` to `CommandRequestId`.
+
 ### Multithreaded connection removed
 
 The `MultiThreadedSpatialOSConnectionHandler` has been removed and the method `SpatialOSConnectionHandlerBuilder.SetThreadingMode` has been removed to reflect this change.

workers/unity/Packages/io.improbable.gdk.core/.codegen/Source/Generators/Core/CommandDiffDeserializerGenerator.cs

@@ -14,6 +14,7 @@ public static CodeWriter Generate(UnityComponentDetails componentDetails)
             {
                 cgw.UsingDirectives(
                     "Improbable.Gdk.Core",
+                    "Improbable.Gdk.Core.Commands",
                     "Improbable.Worker.CInterop"
                 );
 
@@ -60,8 +61,9 @@ public void AddResponseToDiff(CommandResponseOp op, ViewDiff diff, CommandMetaDa
             rawResponse = {command.FqnResponseType}.Serialization.Deserialize(op.Response.SchemaData.Value.GetObject());
         }}
 
-        var commandContext = commandMetaData.GetContext<{command.FqnRequestType}>(ComponentId, {command.CommandIndex}, op.RequestId);
-        commandMetaData.RemoveRequest(ComponentId, {command.CommandIndex}, op.RequestId);
+        var internalRequestId = new InternalCommandRequestId(op.RequestId);
+        var commandContext = commandMetaData.GetContext<{command.FqnRequestType}>(ComponentId, {command.CommandIndex}, internalRequestId);
+        commandMetaData.RemoveRequest(ComponentId, {command.CommandIndex}, internalRequestId);
 
         var response = new {command.PascalCaseName}.ReceivedResponse(
             commandContext.SendingEntity,

workers/unity/Packages/io.improbable.gdk.core/.codegen/Source/Generators/Core/CommandMetaDataStorageGenerator.cs

@@ -13,7 +13,8 @@ public static CodeWriter Generate(UnityComponentDetails componentDetails)
             return CodeWriter.Populate(cgw =>
             {
                 cgw.UsingDirectives(
-                    "Improbable.Gdk.Core"
+                    "Improbable.Gdk.Core",
+                    "Improbable.Gdk.Core.Commands"
                 );
 
                 cgw.Namespace(componentDetails.Namespace, ns =>

workers/unity/Packages/io.improbable.gdk.core/.codegen/Source/Generators/Core/UnityCommandPayloadGenerator.cs

@@ -115,7 +115,7 @@ public Response(long requestId, string failureMessage)
         public readonly {commandDetails.FqnResponseType}? ResponsePayload;
         public readonly {commandDetails.FqnRequestType} RequestPayload;
         public readonly global::System.Object Context;
-        public readonly long RequestId;
+        public readonly CommandRequestId RequestId;
 
         public ReceivedResponse(
             Unity.Entities.Entity sendingEntity,
@@ -125,7 +125,7 @@ public ReceivedResponse(
             {commandDetails.FqnResponseType}? response,
             {commandDetails.FqnRequestType} request,
             global::System.Object context,
-            long requestId)
+            CommandRequestId requestId)
         {{
             SendingEntity = sendingEntity;
             EntityId = entityId;
@@ -137,7 +137,7 @@ public ReceivedResponse(
             RequestId = requestId;
         }}
 
-        long IReceivedCommandResponse.RequestId => RequestId;
+        CommandRequestId IReceivedCommandResponse.RequestId => RequestId;
     }}
 
     public readonly struct RawReceivedResponse : IRawReceivedCommandResponse

workers/unity/Packages/io.improbable.gdk.core/Collections/ReceivedMessageListExtensions.cs

@@ -7,7 +7,7 @@ internal static class ReceivedMessageListExtensions
     {
         // binary search for the command for given request ID
         // invariant: lower <= target <= upper
-        public static int? GetResponseIndex<T>(this MessageList<T> list, long requestId)
+        public static int? GetResponseIndex<T>(this MessageList<T> list, CommandRequestId requestId)
             where T : struct, IReceivedCommandResponse
         {
             var targetId = requestId;

workers/unity/Packages/io.improbable.gdk.core/Commands/CommandComponents.cs

@@ -23,7 +23,7 @@ public interface IReceivedCommandResponse : IReceivedMessage
         ///     Gets the request ID from the request. For use in generic methods.
         /// </summary>
         /// <returns> The request ID associated with the request </returns>
-        long RequestId { get; }
+        CommandRequestId RequestId { get; }
     }
 
     public interface IRawReceivedCommandResponse

workers/unity/Packages/io.improbable.gdk.core/Commands/CommandDiffStorageBase.cs

@@ -69,7 +69,7 @@ public MessagesSpan<TResponse> GetResponses()
             return responseStorage.Slice();
         }
 
-        public MessagesSpan<TResponse> GetResponse(long requestId)
+        public MessagesSpan<TResponse> GetResponse(CommandRequestId requestId)
         {
             if (!responsesSorted)
             {
@@ -83,15 +83,15 @@ public MessagesSpan<TResponse> GetResponse(long requestId)
                 : MessagesSpan<TResponse>.Empty();
         }
 
-        private class RequestComparer<T> : IComparer<T> where T : struct, IReceivedCommandRequest
+        private sealed class RequestComparer<T> : IComparer<T> where T : struct, IReceivedCommandRequest
         {
             public int Compare(T x, T y)
             {
                 return x.EntityId.Id.CompareTo(y.EntityId.Id);
             }
         }
 
-        private class ResponseComparer<T> : IComparer<T> where T : struct, IReceivedCommandResponse
+        private sealed class ResponseComparer<T> : IComparer<T> where T : struct, IReceivedCommandResponse
         {
             public int Compare(T x, T y)
             {

workers/unity/Packages/io.improbable.gdk.core/Worker/CommandMetaData.cs renamed to workers/unity/Packages/io.improbable.gdk.core/Commands/CommandMetaData.cs

@@ -3,16 +3,16 @@
 using System.Linq;
 using Unity.Entities;
 
-namespace Improbable.Gdk.Core
+namespace Improbable.Gdk.Core.Commands
 {
     public readonly struct CommandContext<T>
     {
         public readonly Entity SendingEntity;
         public readonly T Request;
         public readonly object Context;
-        public readonly long RequestId;
+        public readonly CommandRequestId RequestId;
 
-        public CommandContext(Entity sendingEntity, T request, object context, long requestId)
+        public CommandContext(Entity sendingEntity, T request, object context, CommandRequestId requestId)
         {
             SendingEntity = sendingEntity;
             Request = request;
@@ -24,24 +24,24 @@ public CommandContext(Entity sendingEntity, T request, object context, long requ
     public class CommandMetaData
     {
         // Cache the types needed to instantiate a new CommandMetaData
-        private static List<(uint componentId, Type command)> storageTypes;
+        private static readonly List<(uint componentId, Type command)> StorageTypes;
 
-        private readonly HashSet<long> internalRequestIds = new HashSet<long>();
+        private readonly HashSet<InternalCommandRequestId> internalRequestIds = new HashSet<InternalCommandRequestId>();
 
         private readonly Dictionary<(uint componentId, uint commandId), ICommandMetaDataStorage> componentCommandToStorage =
             new Dictionary<(uint componentId, uint commandId), ICommandMetaDataStorage>();
 
-        public CommandMetaData()
+        static CommandMetaData()
         {
-            if (storageTypes == null)
-            {
-                storageTypes = ComponentDatabase.Metaclasses
-                    .SelectMany(type => type.Value.Commands
-                        .Select(c => (componentId: type.Value.ComponentId, command: c.MetaDataStorage)))
-                    .ToList();
-            }
+            StorageTypes = ComponentDatabase.Metaclasses
+                .SelectMany(type => type.Value.Commands
+                    .Select(c => (componentId: type.Value.ComponentId, command: c.MetaDataStorage)))
+                .ToList();
+        }
 
-            foreach (var (componentId, type) in storageTypes)
+        public CommandMetaData()
+        {
+            foreach (var (componentId, type) in StorageTypes)
             {
                 var instance = (ICommandMetaDataStorage) Activator.CreateInstance(type);
                 componentCommandToStorage.Add((componentId, instance.CommandId), instance);
@@ -50,7 +50,7 @@ public CommandMetaData()
             componentCommandToStorage.Add((0, 0), new WorldCommandMetaDataStorage());
         }
 
-        public void RemoveRequest(uint componentId, uint commandId, long internalRequestId)
+        public void RemoveRequest(uint componentId, uint commandId, InternalCommandRequestId internalRequestId)
         {
             var commandMetaDataStorage = GetCommandDiffStorage(componentId, commandId);
             commandMetaDataStorage.RemoveMetaData(internalRequestId);
@@ -63,14 +63,14 @@ public void AddRequest<T>(uint componentId, uint commandId, in CommandContext<T>
             commandPayloadStorage.AddRequest(in context);
         }
 
-        public void AddInternalRequestId(uint componentId, uint commandId, long requestId, long internalRequestId)
+        public void AddInternalRequestId(uint componentId, uint commandId, CommandRequestId requestId, InternalCommandRequestId internalRequestId)
         {
             internalRequestIds.Add(internalRequestId);
             var commandMetaDataStorage = GetCommandDiffStorage(componentId, commandId);
             commandMetaDataStorage.SetInternalRequestId(internalRequestId, requestId);
         }
 
-        public CommandContext<T> GetContext<T>(uint componentId, uint commandId, long internalRequestId)
+        public CommandContext<T> GetContext<T>(uint componentId, uint commandId, InternalCommandRequestId internalRequestId)
         {
             var commandPayloadStorage = (ICommandPayloadStorage<T>) GetCommandDiffStorage(componentId, commandId);
             return commandPayloadStorage.GetPayload(internalRequestId);