Fix bug with components of the prefab root, and add regression test.

Oh but the regression test doesn't work because ConvertToModel regressed ;(

Author: Benoit Hudson

Assets/FbxExporters/Editor/UnitTests/ExporterTestBase.cs

@@ -16,6 +16,13 @@ namespace FbxExporters.UnitTests
 {
     public abstract class ExporterTestBase
     {
+        /// <summary>
+        /// Sleep an amount of time (in ms) so we can safely assume that the timestamp on an fbx will change.
+        /// </summary>
+        public void SleepForFileTimestamp() {
+            System.Threading.Thread.Sleep(1000);
+        }
+
         private string _testDirectory;
         protected string filePath {
             get {

Assets/FbxExporters/Editor/UnitTests/FbxPrefabAutoUpdaterTest.cs

@@ -74,7 +74,7 @@ public void ReplaceTest ()
             // Export it to the same fbx path. But first, wait one second so
             // that its timestamp differs enough for Unity to notice it
             // changed.
-            System.Threading.Thread.Sleep(1000);
+            SleepForFileTimestamp();
             FbxExporters.Editor.ModelExporter.ExportObject(m_fbxPath, newHierarchy);
             AssetDatabase.Refresh();
 
@@ -103,7 +103,7 @@ public void ExpensivePerformanceTest ()
             var stopwatch = new System.Diagnostics.Stopwatch ();
             stopwatch.Start();
 
-            // Create 1000 fbx models and 1000 prefabs.
+            // Create N fbx models and N/2 prefabs.
             // Each prefab points to an fbx model.
             //
             // Then modify one fbx model. Shouldn't take longer than 1s.

Assets/FbxExporters/Editor/UnitTests/FbxPrefabTest.cs

@@ -138,7 +138,7 @@ GameObject ModifySourceFbx()
             // Sleep one second first to make sure the timestamp differs
             // enough, so the asset database knows to reload it. I was getting
             // test failures otherwise.
-            System.Threading.Thread.Sleep(1000);
+            SleepForFileTimestamp();
             FbxExporters.Editor.ModelExporter.ExportObjects (
                     AssetDatabase.GetAssetPath(m_source),
                     new Object[] { newModel } );
@@ -233,6 +233,54 @@ public void ManualToAuto() {
         }
     }
 
+    public class FbxPrefabRegressions : ExporterTestBase
+    {
+        [Ignore("ConvertToModel return value is messed up.")]
+        [Test]
+        public void TestCubeAtRoot()
+        {
+            // vkovec found a bug when removing a mesh at the root.
+            // bhudson fixed it, let's make sure it stays fixed:
+            // 1. Make a cube
+            // 2. Convert to model
+            // 3. In Maya, make a null named 'Cube' and put it at the root.
+            // 4. Update => meshfilter and meshrenderer should be gone
+
+            // Make a cube.
+            var cube = GameObject.CreatePrimitive(PrimitiveType.Cube);
+            cube.name = "Cube";
+            var cubeAssetPath = GetRandomFbxFilePath();
+            var autoPrefab = FbxExporters.Editor.ConvertToModel.CreateInstantiatedModelPrefab(
+                new GameObject[] { cube }, path: cubeAssetPath)[0];
+            Assert.IsTrue(autoPrefab);
+
+            // Make a maya locator.
+            var locator = new GameObject("Cube");
+            var locatorAssetPath = FbxExporters.Editor.ModelExporter.ExportObject(
+                GetRandomFbxFilePath(), locator);
+
+            // Check the prefab has all the default stuff it should have.
+            Assert.IsNotNull(autoPrefab.GetComponent<MeshFilter>());
+            Assert.IsNotNull(autoPrefab.GetComponent<MeshRenderer>());
+            Assert.IsNotNull(autoPrefab.GetComponent<BoxCollider>());
+
+            // Now copy the locator over and refresh.
+            SleepForFileTimestamp();
+            System.IO.File.Copy(locatorAssetPath, cubeAssetPath, overwrite: true);
+            AssetDatabase.Refresh();
+
+            // Check the prefab lost its mesh filter and renderer.
+            Assert.IsNull(autoPrefab.GetComponent<MeshFilter>());
+            Assert.IsNull(autoPrefab.GetComponent<MeshRenderer>());
+
+            // The box collider is controversial: it got generated
+            // automatically, so shouldn't it be deleted automatically? But
+            // right now it doesn't get deleted, so let's test to make sure a
+            // change in behaviour isn't accidental.
+            Assert.IsNotNull(autoPrefab.GetComponent<BoxCollider>());
+        }
+    }
+
     public class FbxPrefabHelpersTest
     {
         class AClass { }

Assets/FbxExporters/FbxPrefab.cs

@@ -424,6 +424,7 @@ void InitHelper(FbxRepresentation fbxrep, string nodeName)
                 }
 
                 public Data(FbxRepresentation fbxrep) {
+                    m_parents.Add("", ""); // the root points to itself
                     InitHelper(fbxrep, "");
                 }
 
@@ -597,8 +598,12 @@ void ClassifyReparenting()
                 //    x    a     b   => conflict! switch to a for now (todo!)
                 //    x    x     a   => no action. Todo: what if a is being destroyed? conflict!
                 foreach(var name in m_prefab.NodeNames) {
+                    if (name == "") {
+                        // Don't reparent the root.
+                        continue;
+                    }
                     if (m_nodesToDestroy.Contains(name)) {
-                        // Reparent to null. This is to avoid the nuisance of
+                        // Reparent to the root. This is to avoid the nuisance of
                         // trying to destroy objects that are already destroyed
                         // because a parent got there first. Maybe there's a
                         // faster way to do it, but performance seems OK.
@@ -758,10 +763,18 @@ public bool NeedsUpdates() {
             /// </summary>
             public void ImplementUpdates(FbxPrefab prefabInstance)
             {
-                // Gather up all the nodes in the prefab.
+                Log("{0}: performing updates", prefabInstance.name);
+
+                // Gather up all the nodes in the prefab so we can look up
+                // nodes. We use the empty string for the root node.
+                var prefabRoot = prefabInstance.transform;
                 var prefabNodes = new Dictionary<string, Transform>();
                 foreach(var node in prefabInstance.GetComponentsInChildren<Transform>()) {
-                    prefabNodes.Add(node.name, node);
+                    if (node == prefabRoot) {
+                        prefabNodes[""] = node;
+                    } else {
+                        prefabNodes.Add(node.name, node);
+                    }
                 }
 
                 // Create new nodes.
@@ -786,7 +799,7 @@ public void ImplementUpdates(FbxPrefab prefabInstance)
                     var parent = kvp.Value;
                     Transform parentNode;
                     if (string.IsNullOrEmpty(parent)) {
-                        parentNode = prefabInstance.transform;
+                        parentNode = prefabRoot;
                     } else {
                         parentNode = prefabNodes[parent];
                     }
@@ -801,17 +814,17 @@ public void ImplementUpdates(FbxPrefab prefabInstance)
                 }
 
                 // Destroy the old components.
-                foreach(var kvp in prefabNodes) {
-                    var name = kvp.Key;
-                    var xfo = kvp.Value;
-                    List<System.Type> typesToDestroy;
-                    if (m_componentsToDestroy.TryGetValue(name, out typesToDestroy)) {
-                        foreach(var componentType in typesToDestroy) {
-                            var component = xfo.GetComponent(componentType);
-                            if (component != null) {
-                                Object.DestroyImmediate(component);
-                                Log("destroyed component {0}:{1}", xfo.name, componentType);
-                            }
+                foreach(var kvp in m_componentsToDestroy) {
+                    Log("destroying components on {0}", kvp.Key);
+                    var nodeName = kvp.Key;
+                    var typesToDestroy = kvp.Value;
+                    var prefabXfo = prefabNodes[nodeName];
+
+                    foreach(var componentType in typesToDestroy) {
+                        var component = prefabXfo.GetComponent(componentType);
+                        if (component != null) {
+                            Object.DestroyImmediate(component);
+                            Log("destroyed component {0}:{1}", nodeName, componentType);
                         }
                     }
                 }
@@ -880,6 +893,7 @@ void CompareAndUpdate()
 
             // If we don't need to do anything, jump out now.
             if (!updates.NeedsUpdates()) {
+                Log("{0}: no updates needed", transform.name);
                 return;
             }