Refactor and test BuildContext (#1461)

spatialos · Aug 28, 2020 · 7678247 · 7678247
1 parent c028616
commit 7678247
Show file tree

Hide file tree

Showing 24 changed files with 606 additions and 151 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,10 +21,15 @@
 - Multithreaded component serialization through `SystemBase` jobs. [#1454](https://github.com/spatialos/gdk-for-unity/pull/1454)
 - Upgrade Unity Burst to 1.3.5. [#1467](https://github.com/spatialos/gdk-for-unity/pull/1467)
 - Removed outline and background around component info button in the Worker Inspector. [#1468](https://github.com/spatialos/gdk-for-unity/pull/1468)
+- Refactored the `BuildContext` class. [#1461](https://github.com/spatialos/gdk-for-unity/pull/1461)
+  - Introduced a `BuildContextSettings` struct, which is required by `GetBuildContexts`.
+  - Changed `BuildConfig` class visibility from `internal` to `public`.
+  - Added more testing around `BuildContext`.
 
 ### Fixed
 
 - Fixed an issue where authority changes returned by `ComponentUpdateSystem.GetAuthorityChangesReceived()` were returned in order from newest to oldest. [#1465](https://github.com/spatialos/gdk-for-unity/pull/1465)
+- Fixed a bug where the build system would throw a null reference exception if you don't have a configuration for a worker type. [#1461](https://github.com/spatialos/gdk-for-unity/pull/1461)
 
 ## `0.3.10` - 2020-08-18
 

diff --git a/workers/unity/Assets/Playground/Config/BuildConfiguration.asset b/workers/unity/Assets/Playground/Config/BuildConfiguration.asset
@@ -22,49 +22,49 @@ MonoBehaviour:
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
     CloudBuildConfig:
       BuildTargets:
       - Options: 0
         Deprecated: 1
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 16384
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
   - WorkerType: UnityGameLogic
     ScenesForWorker:
     - {fileID: 102900000, guid: 8c0c8d31c7a7905409ce6be1f36f162e, type: 3}
@@ -74,49 +74,49 @@ MonoBehaviour:
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
     CloudBuildConfig:
       BuildTargets:
       - Options: 0
         Deprecated: 1
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 16384
         Deprecated: 0
         Enabled: 1
         Required: 1
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
   - WorkerType: MobileClient
     ScenesForWorker:
     - {fileID: 102900000, guid: 82ccd0fd96a2ba24f88d0c7d01592f2e, type: 3}
@@ -126,47 +126,47 @@ MonoBehaviour:
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 1
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 1
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
     CloudBuildConfig:
       BuildTargets:
       - Options: 0
         Deprecated: 1
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 16384
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 0
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 0
+        scriptingImplementation: 0
       - Options: 0
         Deprecated: 0
         Enabled: 1
         Required: 0
-        ScriptingImplementation: 1
+        scriptingImplementation: 1
   isInitialised: 1
diff --git a/...obable.gdk.buildsystem/.codegen/Source/Generators/BuildSystem/UnityWorkerMenuGenerator.cs b/...obable.gdk.buildsystem/.codegen/Source/Generators/BuildSystem/UnityWorkerMenuGenerator.cs
@@ -45,13 +45,13 @@ public static CodeWriter Generate(List<string> workerTypes)
                             buildWorkerMenu.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/"" + LocalMenu + ""/{workerType}"", false, EditorConfig.MenuOffset + {i})")
                                 .Method($"public static void BuildLocal{workerType}()", () => new[]
                                 {
-                                    $@"MenuBuildLocal(new[] {{ {workerTypeString} }});"
+                                    $"MenuBuild(BuildContextSettings.Local({workerTypeString}));"
                                 });
 
                             buildWorkerMenu.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/"" + CloudMenu + ""/{workerType}"", false, EditorConfig.MenuOffset + {i})")
                                 .Method($"public static void BuildCloud{workerType}()", () => new[]
                                 {
-                                    $@"MenuBuildCloud(new[] {{ {workerTypeString} }});"
+                                    $"MenuBuild(BuildContextSettings.Cloud({workerTypeString}));"
                                 });
 
                             allMenuOptionValidators.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/"" + LocalMenu + ""/{workerType}"", true, EditorConfig.MenuOffset + {i})")
@@ -66,13 +66,13 @@ public static CodeWriter Generate(List<string> workerTypes)
                         buildWorkerMenu.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/"" + LocalMenu + ""/All workers"", false, EditorConfig.MenuOffset + {workerTypes.Count})")
                             .Method("public static void BuildLocalAll()", () => new[]
                             {
-                                "MenuBuildLocal(AllWorkers);"
+                                "MenuBuild(BuildContextSettings.Local(AllWorkers));"
                             });
 
                         buildWorkerMenu.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/"" + CloudMenu + ""/All workers"", false, EditorConfig.MenuOffset + {workerTypes.Count})")
                             .Method("public static void BuildCloudAll()", () => new[]
                             {
-                                "MenuBuildCloud(AllWorkers);"
+                                "MenuBuild(BuildContextSettings.Cloud(AllWorkers));"
                             });
 
                         buildWorkerMenu.Annotate($@"MenuItem(EditorConfig.ParentMenu + ""/Clean all workers"", false, EditorConfig.MenuOffset + {workerTypes.Count})")
@@ -81,14 +81,9 @@ public static CodeWriter Generate(List<string> workerTypes)
                                 "MenuCleanAll();"
                             });
 
