Pull request changes

Author: MarcShakour

Assets/FbxExporters/Editor/FbxExporter.cs

@@ -2471,7 +2471,7 @@ static void OnContextItem (MenuCommand command)
             }
 
             /// <summary>
-            // Validate the menu item defined by the function above.
+            /// Validate the menu item defined by the function above.
             /// </summary>
             [MenuItem (MenuItemName, true, 30)]
             public static bool OnValidateMenuItem ()

Assets/FbxExporters/Editor/FbxPrefabAutoUpdater.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System;
 using FbxExporters.Editor;
+using UnityEngine.TestTools;
 
 namespace FbxExporters
 {
@@ -33,6 +34,7 @@ namespace FbxExporters
         #endif
 
         const string MenuItemName = "GameObject/Update from FBX";
+        public static bool runningUnitTest = false;
 
         public static string FindFbxPrefabAssetPath()
         {
@@ -163,7 +165,9 @@ static void OnPostprocessAllAssets(string[] imported, string[] deleted, string[]
                 }
             }
         }
-
+        /// <summary>
+        /// Add an option "Update from FBX" in the contextual GameObject menu.
+        /// </summary>
         [MenuItem(MenuItemName, false,31)]
         static void OnContextItem(MenuCommand command)
         {
@@ -191,42 +195,69 @@ static void OnContextItem(MenuCommand command)
         }
 
         /// <summary>
-        // Validate the menu item defined by the function above.
+        /// Validate the menu item defined by the function above.
         /// </summary>
         [MenuItem(MenuItemName, true,31)]
         public static bool OnValidateMenuItem()
         {
-            GameObject[] selection = Selection.GetFiltered<GameObject>(SelectionMode.Editable | SelectionMode.TopLevel);
+            GameObject[] selection = Selection.gameObjects;
 
             if (selection == null || selection.Length == 0)
             {
-                UnityEditor.EditorUtility.DisplayDialog(
-                    string.Format("{0} Warning", ModelExporter.PACKAGE_UI_NAME),
-                    "No GameObjects selected for update.",
-                    "Ok");
+                string errorMessage = "No GameObjects selected for update.";
+                if (runningUnitTest)
+                { 
+                    Debug.LogError(errorMessage);
+                    LogAssert.Expect(LogType.Error, errorMessage);
+                }
+                else
+                {
+                    DisplayNoSelectionDialog(errorMessage);
+                }
                 return false;
             }
 
+            bool containsLinkedPrefab = false;
             foreach (GameObject selectedObject in selection)
             {
                 GameObject prefab = UnityEditor.PrefabUtility.GetPrefabParent(selectedObject) as GameObject;
-                if (!prefab)
+                if (prefab && prefab.GetComponentInChildren<FbxPrefab>())
+                {
+                    containsLinkedPrefab = true;
+                    break;
+                }
+            }
+
+            if (!containsLinkedPrefab)
+            { 
+                string errorMessage = "No linked prefabs selected.";
+                if (runningUnitTest)
                 {
-                    UnityEditor.EditorUtility.DisplayDialog(
-                        string.Format("{0} Warning", ModelExporter.PACKAGE_UI_NAME),
-                        "These GameObjects aren't prefab.",
-                        "Ok");
-                    return false;
+                    Debug.LogError(errorMessage);
+                    LogAssert.Expect(LogType.Error, errorMessage);
                 }
-                if (prefab.GetComponentInChildren<FbxPrefab>())
+                else
                 {
-                    return true;
+                    DisplayNoSelectionDialog(errorMessage);
                 }
             }
 
-            return false;
+
+            return containsLinkedPrefab;
         }
 
+
+        static void DisplayNoSelectionDialog(string message)
+        {
+            UnityEditor.EditorUtility.DisplayDialog(
+                string.Format("{0} Warning", ModelExporter.PACKAGE_UI_NAME),
+                message,
+                "Ok");
+        }
+
+        /// <summary>
+        /// Launch the manual update of the linked prefab specified
+        /// </summary>
         public static void UpdateLinkedPrefab(GameObject prefabInstance)
         {
             GameObject prefab = UnityEditor.PrefabUtility.GetPrefabParent(prefabInstance) as GameObject;

Assets/FbxExporters/Editor/UnitTests/FbxPrefabAutoUpdaterTest.cs

@@ -215,8 +215,10 @@ public class FbxPrefabAutoUpdaterToggleTest : ExporterTestBase
         [SetUp]
         public void Init()
         {
+            // Save the initial setting for the auto updater toggle and disable it for the unit test
             isAutoUpdaterOn = FbxExporters.EditorTools.ExportSettings.instance.autoUpdaterEnabled;
             FbxExporters.EditorTools.ExportSettings.instance.autoUpdaterEnabled = false;
+            FbxPrefabAutoUpdater.runningUnitTest = true;
         }
 
         [Test]
@@ -274,13 +276,13 @@ public void RemappingTest()
             Assert.IsTrue(cubePrefabInstance.transform.GetChild(0).name == "SphereFBX");
             Assert.IsTrue(cubePrefabInstance.transform.GetChild(0).GetComponent<MeshFilter>().sharedMesh != null);
 
-            // Testing Manual update on some random object that isn't a prefab at all
+            // Testing Manual update on some random object that isn't a prefab at all, the gameobject shouldn't be modified. No error is returned
             GameObject quad = GameObject.CreatePrimitive(PrimitiveType.Quad);
             FbxPrefabAutoUpdater.UpdateLinkedPrefab(quad);
             Assert.IsTrue(quad != null);
             Assert.IsTrue(quad.GetComponent<MeshFilter>().sharedMesh != null);
 
-            // Testing Manual update on some random prefab that doesn't have an FbxPrefab in it.
+            // Testing Manual update on some random prefab that doesn't have an FbxPrefab in it, the prefab shouldn't be modified. No error is returned
             GameObject capsule = GameObject.CreatePrimitive(PrimitiveType.Capsule);
             // Convert to linked prefab instance (auto-updating prefab)
             string prefabPath = GetRandomPrefabAssetPath();
@@ -297,19 +299,26 @@ public void RemappingTest()
             Selection.objects = new GameObject[] { sphere3Instance };
             Assert.IsTrue(FbxPrefabAutoUpdater.OnValidateMenuItem());
 
-            // Check the contextual menu returns false because there is no linked prefab
+            // Check the contextual menu returns false because there is no linked prefab in the selection, an error is returned
             GameObject cylinder4 = GameObject.CreatePrimitive(PrimitiveType.Cylinder);
             GameObject sphere4 = GameObject.CreatePrimitive(PrimitiveType.Sphere);
             sphere4.transform.SetParent(cylinder4.transform);
             Selection.objects = new GameObject[] { cylinder4, sphere4 };
             Assert.IsFalse(FbxPrefabAutoUpdater.OnValidateMenuItem());
+
+            // Assert is true because sphere 3 instance is a linked prefab and all the selection is taken into account
+            sphere3Instance.transform.SetParent(quad.transform);
+            Selection.objects = new GameObject[] { quad, sphere3Instance };
+            Assert.IsTrue(FbxPrefabAutoUpdater.OnValidateMenuItem());
         }
 
 
         [TearDown]
         public void stopTest()
         {
+            // Put back the initial setting for the auto-updater toggle
             FbxExporters.EditorTools.ExportSettings.instance.autoUpdaterEnabled = isAutoUpdaterOn;
+            FbxPrefabAutoUpdater.runningUnitTest = false;
         }
     }
 }