From 0136c924674d6eb1550d69e7b5c158f9adb78f21 Mon Sep 17 00:00:00 2001 From: chcurran <82187351+carlitosan@users.noreply.github.com> Date: Wed, 30 Jun 2021 16:55:26 -0700 Subject: [PATCH] PR white space feedback, better Lua print error message, fix ACM static variable culling Signed-off-by: chcurran <82187351+carlitosan@users.noreply.github.com> --- .../AzCore/AzCore/Script/ScriptDebug.cpp | 5 +- .../Grammar/AbstractCodeModel.cpp | 67 +++++++++++++------ .../ScriptCanvas/Grammar/AbstractCodeModel.h | 2 + .../ScriptUserDataSerializer.cpp | 2 +- .../Code/scriptcanvasgem_common_files.cmake | 3 +- ...scriptcanvasgem_editor_builder_files.cmake | 2 +- 6 files changed, 52 insertions(+), 29 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Script/ScriptDebug.cpp b/Code/Framework/AzCore/AzCore/Script/ScriptDebug.cpp index 2f2ac5bdfa..c10f6d7826 100644 --- a/Code/Framework/AzCore/AzCore/Script/ScriptDebug.cpp +++ b/Code/Framework/AzCore/AzCore/Script/ScriptDebug.cpp @@ -24,7 +24,6 @@ namespace AZ //========================================================================= AZStd::string ExtractUserMessage(const ScriptDataContext& dc) { - AZStd::string userMessage = "Condition failed"; const int argCount = dc.GetNumArguments(); if (argCount > 0 && dc.IsString(argCount - 1)) { @@ -33,12 +32,12 @@ namespace AZ { if (value) { - userMessage = value; + return value; } } } - return userMessage; + return "ExtractUserMessage from print/Debug.Log/Warn/Error/Assert failed. Consider wrapping your argument in tostring()."; } //========================================================================= diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp index a42f624f01..e971375e15 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.cpp @@ -2258,12 +2258,16 @@ namespace ScriptCanvas m_variableScopeMeaning = VariableScopeMeaning_LegacyFunctions::ValueInitialization; } #endif + // The Order Matters: begin + + // add all data to the ACM for easy look up in input/output processing for ACM nodes AddAllVariablesPreParse(); if (!IsErrorFree()) { return; } + // parse basic editor nodes as they may add implicit variables for (auto& nodeEntity : m_source.m_graphData->m_nodes) { if (nodeEntity) @@ -2296,18 +2300,27 @@ namespace ScriptCanvas return; } + // parse the implicit variables added by ebus handling syntax sugar ParseAutoConnectedEBusHandlerVariables(); + // all possible data is available, now parse execution, starting with "main", currently keyed to RuntimeComponent::Activate Parse(m_startNodes); - + // parse any function introduced by nodes other than On Graph Start/"main" for (auto node : m_possibleExecutionRoots) { ParseExecutionTreeRoots(*node); } - + // parse functions introduced by variable change events ParseVariableHandling(); + // parse all user function and function object signatures ParseUserFunctionTopology(); - ParseConstructionInputVariables(); + // culls unused variables, and determine whether the the graph defines an object or static functionality ParseExecutionCharacteristics(); + // now that variables have been culled, determined what data needs to be initialized by an external source + ParseConstructionInputVariables(); + // now that externally initialized data has been identified, associate local, static initializers with individual functions + ParseFunctionLocalStaticUseage(); + + // The Order Matters: end // from here on, nothing more needs to happen during simple parsing // for example, in the editor, to get validation on syntax based effects for the view @@ -2315,7 +2328,10 @@ namespace ScriptCanvas if (IsErrorFree()) { + // the graph could have used several user graphs which required construction, and maybe multiple instances of the same user asset + // this will create indices for those nodes to be able to pass in the proper entry in the construction argument tree at translation and runtime ParseDependenciesAssetIndicies(); + // protect all names against keyword collision and language naming violations ConvertNamesToIdentifiers(); if (m_source.m_addDebugInfo) @@ -4189,6 +4205,32 @@ namespace ScriptCanvas ParseExecutionLoop(execution); } + void AbstractCodeModel::ParseFunctionLocalStaticUseage() + { + for (auto execution : ModAllExecutionRoots()) + { + if (auto localVariables = GetLocalVariables(execution)) + { + for (auto variable : *localVariables) + { + if (const AZStd::pair* pair = FindStaticVariable(variable)) + { + auto& localStatics = ModStaticVariablesNames(execution); + auto iter = AZStd::find_if + ( localStatics.begin() + , localStatics.end() + , [&](const auto& candidate) { return candidate.first == variable; }); + + if (iter == localStatics.end()) + { + localStatics.push_back(*pair); + } + } + } + } + } + } + void AbstractCodeModel::ParseImplicitVariables(const Node& node) { if (IsCycle(node)) @@ -5232,27 +5274,8 @@ namespace ScriptCanvas TraverseTree(execution, listener); const auto& usage = listener.GetUsedVariables(); const bool usesOnlyLocalVariables = usage.memberVariables.empty() && usage.implicitMemberVariables.empty(); - m_variableUse.localVariables.insert(usage.localVariables.begin(), usage.localVariables.end()); m_variableUse.memberVariables.insert(usage.memberVariables.begin(), usage.memberVariables.end()); - - for (auto variable : m_variableUse.localVariables) - { - if (const AZStd::pair* pair = FindStaticVariable(variable)) - { - auto& localStatics = ModStaticVariablesNames(execution); - auto iter = AZStd::find_if - (localStatics.begin() - , localStatics.end() - , [&](const auto& candidate) { return candidate.first == variable; }); - - if (iter == localStatics.end()) - { - localStatics.push_back(*pair); - } - } - } - m_variableUseByExecution.emplace(execution, listener.MoveUsedVariables()); return (!usage.usesExternallyInitializedVariables) && usesOnlyLocalVariables && listener.IsPure(); } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.h index f87913d47a..a5c55a7c4f 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/AbstractCodeModel.h @@ -366,6 +366,8 @@ namespace ScriptCanvas void ParseExecutionWhileLoop(ExecutionTreePtr execution); + void ParseFunctionLocalStaticUseage(); + void ParseImplicitVariables(const Node& node); void ParseInputData(ExecutionTreePtr execution); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/ScriptUserDataSerializer.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/ScriptUserDataSerializer.cpp index 62e361da93..73e16024c9 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/ScriptUserDataSerializer.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/ScriptUserDataSerializer.cpp @@ -88,7 +88,7 @@ namespace AZ { rapidjson::Value typeValue; result.Combine(StoreTypeId(typeValue, inputAnyPtr->type(), context)); - outputValue.AddMember("$type", AZStd::move(typeValue), context.GetJsonAllocator()); + outputValue.AddMember(rapidjson::StringRef(JsonSerialization::TypeIdFieldIdentifier), AZStd::move(typeValue), context.GetJsonAllocator()); } result.Combine(ContinueStoringToJsonObjectField(outputValue, "value", AZStd::any_cast(inputAnyPtr), AZStd::any_cast(defaultAnyPtr), inputAnyPtr->type(), context)); diff --git a/Gems/ScriptCanvas/Code/scriptcanvasgem_common_files.cmake b/Gems/ScriptCanvas/Code/scriptcanvasgem_common_files.cmake index 8e7a8c0411..f980301485 100644 --- a/Gems/ScriptCanvas/Code/scriptcanvasgem_common_files.cmake +++ b/Gems/ScriptCanvas/Code/scriptcanvasgem_common_files.cmake @@ -622,5 +622,4 @@ set(SKIP_UNITY_BUILD_INCLUSION_FILES Include/ScriptCanvas/Libraries/Core/FunctionCallNode.h Include/ScriptCanvas/Libraries/Core/FunctionCallNodeIsOutOfDate.h Include/ScriptCanvas/Libraries/Core/FunctionCallNodeIsOutOfDate.cpp - -) \ No newline at end of file +) diff --git a/Gems/ScriptCanvas/Code/scriptcanvasgem_editor_builder_files.cmake b/Gems/ScriptCanvas/Code/scriptcanvasgem_editor_builder_files.cmake index 0841137435..12cb1a242e 100644 --- a/Gems/ScriptCanvas/Code/scriptcanvasgem_editor_builder_files.cmake +++ b/Gems/ScriptCanvas/Code/scriptcanvasgem_editor_builder_files.cmake @@ -15,4 +15,4 @@ set(FILES Builder/ScriptCanvasBuilderWorker.h Builder/ScriptCanvasBuilderWorkerUtility.cpp Builder/ScriptCanvasFunctionBuilderWorker.cpp -) \ No newline at end of file +)