Resolving type/command/event name clashes (#1222)

Author: Paul Balaji

CHANGELOG.md

@@ -10,6 +10,8 @@
 
 - Fixed an issue where the Deployment Launcher window would feel unresponsive due to saving changes after every input. [#1219](https://github.com/spatialos/gdk-for-unity/pull/1219)
     - It will now wait for at least 1 second to elapse after the last change before writing the configuration back to disk.
+- Fixed issues ([#957](https://github.com/spatialos/gdk-for-unity/issues/957), [#958](https://github.com/spatialos/gdk-for-unity/issues/958)) where valid schema would generate invalid code due to name clashes. [#1222](https://github.com/spatialos/gdk-for-unity/pull/1222)
+    - The offending schema properties will no longer be generated and are now logged in the Unity Editor.
 
 ### Internal
 

test-project/Assets/.schema/illegal.schema

@@ -0,0 +1,27 @@
+// Wait, that's illegal!
+
+package improbable.gdk.test;
+
+type InvalidTypeWithNesting {
+    enum NestedEnum {
+        SUCCESS = 0;
+        FAILURE = 1;
+    }
+    type NestedType {}
+    NestedEnum nested_enum = 1;
+    NestedType nested_type = 2;
+}
+
+type EmptyType {}
+
+component InvalidComponentCommand
+{
+    id=12499;
+    command EmptyType invalid_component_command(EmptyType);
+}
+
+component InvalidComponentEvent
+{
+    id=12599;
+    event EmptyType invalid_component_event;
+}

workers/unity/Packages/io.improbable.gdk.tools/.CodeGenTemplate/CodeGenerationLib/Model/Details/DetailsStore.cs

@@ -83,8 +83,7 @@ public DetailsStore(SchemaBundle bundle, List<string> serializationOverrides, IF
 
             foreach (var kv in Types)
             {
-                kv.Value.PopulateFields(this);
-                kv.Value.PopulateChildren(this);
+                kv.Value.Populate(this);
             }
 
             foreach (var kv in Components)

workers/unity/Packages/io.improbable.gdk.tools/.CodeGenTemplate/CodeGenerationLib/Model/Details/UnityCommandDetails.cs

@@ -4,6 +4,7 @@ namespace Improbable.Gdk.CodeGeneration.Model.Details
 {
     public class UnityCommandDetails
     {
+        public string RawCommandName { get; }
         public string CommandName { get; }
         public string CamelCaseCommandName { get; }
 
@@ -14,8 +15,10 @@ public class UnityCommandDetails
 
         public UnityCommandDetails(ComponentDefinition.CommandDefinition commandDefinitionRaw)
         {
-            CommandName = Formatting.SnakeCaseToPascalCase(commandDefinitionRaw.Name);
+            RawCommandName = commandDefinitionRaw.Name;
+            CommandName = Formatting.SnakeCaseToPascalCase(RawCommandName);
             CamelCaseCommandName = Formatting.PascalCaseToCamelCase(CommandName);
+
             FqnRequestType =
                 CommonDetailsUtils.GetCapitalisedFqnTypename(commandDefinitionRaw.RequestType);
             FqnResponseType =

workers/unity/Packages/io.improbable.gdk.tools/.CodeGenTemplate/CodeGenerationLib/Model/Details/UnityComponentDetails.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 
@@ -26,11 +27,33 @@ public UnityComponentDetails(string package, ComponentDefinition componentDefini
 
             CommandDetails = componentDefinitionRaw.Commands
                 .Select(command => new UnityCommandDetails(command))
+                .Where(commandDetail =>
+                {
+                    // Return true to keep commands that do not have a name clash with the component
+                    if (!commandDetail.CommandName.Equals(ComponentName))
+                    {
+                        return true;
+                    }
+
+                    Console.Error.WriteLine($"Error in component \"{ComponentName}\". Command \"{commandDetail.RawCommandName}\" clashes with component name.");
+                    return false;
+                })
                 .ToList()
                 .AsReadOnly();
 
             EventDetails = componentDefinitionRaw.Events
                 .Select(ev => new UnityEventDetails(ev))
+                .Where(eventDetail =>
+                {
+                    // Return true to keep events that do not have a name clash with the component
+                    if (!eventDetail.EventName.Equals(ComponentName))
+                    {
+                        return true;
+                    }
+
+                    Console.Error.WriteLine($"Error in component \"{ComponentName}\". Event \"{eventDetail.RawEventName}\" clashes with component name.");
+                    return false;
+                })
                 .ToList()
                 .AsReadOnly();
 

workers/unity/Packages/io.improbable.gdk.tools/.CodeGenTemplate/CodeGenerationLib/Model/Details/UnityEventDetails.cs

@@ -4,16 +4,20 @@ namespace Improbable.Gdk.CodeGeneration.Model.Details
 {
     public class UnityEventDetails
     {
+        public string RawEventName { get; }
         public string EventName { get; }
         public string CamelCaseEventName { get; }
         public string FqnPayloadType { get; }
         public uint EventIndex { get; }
 
         public UnityEventDetails(ComponentDefinition.EventDefinition eventDefinitionRaw)
         {
-            EventName = Formatting.SnakeCaseToPascalCase(eventDefinitionRaw.Name);
+            RawEventName = eventDefinitionRaw.Name;
+            EventName = Formatting.SnakeCaseToPascalCase(RawEventName);
             CamelCaseEventName = Formatting.PascalCaseToCamelCase(EventName);
+
             FqnPayloadType = CommonDetailsUtils.GetCapitalisedFqnTypename(eventDefinitionRaw.Type);
+
             EventIndex = eventDefinitionRaw.EventIndex;
         }
     }

workers/unity/Packages/io.improbable.gdk.tools/.CodeGenTemplate/CodeGenerationLib/Model/Details/UnityTypeDetails.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using Improbable.Gdk.CodeGeneration.Utils;
@@ -37,7 +38,13 @@ public string GetPartialResourceTypeName()
             return FullyQualifiedTypeName.Split("::")[1];
         }
 
-        public void PopulateChildren(DetailsStore store)
+        public void Populate(DetailsStore store)
+        {
+            PopulateChildren(store);
+            PopulateFields(store);
+        }
+
+        private void PopulateChildren(DetailsStore store)
         {
             var children = store.GetNestedTypes(raw.QualifiedName);
 
@@ -54,10 +61,41 @@ public void PopulateChildren(DetailsStore store)
                 .AsReadOnly();
         }
 
-        public void PopulateFields(DetailsStore store)
+        private void PopulateFields(DetailsStore store)
         {
             FieldDetails = raw.Fields
                 .Select(field => new UnityFieldDetails(field, store))
+                .Where(fieldDetail =>
+                {
+                    var clashingChildEnums = ChildEnums
+                        .Where(childEnum =>
+                        {
+                            // When field does not clash with child enum, return false
+                            if (!fieldDetail.PascalCaseName.Equals(childEnum.TypeName))
+                            {
+                                return false;
+                            }
+
+                            Console.Error.WriteLine($"Error in type \"{CapitalisedName}\". Field \"{fieldDetail.Raw.Name}\" clashes with child enum \"{childEnum.TypeName}\".");
+                            return true;
+                        });
+
+                    var clashingChildTypes = ChildTypes
+                        .Where(childType =>
+                        {
+                            // When field does not clash with child type, return false
+                            if (!fieldDetail.PascalCaseName.Equals(childType.CapitalisedName))
+                            {
+                                return false;
+                            }
+
+                            Console.Error.WriteLine($"Error in type \"{CapitalisedName}\". Field \"{fieldDetail.Raw.Name}\" clashes with child type \"{childType.CamelCaseName}\".");
+                            return true;
+                        });
+
+                    // Only return true if the field has no name clashes with child enums and types
+                    return !clashingChildEnums.Any() && !clashingChildTypes.Any();
+                })
                 .ToList()
                 .AsReadOnly();
         }

workers/unity/Packages/io.improbable.gdk.tools/GenerateCode.cs

@@ -157,6 +157,7 @@ private static void Generate()
                         .WithArgs(ConstructArgs(CodegenExe, schemaCompilerPath, workerJsonPath))
                         .RedirectOutputOptions(OutputRedirectBehaviour.None)
                         .AddErrorProcessing((line) => errorMessage.Append($"\n{line}"))
+                        .AddErrorProcessing(Debug.LogError)
                         .AddOutputProcessing(ProcessStdOut)
                         .Run();