Fix two bugs found by tests (and fix tests).

- Ignore the root transform values, we don't want to drive them.
- Don't store the FbxPrefab, we don't want it driven by the FBX.

- Comparing FbxRepresentation is much more complicated now, because the
  Transform isn't exact after the roundtrip. Only affects the test.
- Added some debug logs in the test.

Author: Benoit Hudson

Assets/FbxExporters/Editor/UnitTests/FbxPrefabTest.cs

@@ -2,6 +2,7 @@
 using UnityEditor;
 using UnityEngine.TestTools;
 using NUnit.Framework;
+using System.Collections.Generic;
 
 namespace FbxExporters.UnitTests
 {
@@ -17,7 +18,61 @@ public class FbxPrefabTest : ExporterTestBase
         public static void AssertAreIdentical(
                 FbxPrefab.FbxRepresentation a,
                 FbxPrefab.FbxRepresentation b) {
-            Assert.AreEqual(a.ToJson(), b.ToJson());
+            // A bit of a laborious comparison scheme. This is due to the
+            // round-trip through FBX causing tiny errors in the transforms.
+            var astack = new List<FbxPrefab.FbxRepresentation> ();
+            astack.Add(a);
+            var bstack = new List<FbxPrefab.FbxRepresentation> ();
+            bstack.Add(b);
+
+            var aDummy = new GameObject("aDummy").transform;
+            var bDummy = new GameObject("bDummy").transform;
+            while (astack.Count > 0) {
+                Assert.AreEqual(astack.Count, bstack.Count); // should never fail
+                a = astack[astack.Count - 1]; astack.RemoveAt(astack.Count - 1);
+                b = bstack[bstack.Count - 1]; bstack.RemoveAt(bstack.Count - 1);
+
+                // Verify that they have the same children (by name).
+                var achildren = a.ChildNames;
+                var bchildren = b.ChildNames;
+                Assert.That(achildren, Is.EquivalentTo(bchildren));
+
+                // Add the children to each stack.
+                foreach(var child in achildren) {
+                    astack.Add(a.GetChild(child));
+                    bstack.Add(b.GetChild(child));
+                }
+
+                // Verify that they have the same components.
+                var atypes = a.ComponentTypes;
+                var btypes = b.ComponentTypes;
+                Assert.That(atypes, Is.EquivalentTo(btypes));
+
+                foreach(var t in atypes) {
+                    var avalues = a.GetComponentValues(t);
+                    var bvalues = b.GetComponentValues(t);
+                    Assert.AreEqual(avalues.Count, bvalues.Count);
+
+                    if (t != "UnityEngine.Transform") {
+                        Assert.AreEqual(avalues, bvalues);
+                    } else {
+                        // Verify that the transforms are nearly (but don't require bitwise) equal.
+                        EditorJsonUtility.FromJsonOverwrite(avalues[0], aDummy);
+                        EditorJsonUtility.FromJsonOverwrite(bvalues[0], bDummy);
+                        var dist = Vector3.Distance(aDummy.localPosition, bDummy.localPosition);
+                        Assert.That(dist, Is.LessThan(1e-6), () => string.Format("position {0} vs {1} dist {2}",
+                                aDummy.localPosition, bDummy.localPosition, dist));
+
+                        dist = Vector3.Distance(aDummy.localScale, bDummy.localScale);
+                        Assert.That(dist, Is.LessThan(1e-6), () => string.Format("scale {0} vs {1} dist {2}",
+                                aDummy.localScale, bDummy.localScale, dist));
+
+                        dist = Quaternion.Angle(aDummy.localRotation, bDummy.localRotation);
+                        Assert.That(dist, Is.LessThan(1e-6), () => string.Format("rotation {0} vs {1} angle {2}",
+                                aDummy.localRotation.eulerAngles, bDummy.localRotation.eulerAngles, dist));
+                    }
+                }
+            }
         }
 
         public static void AssertAreDifferent(
@@ -72,7 +127,6 @@ FbxPrefab.FbxRepresentation History(GameObject go) {
 
         GameObject ModifySourceFbx()
         {
-
             // Modify the source fbx file:
             // - delete parent1
             // - add parent3
@@ -101,6 +155,7 @@ public void BasicTest() {
             AssertAreIdentical(m_originalRep, History(m_manualPrefab));
             AssertAreIdentical(m_originalRep, History(m_autoPrefab));
 
+            Debug.Log("Testing auto update");
             var newHierarchy = Rep(ModifySourceFbx());
             AssertAreDifferent(m_originalRep, newHierarchy);
 
@@ -117,6 +172,7 @@ public void BasicTest() {
             AssertAreIdentical(m_originalRep, History(m_manualPrefab));
 
             // Manual update, make sure it updated.
+            Debug.Log("Testing manual update");
             var manualPrefabComponent = m_manualPrefab.GetComponent<FbxPrefab>();
             manualPrefabComponent.SyncPrefab();
             AssertAreIdentical(newHierarchy, Rep(m_manualPrefab));
@@ -128,14 +184,17 @@ public void BasicTest() {
             // Illegal to set the source model to something that isn't an
             // asset.
             var go = CreateGameObject("foo");
+            Debug.Log("Testing SetSourceModel to scene object");
             Assert.That( () => manualPrefabComponent.SetSourceModel(go), Throws.Exception );
 
             // Illegal to set the source model to something that isn't an fbx
             // asset (it's a prefab).
+            Debug.Log("Testing SetSourceModel to prefab");
             Assert.That( () => manualPrefabComponent.SetSourceModel(m_autoPrefab), Throws.Exception );
 
             // Legal to set the source model to null. It doesn't change the
             // hierarchy or anything.
+            Debug.Log("Testing SetSourceModel to null");
             Assert.That( () => manualPrefabComponent.SetSourceModel(null), Throws.Nothing );
             Assert.IsNull(manualPrefabComponent.GetFbxAsset());
             AssertAreIdentical(newHierarchy, Rep(m_manualPrefab));
@@ -151,6 +210,7 @@ public void BasicTest() {
                     GetRandomFbxFilePath(), m_original);
             var newSource = AssetDatabase.LoadMainAssetAtPath(fbxAsset) as GameObject;
             Assert.IsTrue(newSource);
+            Debug.Log("Testing SetSourceModel relink");
             manualPrefabComponent.SetSourceModel(newSource);
             AssertAreIdentical(m_originalRep, Rep(m_manualPrefab));
             AssertAreIdentical(m_originalRep, History(m_manualPrefab));

Assets/FbxExporters/FbxPrefab.cs

@@ -145,25 +145,34 @@ public class FbxRepresentation
             /// The key is the name, which is assumed to be unique.
             /// The value is, recursively, the representation of that subtree.
             /// </summary>
-            Dictionary<string, FbxRepresentation> m_children;
+            Dictionary<string, FbxRepresentation> m_children = new Dictionary<string, FbxRepresentation>();
 
             /// <summary>
-            /// Children of this node.
+            /// Components of this node.
             /// The key is the name of the type of the Component. We accept that there may be several.
             /// The value is the json for the component, to be decoded with EditorJsonUtility.
+            ///
+            /// Note that we skip the FbxPrefab component because we never want to update that
+            /// automatically.
             /// </summary>
-            Dictionary<string, List<string>> m_components;
+            Dictionary<string, List<string>> m_components = new Dictionary<string, List<string>>();
 
             /// <summary>
             /// Build a hierarchical representation based on a transform.
             /// </summary>
-            public FbxRepresentation(Transform xfo)
+            public FbxRepresentation(Transform xfo, bool isRoot = true)
             {
                 m_children = new Dictionary<string, FbxRepresentation>();
                 foreach(Transform child in xfo) {
-                    m_children.Add(child.name, new FbxRepresentation(child));
+                    m_children.Add(child.name, new FbxRepresentation(child, isRoot: false));
                 }
                 foreach(var component in xfo.GetComponents<Component>()) {
+                    // Don't save the prefab link, to avoid a logic loop.
+                    if (component is FbxPrefab) { continue; }
+
+                    // Don't save the root transform, to allow importing at a place other than zero.
+                    if (isRoot && component is Transform) { continue; }
+
                     var typeName = component.GetType().ToString();
                     var jsonValue = UnityEditor.EditorJsonUtility.ToJson(component);
                     Append(ref m_components, typeName, jsonValue);
@@ -309,32 +318,22 @@ public string ToJson() {
                 return builder.ToString();
             }
 
-            public static bool IsLeaf(FbxRepresentation rep) {
-                return rep == null || rep.m_children == null;
-            }
-
-            public static HashSet<string> GetChildren(FbxRepresentation rep) {
-                if (IsLeaf(rep)) {
-                    return new HashSet<string>();
-                } else {
-                    return new HashSet<string>(rep.m_children.Keys);
-                }
-            }
+            public HashSet<string> ChildNames { get { return new HashSet<string> (m_children.Keys); } }
 
-            public static IEnumerable<KeyValuePair<string, List<string>>> GetComponents(FbxRepresentation rep) {
-                if (rep == null || rep.m_components == null) {
-                    return new KeyValuePair<string, List<string>>[0];
-                } else {
-                    return rep.m_components;
+            public FbxRepresentation GetChild(string childName) {
+                FbxRepresentation child;
+                if (m_children.TryGetValue(childName, out child)) {
+                    return child;
                 }
+                return null;
             }
 
-            public static FbxRepresentation Find(FbxRepresentation rep, string key) {
-                if (IsLeaf(rep)) { return null; }
+            public HashSet<string> ComponentTypes { get { return new HashSet<string> (m_components.Keys); } }
 
-                FbxRepresentation child;
-                if (rep.m_children.TryGetValue(key, out child)) {
-                    return child;
+            public List<string> GetComponentValues(string componentType) {
+                List<string> jsonValues;
+                if (m_components.TryGetValue(componentType, out jsonValues)) {
+                    return jsonValues;
                 }
                 return null;
             }
@@ -475,22 +474,23 @@ public static HashSet<string> GetAllNames(params Data [] data) {
             /// </summary>
             Dictionary<string, List<Component>> m_componentsToUpdate;
 
-            static void SetupDataHelper(Data data, FbxRepresentation fbxrep, string parent)
+            static void SetupDataHelper(Data data, FbxRepresentation fbxrep, string nodeName)
             {
-                foreach(var kvp in FbxRepresentation.GetComponents(fbxrep)) {
-                    var typename = kvp.Key;
-                    var jsonValues = kvp.Value;
-                    data.AddComponents(parent, typename, jsonValues);
+                foreach(var typename in fbxrep.ComponentTypes) {
+                    var jsonValues = fbxrep.GetComponentValues(typename);
+                    data.AddComponents(nodeName, typename, jsonValues);
                 }
-                foreach(var child in FbxRepresentation.GetChildren(fbxrep)) {
-                    data.AddNode(child, parent);
-                    SetupDataHelper(data, FbxRepresentation.Find(fbxrep, child), child);
+                foreach(var child in fbxrep.ChildNames) {
+                    data.AddNode(child, nodeName);
+                    SetupDataHelper(data, fbxrep.GetChild(child), child);
                 }
             }
 
             static void SetupData(ref Data data, FbxRepresentation fbxrep)
             {
                 Initialize(ref data);
+
+                // The root node has no name
                 SetupDataHelper(data, fbxrep, "");
             }