Fix incorrect prefab serialization to correctly handle diff on object references to prefab objects

#3136

Source/Engine/Serialization/Serialization.cpp

@@ -23,6 +23,7 @@
 #include "Engine/Scripting/ManagedCLR/MClass.h"
 #include "Engine/Scripting/ScriptingObjectReference.h"
 #include "Engine/Content/Asset.h"
+#include "Engine/Level/SceneObject.h"
 #include "Engine/Utilities/Encryption.h"
 
 void ISerializable::DeserializeIfExists(DeserializeStream& stream, const char* memberName, ISerializeModifier* modifier)
@@ -789,4 +790,16 @@ void Serialization::Deserialize(ISerializable::DeserializeStream& stream, Matrix
     DESERIALIZE_HELPER(stream, "M44", v.M44, 0);
 }
 
+bool Serialization::ShouldSerialize(const SceneObject* v, const SceneObject* other)
+{
+    bool result = v != other;
+    if (result && v && other && v->HasPrefabLink() && other->HasPrefabLink())
+    {
+        // Special case when saving reference to prefab object and the objects are different but the point to the same prefab object
+        // In that case, skip saving reference as it's defined in prefab (will be populated via IdsMapping during deserialization)
+        result &= v->GetPrefabObjectID() != other->GetPrefabObjectID();
+    }
+    return result;
+}
+
 #undef DESERIALIZE_HELPER

Source/Engine/Serialization/Serialization.h

@@ -441,10 +441,12 @@ namespace Serialization
 
     // Scripting Object
 
+    FLAXENGINE_API bool ShouldSerialize(const SceneObject* v, const SceneObject* other);
+
     template<typename T>
     inline typename TEnableIf<TIsBaseOf<ScriptingObject, T>::Value, bool>::Type ShouldSerialize(const T*& v, const void* otherObj)
     {
-        return !otherObj || v != *(T**)otherObj;
+        return !otherObj || v != *(const T**)otherObj;
     }
     template<typename T>
     inline typename TEnableIf<TIsBaseOf<ScriptingObject, T>::Value>::Type Serialize(ISerializable::SerializeStream& stream, const T*& v, const void* otherObj)
@@ -460,12 +462,18 @@ namespace Serialization
         v = (T*)::FindObject(id, T::GetStaticClass());
     }
 
+    template<typename T>
+    inline typename TEnableIf<TIsBaseOf<SceneObject, T>::Value, bool>::Type ShouldSerialize(const T*& v, const void* otherObj)
+    {
+        return !otherObj || ShouldSerialize((const SceneObject*)v, *(const SceneObject**)otherObj);
+    }
+
     // Scripting Object Reference
 
     template<typename T>
     inline bool ShouldSerialize(const ScriptingObjectReference<T>& v, const void* otherObj)
     {
-        return !otherObj || v.Get() != ((ScriptingObjectReference<T>*)otherObj)->Get();
+        return !otherObj || ShouldSerialize(v.Get(), ((ScriptingObjectReference<T>*)otherObj)->Get());
     }
     template<typename T>
     inline void Serialize(ISerializable::SerializeStream& stream, const ScriptingObjectReference<T>& v, const void* otherObj)
@@ -486,7 +494,7 @@ namespace Serialization
     template<typename T>
     inline bool ShouldSerialize(const SoftObjectReference<T>& v, const void* otherObj)
     {
-        return !otherObj || v.Get() != ((SoftObjectReference<T>*)otherObj)->Get();
+        return !otherObj || ShouldSerialize(v.Get(), ((SoftObjectReference<T>*)otherObj)->Get());
     }
     template<typename T>
     inline void Serialize(ISerializable::SerializeStream& stream, const SoftObjectReference<T>& v, const void* otherObj)

Source/Engine/Tests/TestPrefabs.cpp

@@ -5,6 +5,8 @@
 #include "Engine/Core/Log.h"
 #include "Engine/Level/Actor.h"
 #include "Engine/Level/Actors/EmptyActor.h"
+#include "Engine/Level/Actors/DirectionalLight.h"
+#include "Engine/Level/Actors/ExponentialHeightFog.h"
 #include "Engine/Level/Prefabs/Prefab.h"
 #include "Engine/Level/Prefabs/PrefabManager.h"
 #include "Engine/Scripting/ScriptingObjectReference.h"
@@ -557,4 +559,59 @@ TEST_CASE("Prefabs")
         Content::DeleteAsset(prefabNested1);
         Content::DeleteAsset(prefabBase);
     }
+    SECTION("Test Applying Prefab ChangeTo Object References")
+    {
+        // https://github.com/FlaxEngine/FlaxEngine/issues/3136
+
+        // Create Prefab
+        AssetReference<Prefab> prefab = Content::CreateVirtualAsset<Prefab>();
+        REQUIRE(prefab);
+        Guid id;
+        Guid::Parse("690e55514cd6fdc2a269429a2bf84133", id);
+        prefab->ChangeID(id);
+        auto prefabInit = prefab->Init(Prefab::TypeName,
+                                         "["
+                                         "{"
+                                         "\"ID\": \"fc3f88cf413c2e668039a0bb7429900d\","
+                                         "\"TypeName\": \"FlaxEngine.ExponentialHeightFog\","
+                                         "\"Name\": \"Fog\","
+                                         "\"DirectionalInscatteringLight\": \"44873cc44e950c754f0c7bb59dd432d6\""
+                                         "},"
+                                         "{"
+                                         "\"ID\": \"44873cc44e950c754f0c7bb59dd432d6\","
+                                         "\"TypeName\": \"FlaxEngine.DirectionalLight\","
+                                         "\"ParentID\": \"fc3f88cf413c2e668039a0bb7429900d\","
+                                         "\"Name\": \"Sun 1\""
+                                         "},"
+                                         "{"
+                                         "\"ID\": \"583f91604b622e3b7aa698b51c9966d6\","
+                                         "\"TypeName\": \"FlaxEngine.DirectionalLight\","
+                                         "\"ParentID\": \"fc3f88cf413c2e668039a0bb7429900d\","
+                                         "\"Name\": \"Sun 2\""
+                                         "}"
+                                         "]");
+        REQUIRE(!prefabInit);
+        
+        // Spawn test instances
+        ScriptingObjectReference<Actor> instanceA = PrefabManager::SpawnPrefab(prefab);
+        ScriptingObjectReference<Actor> instanceB = PrefabManager::SpawnPrefab(prefab);
+
+        // Swap reference from Sun 1 to Sun 2 on a Fog
+        REQUIRE(instanceA);
+        REQUIRE(instanceA->Children.Count() == 2);
+        CHECK(instanceA.As<ExponentialHeightFog>()->DirectionalInscatteringLight == instanceA->Children[0]);
+        instanceA.As<ExponentialHeightFog>()->DirectionalInscatteringLight = (DirectionalLight*)instanceA->Children[1];
+
+        // Apply change on instance A and verify it's applied on instance B
+        bool applyResult = PrefabManager::ApplyAll(instanceA);
+        REQUIRE(!applyResult);
+        REQUIRE(instanceB);
+        REQUIRE(instanceB->Children.Count() == 2);
+        CHECK(instanceB.As<ExponentialHeightFog>()->DirectionalInscatteringLight == instanceB->Children[1]);
+
+        // Cleanup
+        instanceA->DeleteObject();
+        instanceB->DeleteObject();
+        Content::DeleteAsset(prefab);
+    }
 }