From a766e3af5c08a63e2f6edcddae26376ef8b57dd4 Mon Sep 17 00:00:00 2001 From: chcurran Date: Tue, 8 Jun 2021 13:54:25 -0700 Subject: [PATCH 1/2] Fixes for internal if-branch node parser bug (LYN-4347) and exposing properties for AZStd::tuple (LYN-3910) --- .../AzCore/RTTI/AzStdOnDemandPrettyName.inl | 51 +- .../AzCore/RTTI/AzStdOnDemandReflection.inl | 24 +- .../Grammar/AbstractCodeModel.cpp | 36 +- ...ionIfBranchWithConnectedInput.scriptcanvas | 2499 +++++++++++++++++ .../Tests/ScriptCanvas_RuntimeInterpreted.cpp | 5 + 5 files changed, 2590 insertions(+), 25 deletions(-) create mode 100644 Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl index 26e269573d..4f4a21301c 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandPrettyName.inl @@ -161,7 +161,56 @@ namespace AZ return "A pair is an fixed size collection of two elements."; } }; - + + template + void GetTypeNamesFold(AZStd::vector& result, AZ::BehaviorContext& context) + { + result.push_back(OnDemandPrettyName::Get(context)); + }; + + template + void GetTypeNames(AZStd::vector& result, AZ::BehaviorContext& context) + { + (GetTypeNamesFold(result, context), ...); + }; + + template + void GetTypeNamesFold(AZStd::string& result, AZ::BehaviorContext& context) + { + if (!result.empty()) + { + result += ", "; + } + + result += OnDemandPrettyName::Get(context); + }; + + template + void GetTypeNames(AZStd::string& result, AZ::BehaviorContext& context) + { + (GetTypeNamesFold(result, context), ...); + }; + + template + struct OnDemandPrettyName> + { + static AZStd::string Get(AZ::BehaviorContext& context) + { + AZStd::string typeNames; + GetTypeNames(typeNames, context); + return AZStd::string::format("Tuple<%s>", typeNames.c_str()); + } + }; + + template + struct OnDemandToolTip> + { + static AZStd::string Get(AZ::BehaviorContext&) + { + return "A tuple is an fixed size collection of any number of any type of element."; + } + }; + template struct OnDemandPrettyName< AZStd::unordered_map > { diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl index 537d3e5c83..2132dff3c4 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl @@ -813,20 +813,27 @@ namespace AZ { using ContainerType = AZStd::tuple; - template - static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder) + template + static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames, [[maybe_unused]] size_t inputIndex) { const AZStd::string methodName = AZStd::string::format("Get%zu", Index); - builder->Method(methodName.data(), [](ContainerType& value) { return AZStd::get(value); }) - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) + builder->Method(methodName.data(), [](ContainerType& thisPointer) { return AZStd::get(thisPointer); }) + ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::List) ->Attribute(AZ::ScriptCanvasAttributes::TupleGetFunctionIndex, Index) ; + + builder->Property + ( AZStd::string::format("element_%zu_%s", Index, typeNames[Index].c_str()).c_str() + , [](ContainerType& thisPointer) { return AZStd::get(thisPointer); } + , [](ContainerType& thisPointer, const t_Arg& element) { AZStd::get(thisPointer) = element; }); } - template + template static void ReflectUnpackMethods(BehaviorContext::ClassBuilder& builder, AZStd::index_sequence) { - (ReflectUnpackMethodFold(builder), ...); + AZStd::vector typeNames; + ScriptCanvasOnDemandReflection::GetTypeNames(typeNames, *builder.m_context); + (ReflectUnpackMethodFold(builder, typeNames, Indices), ...); } static void Reflect(ReflectContext* context) @@ -851,9 +858,10 @@ namespace AZ ->Attribute(AZ::ScriptCanvasAttributes::TupleConstructorFunction, constructorHolder) ; - ReflectUnpackMethods(builder, AZStd::make_index_sequence{}); + ReflectUnpackMethods(builder, AZStd::make_index_sequence{}); + builder->Method("GetSize", []() { return AZStd::tuple_size::value; }) - ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::All) + ->Attribute(AZ::Script::Attributes::ExcludeFrom, AZ::Script::Attributes::ExcludeFlags::List) ; } } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp index 2a485c9501..bc07d16a7e 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp @@ -3356,23 +3356,27 @@ namespace ScriptCanvas if (executionIf->GetId().m_node->IsIfBranchPrefacedWithBooleanExpression()) { - auto removeChildOutcome = RemoveChild(executionIf->ModParent(), executionIf); - if (!removeChildOutcome.IsSuccess()) + ExecutionTreePtr booleanExpression; + { - AddError(executionIf->GetNodeId(), executionIf, ScriptCanvas::ParseErrors::FailedToRemoveChild); + auto removeChildOutcome = RemoveChild(executionIf->ModParent(), executionIf); + if (!removeChildOutcome.IsSuccess()) + { + AddError(executionIf->GetNodeId(), executionIf, ScriptCanvas::ParseErrors::FailedToRemoveChild); + } + + if (!IsErrorFree()) + { + return; + } + + const auto indexAndChild = removeChildOutcome.TakeValue(); + + booleanExpression = CreateChild(executionIf->ModParent(), executionIf->GetId().m_node, executionIf->GetId().m_slot); + executionIf->ModParent()->InsertChild(indexAndChild.first, { indexAndChild.second.m_slot, indexAndChild.second.m_output, booleanExpression }); + executionIf->SetParent(booleanExpression); } - if (!IsErrorFree()) - { - return; - } - - const auto indexAndChild = removeChildOutcome.TakeValue(); - - ExecutionTreePtr booleanExpression = CreateChild(executionIf->ModParent(), executionIf->GetId().m_node, executionIf->GetId().m_slot); - executionIf->ModParent()->InsertChild(indexAndChild.first, { indexAndChild.second.m_slot, indexAndChild.second.m_output, booleanExpression }); - executionIf->SetParent(booleanExpression); - // make a condition here auto symbol = CheckLogicalExpressionSymbol(booleanExpression); if (symbol != Symbol::FunctionCall && symbol != Symbol::Count) @@ -3402,7 +3406,7 @@ namespace ScriptCanvas return; } - const auto indexAndChild2 = removeChildOutcome.TakeValue(); + const auto indexAndChild2 = removeChildOutcome2.TakeValue(); // parse if statement internal function ExecutionTreePtr internalFunction = CreateChild(booleanExpression->ModParent(), booleanExpression->GetId().m_node, booleanExpression->GetId().m_slot); @@ -4782,7 +4786,7 @@ namespace ScriptCanvas { PropertyExtractionPtr extraction = AZStd::make_shared(); extraction->m_slot = slot; - extraction->m_name = propertyField.first; + extraction->m_name = AZ::ReplaceCppArtifacts(propertyField.first); execution->AddPropertyExtractionSource(slot, extraction); } else diff --git a/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas b/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas new file mode 100644 index 0000000000..38df5159c2 --- /dev/null +++ b/Gems/ScriptCanvasTesting/Assets/ScriptCanvas/UnitTests/LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput.scriptcanvas @@ -0,0 +1,2499 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp b/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp index 1633b0fc51..5bda0f3941 100644 --- a/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp +++ b/Gems/ScriptCanvasTesting/Code/Tests/ScriptCanvas_RuntimeInterpreted.cpp @@ -90,6 +90,11 @@ public: } }; +TEST_F(ScriptCanvasTestFixture, ParseFunctionIfBranchWithConnectedInput) +{ + RunUnitTestGraph("LY_SC_UnitTest_ParseFunctionIfBranchWithConnectedInput"); +} + TEST_F(ScriptCanvasTestFixture, UseRawBehaviorProperties) { RunUnitTestGraph("LY_SC_UnitTest_UseRawBehaviorProperties"); From 8cb6ef0721bb295b74bbf5c72c3bd1bc1d83ff86 Mon Sep 17 00:00:00 2001 From: chcurran Date: Tue, 8 Jun 2021 16:39:36 -0700 Subject: [PATCH 2/2] Fix tmeplate arg names --- .../AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl index 2132dff3c4..9bda5cdcce 100644 --- a/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl +++ b/Code/Framework/AzCore/AzCore/RTTI/AzStdOnDemandReflection.inl @@ -813,8 +813,8 @@ namespace AZ { using ContainerType = AZStd::tuple; - template - static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames, [[maybe_unused]] size_t inputIndex) + template + static void ReflectUnpackMethodFold(BehaviorContext::ClassBuilder& builder, const AZStd::vector& typeNames) { const AZStd::string methodName = AZStd::string::format("Get%zu", Index); builder->Method(methodName.data(), [](ContainerType& thisPointer) { return AZStd::get(thisPointer); }) @@ -825,15 +825,15 @@ namespace AZ builder->Property ( AZStd::string::format("element_%zu_%s", Index, typeNames[Index].c_str()).c_str() , [](ContainerType& thisPointer) { return AZStd::get(thisPointer); } - , [](ContainerType& thisPointer, const t_Arg& element) { AZStd::get(thisPointer) = element; }); + , [](ContainerType& thisPointer, const Targ& element) { AZStd::get(thisPointer) = element; }); } - template + template static void ReflectUnpackMethods(BehaviorContext::ClassBuilder& builder, AZStd::index_sequence) { AZStd::vector typeNames; ScriptCanvasOnDemandReflection::GetTypeNames(typeNames, *builder.m_context); - (ReflectUnpackMethodFold(builder, typeNames, Indices), ...); + (ReflectUnpackMethodFold(builder, typeNames), ...); } static void Reflect(ReflectContext* context)