Build system: Android SDK check & active target switch (#805)

* taking care of Android SDK check and switching back to active build target * changelog * making cli fail if build is skipped * added tooltip error in config to indicate that android sdk is missing * PR feedback
spatialos · Mar 6, 2019 · a44d66e · a44d66e
1 parent d2430cf
commit a44d66e
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,10 +46,13 @@
 - Improved the UX of the BuildConfiguration inspector.
 - Improved the UX of the GDK Tools Configuration window.
 - Deleting a `GameObject` now automatically unlinks it from its ECS entity. Note that the ECS entity and the SpatialOS entity are _not_ also deleted.
+- Changed the format of the BuildConfiguration asset. Please recreate, or copy it from `workers/unity/Playground/Assets/Config/BuildConfiguration.asset`.
+- Building workers will not change the active build target anymore. The build target will be set back to whatever was set before starting the build process.
 
 ### Fixed
 
 - Fixed a bug where running `SpatialOS -> Generate code` would always regenerate code, even if no files had changed.
+- Fixed a bug where building all workers in our sample projects would fail, if you have Android build support installed, but didn't set the path to the Android SDK.
 
 ### Internal
 

diff --git a/workers/unity/Packages/com.improbable.gdk.buildsystem/Configuration/BuildConfigEditor.cs b/workers/unity/Packages/com.improbable.gdk.buildsystem/Configuration/BuildConfigEditor.cs
@@ -164,6 +164,13 @@ private bool DrawWorkerConfiguration(WorkerBuildConfiguration configurationForWo
                             new GUIContent(EditorGUIUtility.IconContent(BuildConfigEditorStyle.BuiltInErrorIcon))
                                 { tooltip = "Missing build support for one or more build targets." };
                     }
+                    else if (configurationForWorker.CloudBuildConfig.BuildTargets.Any(NeedsAndroidSdk) ||
+                        configurationForWorker.LocalBuildConfig.BuildTargets.Any(NeedsAndroidSdk))
+                    {
+                        foldoutState.Icon =
+                            new GUIContent(EditorGUIUtility.IconContent(BuildConfigEditorStyle.BuiltInErrorIcon))
+                                { tooltip = "Missing Android SDK installation. Go to Preferences > External Tools to set it up." };
+                    }
                     else
                     {
                         foldoutState.Icon = null;
@@ -831,6 +838,11 @@ private static bool IsBuildTargetError(BuildTargetConfig t)
             return !WorkerBuildData.BuildTargetsThatCanBeBuilt[t.Target] && t.Enabled;
         }
 
+        private static bool NeedsAndroidSdk(BuildTargetConfig t)
+        {
+            return t.Enabled && t.Target == BuildTarget.Android && string.IsNullOrEmpty(EditorPrefs.GetString("AndroidSdkRoot"));
+        }
+
         /// <summary>
         /// Unity's GUIUtility.GetStateObject changes based on the structure of the GUI, for example when expanding or collapsing foldouts.
         /// Even with hints, tracking the state objects goes awry.

diff --git a/workers/unity/Packages/com.improbable.gdk.buildsystem/WorkerBuilder.cs b/workers/unity/Packages/com.improbable.gdk.buildsystem/WorkerBuilder.cs
@@ -67,21 +67,12 @@ public static void Build()
                         throw new BuildFailedException("Unknown scripting backend value: " + wantedScriptingBackend);
                 }
 
-                LocalLaunch.BuildConfig();
+                var buildsSucceeded = BuildWorkers(wantedWorkerTypes, buildEnvironment, scriptingBackend);
 
-                var workerResults = new Dictionary<string, bool>();
-                foreach (var wantedWorkerType in wantedWorkerTypes)
+                if (!buildsSucceeded)
                 {
-                    var result = BuildWorkerForEnvironment(wantedWorkerType, buildEnvironment, scriptingBackend);
-                    workerResults[wantedWorkerType] = result;
+                    throw new BuildFailedException("Not all builds were completed successfully. See the log for more information.");
                 }
