From 086ddc96bbdb7ead36a6e3d5c6af662b8aa9fb37 Mon Sep 17 00:00:00 2001 From: Wojtek Figat Date: Fri, 14 Feb 2025 17:07:33 +0100 Subject: [PATCH] Fix incorrect prefab serialization to correctly handle diff on object references to prefab objects #3136 --- Source/Engine/Serialization/JsonTools.cpp | 2 +- Source/Engine/Serialization/Serialization.cpp | 13 +++++ Source/Engine/Serialization/Serialization.h | 14 ++++- Source/Engine/Tests/TestPrefabs.cpp | 57 +++++++++++++++++++ 4 files changed, 82 insertions(+), 4 deletions(-) diff --git a/Source/Engine/Serialization/JsonTools.cpp b/Source/Engine/Serialization/JsonTools.cpp index 3d6ec8ebf..be711f77a 100644 --- a/Source/Engine/Serialization/JsonTools.cpp +++ b/Source/Engine/Serialization/JsonTools.cpp @@ -41,7 +41,7 @@ void ChangeIds(rapidjson_flax::Value& obj, rapidjson_flax::Document& document, c '0','0','0','0','0','0','0','0','0','0', '0','0','0','0','0','0','0','0','0','0', '0','0' - // @formatter:on + // @formatter:on }; static const char* digits = "0123456789abcdef"; uint32 n = value.A; diff --git a/Source/Engine/Serialization/Serialization.cpp b/Source/Engine/Serialization/Serialization.cpp index d6aae1991..aa60ec944 100644 --- a/Source/Engine/Serialization/Serialization.cpp +++ b/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 diff --git a/Source/Engine/Serialization/Serialization.h b/Source/Engine/Serialization/Serialization.h index 9c3e4f697..125803f5b 100644 --- a/Source/Engine/Serialization/Serialization.h +++ b/Source/Engine/Serialization/Serialization.h @@ -441,10 +441,12 @@ namespace Serialization // Scripting Object + FLAXENGINE_API bool ShouldSerialize(const SceneObject* v, const SceneObject* other); + template inline typename TEnableIf::Value, bool>::Type ShouldSerialize(const T*& v, const void* otherObj) { - return !otherObj || v != *(T**)otherObj; + return !otherObj || v != *(const T**)otherObj; } template inline typename TEnableIf::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 + inline typename TEnableIf::Value, bool>::Type ShouldSerialize(const T*& v, const void* otherObj) + { + return !otherObj || ShouldSerialize((const SceneObject*)v, *(const SceneObject**)otherObj); + } + // Scripting Object Reference template inline bool ShouldSerialize(const ScriptingObjectReference& v, const void* otherObj) { - return !otherObj || v.Get() != ((ScriptingObjectReference*)otherObj)->Get(); + return !otherObj || ShouldSerialize(v.Get(), ((ScriptingObjectReference*)otherObj)->Get()); } template inline void Serialize(ISerializable::SerializeStream& stream, const ScriptingObjectReference& v, const void* otherObj) @@ -486,7 +494,7 @@ namespace Serialization template inline bool ShouldSerialize(const SoftObjectReference& v, const void* otherObj) { - return !otherObj || v.Get() != ((SoftObjectReference*)otherObj)->Get(); + return !otherObj || ShouldSerialize(v.Get(), ((SoftObjectReference*)otherObj)->Get()); } template inline void Serialize(ISerializable::SerializeStream& stream, const SoftObjectReference& v, const void* otherObj) diff --git a/Source/Engine/Tests/TestPrefabs.cpp b/Source/Engine/Tests/TestPrefabs.cpp index 8c6e29249..fd8ffa1e2 100644 --- a/Source/Engine/Tests/TestPrefabs.cpp +++ b/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 = Content::CreateVirtualAsset(); + 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 instanceA = PrefabManager::SpawnPrefab(prefab); + ScriptingObjectReference 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()->DirectionalInscatteringLight == instanceA->Children[0]); + instanceA.As()->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()->DirectionalInscatteringLight == instanceB->Children[1]); + + // Cleanup + instanceA->DeleteObject(); + instanceB->DeleteObject(); + Content::DeleteAsset(prefab); + } }