From ddc60041d345e97ebd2a62683cd99166ed7295db Mon Sep 17 00:00:00 2001 From: AMZN-koppersr <82230785+AMZN-koppersr@users.noreply.github.com> Date: Wed, 16 Jun 2021 16:40:46 -0700 Subject: [PATCH] Addressing PR feedback --- .../Json/BasicContainerSerializer.cpp | 2 +- .../AzCore/Serialization/Json/DoubleSerializer.cpp | 2 +- .../AzCore/Serialization/Json/TupleSerializer.cpp | 14 +++++++------- .../AzCore/Tests/AssetJsonSerializerTests.cpp | 6 +++--- .../Serialization/Json/ArraySerializerTests.cpp | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp index 4eb4748eb5..d61366413f 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/BasicContainerSerializer.cpp @@ -255,7 +255,7 @@ namespace AZ size_t addedCount = container->Size(outputValue) - containerSize; if (addedCount > 0) { - // Values were added which means the container is no longer in its default state of being emtpy. + // Values were added which means the container is no longer in its default state of being empty. retVal.Combine(JSR::ResultCode(JSR::Tasks::ReadField, JSR::Outcomes::Success)); } AZStd::string_view message = diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp index 14f2eaae6f..be885dfaa9 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/DoubleSerializer.cpp @@ -73,7 +73,7 @@ namespace AZ if (isExplicitDefault) { *outputValue = 0.0f; - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Double value set to default of 0.0."); + return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::DefaultsUsed, "Floating point value set to default of 0.0."); } switch (inputValue.GetType()) diff --git a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp index fd8951517a..e8a6f549a7 100644 --- a/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp +++ b/Code/Framework/AzCore/AzCore/Serialization/Json/TupleSerializer.cpp @@ -170,13 +170,6 @@ namespace AZ }; container->EnumTypes(typeCountCallback); - rapidjson::SizeType arraySize = isNewInstance ? typeCount : inputValue.Size(); - if (arraySize < typeCount) - { - return context.Report(JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, - "Not enough entries in array to load an AZStd::pair or AZStd::tuple from."); - } - AZStd::vector classElements; classElements.reserve(typeCount); auto typeEnumCallback = [&classElements](const Uuid&, const SerializeContext::ClassElement* genericClassElement) @@ -214,6 +207,13 @@ namespace AZ } else { + if (inputValue.Size() < typeCount) + { + return context.Report( + JSR::Tasks::ReadField, JSR::Outcomes::Unsupported, + "Not enough entries in array to load an AZStd::pair or AZStd::tuple from."); + } + rapidjson::SizeType arrayIndex = 0; size_t numElementsWritten = 0; for (size_t i = 0; i < typeCount; ++i) diff --git a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp index ceff314c81..47dba62ec8 100644 --- a/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/AssetJsonSerializerTests.cpp @@ -168,9 +168,9 @@ namespace JsonSerializationTests { features.EnableJsonType(rapidjson::kObjectType); features.m_typeToInject = rapidjson::kNullType; - // The type information in the Serialize Context is incomplete so this test will fail. - // This is because assets have traditionally been treated as a special case, so there's - // information missing in the Json Serialization to deal with these. + // Assets are not fully registered with the Serialize Context for historical reasons. Due to the missing + // information the Json Serializer Conformity Tests can't run the subsection of tests that explicitly + // require the missing information. features.m_enableNewInstanceTests = false; } diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp index 1f39feccbd..f696a9d270 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/ArraySerializerTests.cpp @@ -257,7 +257,7 @@ namespace JsonSerializationTests { Base::ConfigureFeatures(features); // These tests don't work with pointers because there'll be a random value in the pointer - // which the Json Serialization try to delete. The POD version of these tests already cover + // which the Json Serialization will try to delete. The POD version of these tests already cover // these cases. features.m_enableNewInstanceTests = false; }