-
-                var missingWorkerTypes = string.Join(" ", workerResults.Keys.Where(k => workerResults[k]));
-                var completedWorkerTypes = string.Join(" ", workerResults.Keys.Where(k => !workerResults[k]));
-                Debug.LogWarning(
-                    $"Completed build for {buildEnvironment} target.\n"
-                    + $"Completed builds for: {completedWorkerTypes}\n"
-                    + $"Skipped builds for: {missingWorkerTypes}. See above for more information.");
             }
             catch (Exception e)
             {
@@ -102,14 +93,7 @@ internal static void MenuBuild(BuildEnvironment environment, params string[] wor
             {
                 try
                 {
-                    LocalLaunch.BuildConfig();
-
-                    foreach (var workerType in workerTypes)
-                    {
-                        BuildWorkerForEnvironment(workerType, environment);
-                    }
-
-                    Debug.LogFormat("Completed build for {0} target", environment);
+                    BuildWorkers(workerTypes, environment);
                 }
                 catch (Exception)
                 {
@@ -122,7 +106,50 @@ internal static void MenuBuild(BuildEnvironment environment, params string[] wor
             };
         }
 
-        internal static bool BuildWorkerForEnvironment(string workerType, BuildEnvironment targetEnvironment, ScriptingImplementation? scriptingBackend = null)
+        private static bool BuildWorkers(string[] workerTypes, BuildEnvironment buildEnvironment, ScriptingImplementation? scriptingBackend = null)
+        {
+            var activeBuildTarget = EditorUserBuildSettings.activeBuildTarget;
+            var activeBuildTargetGroup = BuildPipeline.GetBuildTargetGroup(activeBuildTarget);
+
+            try
+            {
+                LocalLaunch.BuildConfig();
+
+                var workerResults = new Dictionary<string, bool>();
+                foreach (var wantedWorkerType in workerTypes)
+                {
+                    var result = BuildWorkerForEnvironment(wantedWorkerType, buildEnvironment, scriptingBackend);
+                    workerResults[wantedWorkerType] = result;
+                }
+
+                var missingWorkerTypes = string.Join(" ", workerResults.Keys.Where(k => !workerResults[k]));
+                var completedWorkerTypes = string.Join(" ", workerResults.Keys.Where(k => workerResults[k]));
+
+                if (missingWorkerTypes.Length > 0)
+                {
+                    Debug.LogWarning(
+                        $"Completed build for {buildEnvironment} target.\n"
+                        + $"Completed builds for: {completedWorkerTypes}\n"
+                        + $"Skipped builds for: {missingWorkerTypes}. See above for more information.");
+                    return false;
+                }
+                else
+                {
+                    Debug.Log($"Completed build for {buildEnvironment} target.");
+                    return true;
+                }
+            }
+            catch (Exception e)
+            {
+                throw new BuildFailedException(e);
+            }
+            finally
+            {
+                EditorUserBuildSettings.SwitchActiveBuildTarget(activeBuildTargetGroup, activeBuildTarget);
+            }
+        }
+
+        private static bool BuildWorkerForEnvironment(string workerType, BuildEnvironment targetEnvironment, ScriptingImplementation? scriptingBackend = null)
         {
             var spatialOSBuildConfiguration = BuildConfig.GetInstance();
             var environmentConfig = spatialOSBuildConfiguration.GetEnvironmentConfigForWorker(workerType, targetEnvironment);
@@ -140,6 +167,8 @@ internal static bool BuildWorkerForEnvironment(string workerType, BuildEnvironme
                 Directory.CreateDirectory(PlayerBuildDirectory);
             }
 
+            var hasBuildSucceeded = true;
+
             foreach (var config in enabledTargets)
             {
                 var buildTargetGroup = BuildPipeline.GetBuildTargetGroup(config.Target);
@@ -152,7 +181,7 @@ internal static bool BuildWorkerForEnvironment(string workerType, BuildEnvironme
                         PlayerSettings.SetScriptingBackend(buildTargetGroup, scriptingBackend.Value);
                     }
 
-                    BuildWorkerForTarget(workerType, config.Target, config.Options, targetEnvironment);
+                    hasBuildSucceeded &= BuildWorkerForTarget(workerType, config.Target, config.Options, targetEnvironment);
                 }
                 catch (Exception e)
                 {
@@ -164,7 +193,7 @@ internal static bool BuildWorkerForEnvironment(string workerType, BuildEnvironme
                 }
             }
 
-            return true;
+            return hasBuildSucceeded;
         }
 
         public static void Clean()
@@ -180,13 +209,13 @@ public static void Clean()
             }
         }
 
-        private static void BuildWorkerForTarget(string workerType, BuildTarget buildTarget,
+        private static bool BuildWorkerForTarget(string workerType, BuildTarget buildTarget,
             BuildOptions buildOptions, BuildEnvironment targetEnvironment)
         {
             if (!WorkerBuildData.BuildTargetsThatCanBeBuilt[buildTarget])
             {
                 Debug.LogWarning($"Skipping {buildTarget} because support is not installed in the Unity Editor.");
-                return;
+                return false;
             }
 
             Debug.Log(
@@ -207,18 +236,26 @@ private static void BuildWorkerForTarget(string workerType, BuildTarget buildTar
             var result = BuildPipeline.BuildPlayer(buildPlayerOptions);
             if (result.summary.result != BuildResult.Succeeded)
             {
+                if (buildTarget == BuildTarget.Android && string.IsNullOrEmpty(EditorPrefs.GetString("AndroidSdkRoot")))
+                {
+                    Debug.LogWarning($"Unable to build worker {workerType} for platform Android. " +
+                        $"Ensure you have the Android SDK set inside the Unity Editor Preferences.");
+                    return false;
+                }
+
                 throw new BuildFailedException($"Build failed for {workerType}");
             }
 
             if (buildTarget == BuildTarget.Android || buildTarget == BuildTarget.iOS)
             {
                 // Mobile clients can only be run locally, no need to package them
-                return;
+                return true;
             }
 
             var zipPath = Path.Combine(PlayerBuildDirectory, workerBuildData.PackageName);
             var basePath = Path.Combine(EditorPaths.BuildScratchDirectory, workerBuildData.PackageName);
             Zip(zipPath, basePath, targetEnvironment == BuildEnvironment.Cloud);
+            return true;
         }
 
         private static void Zip(string zipAbsolutePath, string basePath, bool useCompression)