From 956a1e59bbbfaf2808cb45710a24ce86c30e147e Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Tue, 25 Jan 2022 12:58:17 -0600 Subject: [PATCH] [LYN-9039] Object stream write response change (#7085) * Updated the ObjectStreamWriteOverride Callback to return a ObjectStreamWriteOverrideResponse. The ObjectStreamWriteOverrideResponse allows the callback to indicate that the default ObjectStream::WriteElement behavior should occur if the callback hasn't implemented the write itself. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed forward declaration not matching actual enum class name Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add unit tests Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Remove unused member Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Add virtual destructor for test class Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Change string literal to not have tabs in it Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Co-authored-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../AzCore/Serialization/ObjectStream.cpp | 18 ++- .../AzCore/Serialization/ObjectStream.h | 14 ++- .../Serialization/std/VariantReflection.inl | 7 +- Code/Framework/AzCore/Tests/Serialization.cpp | 103 ++++++++++++++++++ .../AzFramework/Asset/AssetBundleManifest.cpp | 7 +- 5 files changed, 142 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.cpp b/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.cpp index ac44ac150c..6132cf0a56 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.cpp @@ -1668,9 +1668,23 @@ namespace AZ SerializeContext::ENUM_ACCESS_FOR_READ, &m_errorLogger ); - if (objectStreamWriteOverrideCB.Invoke(callContext, objectPtr, *classData, classElement)) + if (ObjectStreamWriteOverrideResponse writeResponse; + objectStreamWriteOverrideCB.Read(writeResponse, callContext, objectPtr, *classData, classElement)) { - return false; + switch (writeResponse) + { + case ObjectStreamWriteOverrideResponse::FallbackToDefaultWrite: + break; + case ObjectStreamWriteOverrideResponse::AbortWrite: + m_errorLogger.ReportError(AZStd::string::format("ObjectStream Write Element Override callback has aborted the write for class data %s", + classData->m_name).c_str()); + [[fallthrough]]; + case ObjectStreamWriteOverrideResponse::CompletedWrite: + return false; + default: + AZ_Error("Serialize", false, "Invalid Response %d returned from the ObjectStream Write Element Override callback", static_cast(writeResponse)); + return false; + } } else { diff --git a/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.h b/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.h index 9f9b3fc9f0..1985821c7b 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.h +++ b/Code/Framework/AzCore/AzCore/Serialization/ObjectStream.h @@ -49,14 +49,24 @@ namespace AZ static const AZ::Crc32 ObjectStreamWriteElementOverride = AZ_CRC("ObjectStreamWriteElementOverride", 0x35eb659f); } + enum class ObjectStreamWriteOverrideResponse + { + CompletedWrite, + FallbackToDefaultWrite, + AbortWrite + }; + AZ_TYPE_INFO_SPECIALIZE(ObjectStreamWriteOverrideResponse, "{BDF960A8-0F18-4E9D-96DA-F800A122C42D}"); + ///< Callback that the object stream invokes to override saving an instance of the registered class ///< @param callContext EnumerateInstanceCallContext which contains the WriteElement BeingElemCB and the CloseElement EndElemCB ///< the callContext parameter can be passed to the SerializeContext::EnumerateInstance to continue object stream writing ///< @param classPtr class type which is of pointer to the type represented by the m_typeId value ///< @param classData reference to this instance Class Data that will be supplied to the callback ///< @param classElement class element pointer which contains information about the element being serialized. - ///< root elements do not not have a valid class element pointer - using ObjectStreamWriteOverrideCB = AZStd::function; AZ_TYPE_INFO_SPECIALIZE(ObjectStreamWriteOverrideCB, "{87B1A36B-8C8A-42B6-A0B5-E770D9FDBAD4}"); diff --git a/Code/Framework/AzCore/AzCore/Serialization/std/VariantReflection.inl b/Code/Framework/AzCore/AzCore/Serialization/std/VariantReflection.inl index 06d4f76c80..33f9995752 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/std/VariantReflection.inl +++ b/Code/Framework/AzCore/AzCore/Serialization/std/VariantReflection.inl @@ -12,6 +12,8 @@ namespace AZ { + enum class ObjectStreamWriteOverrideResponse; + namespace VariantSerializationInternal { template @@ -480,7 +482,7 @@ namespace AZ } } private: - static void ObjectStreamWriter(SerializeContext::EnumerateInstanceCallContext& callContext, const void* variantPtr, + static ObjectStreamWriteOverrideResponse ObjectStreamWriter(SerializeContext::EnumerateInstanceCallContext& callContext, const void* variantPtr, [[maybe_unused]] const SerializeContext::ClassData& variantClassData, const SerializeContext::ClassElement* variantClassElement) { auto alternativeVisitor = [&callContext, variantClassElement](auto&& elementAlt) @@ -503,6 +505,9 @@ namespace AZ }; AZStd::visit(AZStd::move(alternativeVisitor), *reinterpret_cast(variantPtr)); + // To avoid including ObjectStream.h into this file, we static cast the value of 0 + // to an AZ::ObjectStreamWriteElemntResponse which corresponds to the CompletedWrite enum value + return static_cast(0); } VariantSerializationInternal::AZStdVariantContainer m_variantContainer; diff --git a/Code/Framework/AzCore/Tests/Serialization.cpp b/Code/Framework/AzCore/Tests/Serialization.cpp index bbfafc352f..102fa8b2c8 100644 --- a/Code/Framework/AzCore/Tests/Serialization.cpp +++ b/Code/Framework/AzCore/Tests/Serialization.cpp @@ -720,6 +720,49 @@ namespace SerializeTestClasses { AZStd::intrusive_ptr m_intrusivePtr; AZStd::unique_ptr m_uniquePtr; }; + + struct ElementOverrideType + { + AZ_RTTI(ElementOverrideType, "{BAA18B6C-3CB3-476C-8B41-21EA7CE1F4CF}"); + AZ_CLASS_ALLOCATOR(ElementOverrideType, AZ::SystemAllocator, 0); + + virtual ~ElementOverrideType() = default; + + static AZ::ObjectStreamWriteOverrideResponse Writer( + AZ::SerializeContext::EnumerateInstanceCallContext& callContext, + const void* object, + const AZ::SerializeContext::ClassData&, + const AZ::SerializeContext::ClassElement*) + { + auto ptr = static_cast(object); + + if(ptr) + { + switch(ptr->m_field) + { + case 0: + { + float output{}; + callContext.m_context->EnumerateInstanceConst(&callContext, &output, azrtti_typeid(), nullptr, nullptr); + return AZ::ObjectStreamWriteOverrideResponse::CompletedWrite; + } + case 1: + return AZ::ObjectStreamWriteOverrideResponse::FallbackToDefaultWrite; + } + } + + return AZ::ObjectStreamWriteOverrideResponse::AbortWrite; + } + + static void Reflect(AZ::SerializeContext& sc) + { + sc.Class() + ->Attribute(AZ::SerializeContextAttributes::ObjectStreamWriteElementOverride, &ElementOverrideType::Writer) + ->Field("field", &ElementOverrideType::m_field); + } + + int m_field = 0; + }; } //SerializeTestClasses namespace AZ @@ -1698,6 +1741,66 @@ namespace UnitTest } } + TEST_F(Serialization, ElementOverrideTest_DefaultSerializationWorks) + { + ElementOverrideType::Reflect(*m_serializeContext); + + ElementOverrideType testType; + testType.m_field = 1; // Our custom serializer will use the default output when this value is 1 + + AZStd::vector buffer; + IO::ByteContainerStream> stream(&buffer); + ASSERT_TRUE(Utils::SaveObjectToStream(stream, DataStream::ST_XML, &testType, m_serializeContext.get())); + + constexpr const char* expectedValue = + R"()" "\n" + "\t" R"()" "\n" + "\t\t" R"()" "\n" + "\t" R"()" "\n" + R"()"; + + AZStd::string result(buffer.data(), stream.GetLength()); + AZ::StringFunc::TrimWhiteSpace(result, true, true); + + EXPECT_STREQ(result.c_str(), expectedValue); + } + + TEST_F(Serialization, ElementOverrideTest_CustomSerializationWorks) + { + ElementOverrideType::Reflect(*m_serializeContext); + + ElementOverrideType testType; + testType.m_field = 0; // Our custom serializer will do its own output when this value is 0 + + AZStd::vector buffer; + IO::ByteContainerStream> stream(&buffer); + ASSERT_TRUE(Utils::SaveObjectToStream(stream, DataStream::ST_XML, &testType, m_serializeContext.get())); + + constexpr const char* expectedValue = + R"()" "\n" + "\t" R"()" "\n" + R"()"; + + AZStd::string result(buffer.data(), stream.GetLength()); + AZ::StringFunc::TrimWhiteSpace(result, true, true); + + EXPECT_STREQ(result.c_str(), expectedValue); + } + + TEST_F(Serialization, ElementOverrideTest_FailureCase) + { + ElementOverrideType::Reflect(*m_serializeContext); + + ElementOverrideType testType; + testType.m_field = 2; // Our custom serializer will report a failure when this value is not 0/1 + + AZStd::vector buffer; + IO::ByteContainerStream> stream(&buffer); + AZ_TEST_START_TRACE_SUPPRESSION; + ASSERT_FALSE(Utils::SaveObjectToStream(stream, DataStream::ST_XML, &testType, m_serializeContext.get())); + AZ_TEST_STOP_TRACE_SUPPRESSION(1); + } + TEST_F(Serialization, ContainerTypeContainedTypeDiffersByPointer) { ContainersTest::ReflectVectorOfInts(m_serializeContext.get()); diff --git a/Code/Framework/AzFramework/AzFramework/Asset/AssetBundleManifest.cpp b/Code/Framework/AzFramework/AzFramework/Asset/AssetBundleManifest.cpp index b4da5745be..0c941fc016 100644 --- a/Code/Framework/AzFramework/AzFramework/Asset/AssetBundleManifest.cpp +++ b/Code/Framework/AzFramework/AzFramework/Asset/AssetBundleManifest.cpp @@ -15,7 +15,7 @@ namespace AzFramework { // Redirects writing of the AssetBundleManifest to an older version if the bundle version // is not set to the current version - static void OldBundleManifestWriter(AZ::SerializeContext::EnumerateInstanceCallContext& callContext, const void* bundleManifestPointer, + static AZ::ObjectStreamWriteOverrideResponse OldBundleManifestWriter(AZ::SerializeContext::EnumerateInstanceCallContext& callContext, const void* bundleManifestPointer, const AZ::SerializeContext::ClassData&, const AZ::SerializeContext::ClassElement* assetBundleManifestClassElement); static bool BundleManifestVersionConverter(AZ::SerializeContext& context, AZ::SerializeContext::DataElementNode& rootElement); @@ -67,7 +67,7 @@ namespace AzFramework return true; } - void OldBundleManifestWriter(AZ::SerializeContext::EnumerateInstanceCallContext& callContext, const void* bundleManifestPointer, + AZ::ObjectStreamWriteOverrideResponse OldBundleManifestWriter(AZ::SerializeContext::EnumerateInstanceCallContext& callContext, const void* bundleManifestPointer, const AZ::SerializeContext::ClassData&, const AZ::SerializeContext::ClassElement* assetBundleManifestClassElement) { // Copy the AssetBundleManifest current version instance to the AssetBundleManifest V2 instance @@ -145,7 +145,10 @@ namespace AzFramework ReflectAssetBundleManifestV2(serializeContext); serializeContext->DisableRemoveReflection(); AssetBundleManifest::ReflectSerialize(serializeContext); + return AZ::ObjectStreamWriteOverrideResponse::CompletedWrite; } + + return AZ::ObjectStreamWriteOverrideResponse::FallbackToDefaultWrite; } const AZStd::vector& AssetBundleManifest::GetLevelDirectories() const