-                        buildWorkerMenu.Method("private static void MenuBuildLocal(string[] filteredWorkerTypes)", () => new[]
+                        buildWorkerMenu.Method("private static void MenuBuild(BuildContextSettings buildContextSettings)", () => new[]
                         {
-                            "WorkerBuilder.MenuBuild(BuildEnvironment.Local, filteredWorkerTypes);"
-                        });
-
-                        buildWorkerMenu.Method("private static void MenuBuildCloud(string[] filteredWorkerTypes)", () => new[]
-                        {
-                            "WorkerBuilder.MenuBuild(BuildEnvironment.Cloud, filteredWorkerTypes);"
+                            "WorkerBuilder.MenuBuild(buildContextSettings);"
                         });
 
                         buildWorkerMenu.Method("private static void MenuCleanAll()", () => new[]

diff --git a/workers/unity/Packages/io.improbable.gdk.buildsystem/AssemblyInfo.cs b/workers/unity/Packages/io.improbable.gdk.buildsystem/AssemblyInfo.cs
@@ -1,3 +1,4 @@
 using System.Runtime.CompilerServices;
 
 [assembly: InternalsVisibleTo("Improbable.Gdk.Generated.BuildSystem")]
+[assembly: InternalsVisibleTo("Improbable.Gdk.BuildSystem.Tests")]
diff --git a/workers/unity/Packages/io.improbable.gdk.buildsystem/Configuration/BuildConfig.cs b/workers/unity/Packages/io.improbable.gdk.buildsystem/Configuration/BuildConfig.cs
@@ -7,15 +7,15 @@
 namespace Improbable.Gdk.BuildSystem.Configuration
 {
     [CreateAssetMenu(fileName = "SpatialOS Build Configuration", menuName = BuildConfigEditor.BuildConfigurationMenu)]
-    internal class BuildConfig : SingletonScriptableObject<BuildConfig>
+    public class BuildConfig : SingletonScriptableObject<BuildConfig>
     {
         [SerializeField]
-        public List<WorkerBuildConfiguration> WorkerBuildConfigurations =
+        internal List<WorkerBuildConfiguration> WorkerBuildConfigurations =
             new List<WorkerBuildConfiguration>();
 
         [SerializeField] private bool isInitialised;
 
-        public BuildEnvironmentConfig GetEnvironmentConfigForWorker(string workerType, BuildEnvironment environment)
+        internal BuildEnvironmentConfig GetEnvironmentConfigForWorker(string workerType, BuildEnvironment environment)
         {
             var config = WorkerBuildConfigurations.FirstOrDefault(x => x.WorkerType == workerType);
             if (config == null)
@@ -56,8 +56,18 @@ private void ResetToDefault()
                             WorkerBuildData.GetCurrentBuildTargetConfig()),
                     CloudBuildConfig = new BuildEnvironmentConfig(
                         WorkerBuildData.AllBuildTargets,
-                        new BuildTargetConfig(BuildTarget.StandaloneOSX, BuildOptions.Development, enabled: true, required: false),
-                        new BuildTargetConfig(BuildTarget.StandaloneWindows64, BuildOptions.Development, enabled: true, required: false))
+                        new BuildTargetConfig(BuildTarget.StandaloneOSX)
+                        {
+                            Options = BuildOptions.Development,
+                            Enabled = true,
+                            Required = false
+                        },
+                        new BuildTargetConfig(BuildTarget.StandaloneWindows64)
+                        {
+                            Options = BuildOptions.Development,
+                            Enabled = true,
+                            Required = false
+                        })
                 },
                 new WorkerBuildConfiguration
                 {

diff --git a/workers/unity/Packages/io.improbable.gdk.buildsystem/Configuration/BuildConfigEditor.cs b/workers/unity/Packages/io.improbable.gdk.buildsystem/Configuration/BuildConfigEditor.cs
@@ -575,13 +575,17 @@ private void DrawEnvironmentInspector(BuildEnvironment environment,
                 {
                     foldoutState.Icon =
                         new GUIContent(EditorGUIUtility.IconContent(BuildConfigEditorStyle.BuiltInErrorIcon))
-                        { tooltip = "Missing build support for one or more build targets." };
+                        {
+                            tooltip = "Missing build support for one or more build targets."
+                        };
                 }
                 else if (environmentConfiguration.BuildTargets.Any(IsBuildTargetWarning))
                 {
                     foldoutState.Icon =
                         new GUIContent(EditorGUIUtility.IconContent(BuildConfigEditorStyle.BuiltInWarningIcon))
-                        { tooltip = "Missing build support for one or more build targets." };
+                        {
+                            tooltip = "Missing build support for one or more build targets."
+                        };
                 }
                 else
                 {
@@ -721,8 +725,14 @@ private void DrawBuildTargets(BuildEnvironmentConfig env, int hash)
                 {
                     RecordUndo("Worker build options");
 
-                    env.BuildTargets[selectedTargetIndex] =
-                        new BuildTargetConfig(buildTarget.Target, options, enabled, required, deprecated: false, scriptingImplementation);
+                    env.BuildTargets[selectedTargetIndex] = new BuildTargetConfig(buildTarget.Target)
+                    {
+                        Options = options,
+                        Enabled = enabled,
+                        Required = required,
+                        Deprecated = false,
+                        ScriptingImplementation = scriptingImplementation
+                    };
 
                     selectedBuildTarget.Choices = null;
                 }