From a870c6ad80a77a03a4869f79595c448f99b9091b Mon Sep 17 00:00:00 2001 From: chcurran <82187351+carlitosan@users.noreply.github.com> Date: Tue, 5 Oct 2021 09:52:52 -0700 Subject: [PATCH] Fix explorer bugs and restore upgrade single option Signed-off-by: chcurran <82187351+carlitosan@users.noreply.github.com> --- .../ScriptCanvasBuilderWorkerUtility.cpp | 2 + .../Windows/Tools/UpgradeTool/Controller.cpp | 36 ++++++++------- .../Windows/Tools/UpgradeTool/Controller.h | 2 + .../View/Windows/Tools/UpgradeTool/Model.cpp | 24 +++++++++- .../Windows/Tools/UpgradeTool/ModelTraits.h | 2 +- .../Libraries/Core/ReceiveScriptEvent.cpp | 27 +++++------ .../Libraries/Core/ReceiveScriptEvent.h | 3 +- .../Libraries/Core/ScriptEventBase.cpp | 7 +-- .../ScriptCanvas/Translation/GraphToLua.cpp | 11 +++-- .../ScriptCanvas/Translation/GraphToX.cpp | 45 ++++++++++--------- .../ScriptCanvas/Translation/GraphToX.h | 5 ++- .../Translation/TranslationResult.cpp | 9 ++++ .../Translation/TranslationResult.h | 8 ++-- 13 files changed, 113 insertions(+), 68 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Builder/ScriptCanvasBuilderWorkerUtility.cpp b/Gems/ScriptCanvas/Code/Builder/ScriptCanvasBuilderWorkerUtility.cpp index 0853789b19..665a481a17 100644 --- a/Gems/ScriptCanvas/Code/Builder/ScriptCanvasBuilderWorkerUtility.cpp +++ b/Gems/ScriptCanvas/Code/Builder/ScriptCanvasBuilderWorkerUtility.cpp @@ -113,6 +113,7 @@ namespace ScriptCanvasBuilder if (!isSuccessOutcome.IsSuccess()) { + // sanity check return AZ::Failure(isSuccessOutcome.TakeError()); } auto& translation = translationResult.m_translations.find(ScriptCanvas::Translation::TargetFlags::Lua)->second; @@ -481,6 +482,7 @@ namespace ScriptCanvasBuilder buildEntity->Activate(); } + AZ_Assert(buildEntity->GetState() == AZ::Entity::State::Active, "build entity not active"); return sourceGraph; } diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.cpp b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.cpp index 4e4e842cb8..207f70aec1 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.cpp @@ -131,6 +131,11 @@ namespace ScriptCanvasEditor } void Controller::OnButtonPressUpgrade() + { + OnButtonPressUpgradeImplementation({}); + } + + void Controller::OnButtonPressUpgradeImplementation(const AZ::Data::AssetInfo& assetInfo) { auto simpleUpdate = [this](AZ::Data::Asset asset) { @@ -202,12 +207,18 @@ namespace ScriptCanvasEditor SetLoggingPreferences(); ModifyConfiguration config; + config.modifySingleAsset = assetInfo; config.modification = simpleUpdate; config.onReadOnlyFile = onReadyOnlyFile; config.backupGraphBeforeModification = m_view->makeBackupCheckbox->isChecked(); ModelRequestsBus::Broadcast(&ModelRequestsTraits::Modify, config); } + void Controller::OnButtonPressUpgradeSingle(const AZ::Data::AssetInfo& assetInfo) + { + OnButtonPressUpgradeImplementation(assetInfo); + } + void Controller::OnUpgradeModificationBegin([[maybe_unused]] const ModifyConfiguration& config, const AZ::Data::AssetInfo& info) { QList items = m_view->tableWidget->findItems(info.m_relativePath.c_str(), Qt::MatchFlag::MatchExactly); @@ -325,7 +336,7 @@ namespace ScriptCanvasEditor OnScannedGraph(info, Filtered::Yes); } - void Controller::OnScannedGraph(const AZ::Data::AssetInfo& assetInfo, Filtered filtered) + void Controller::OnScannedGraph(const AZ::Data::AssetInfo& assetInfo, [[maybe_unused]] Filtered filtered) { m_view->tableWidget->insertRow(m_handledAssetCount); QTableWidgetItem* rowName = new QTableWidgetItem(tr(assetInfo.m_relativePath.c_str())); @@ -338,19 +349,14 @@ namespace ScriptCanvasEditor rowGoToButton->setText("Upgrade"); rowGoToButton->setEnabled(false); SetRowBusy(m_handledAssetCount); -// \\ todo restore this -// connect(rowGoToButton, &QPushButton::clicked, [this, rowGoToButton, assetInfo] { -// -// // request upgrade of a single graph -// // AZ::SystemTickBus::QueueFunction([this, rowGoToButton, assetInfo]() { -// // // Queue the process state change because we can't connect to the SystemTick bus in a Qt lambda -// // UpgradeSingle(rowGoToButton, spinner, assetInfo); -// // }); -// // -// // AZ::SystemTickBus::ExecuteQueuedEvents(); -// -// }); - + connect + ( rowGoToButton + , &QPushButton::pressed + , this + , [this, assetInfo]() + { + this->OnButtonPressUpgradeSingle(assetInfo); + }); m_view->tableWidget->setCellWidget(m_handledAssetCount, static_cast(ColumnAction), rowGoToButton); } @@ -424,7 +430,7 @@ namespace ScriptCanvasEditor } QString spinnerText = QStringLiteral("Upgrade in progress - "); - if (config.modifySingleAsset) + if (config.modifySingleAsset.m_assetId.IsValid()) { spinnerText.append(" single graph"); } diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.h b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.h index fd297832bb..23f35840be 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.h +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Controller.h @@ -64,6 +64,8 @@ namespace ScriptCanvasEditor void OnButtonPressClose(); void OnButtonPressScan(); void OnButtonPressUpgrade(); + void OnButtonPressUpgradeImplementation(const AZ::Data::AssetInfo& assetInfo); + void OnButtonPressUpgradeSingle(const AZ::Data::AssetInfo& assetInfo); void OnGraphUpgradeComplete(AZ::Data::Asset&, bool skipped) override; diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Model.cpp b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Model.cpp index ed17293ab7..98edef54bf 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Model.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Model.cpp @@ -101,11 +101,33 @@ namespace ScriptCanvasEditor return; } + auto results = m_scanner->TakeResult(); + if (modification.modifySingleAsset.m_assetId.IsValid()) + { + auto iter = AZStd::find_if + ( results.m_unfiltered.begin() + , results.m_unfiltered.end() + , [&modification](const auto& candidate) + { + return candidate.info.m_assetId == modification.modifySingleAsset.m_assetId; + }); + + if (iter == results.m_unfiltered.end()) + { + AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "Requested upgrade graph not found in scanned list."); + return; + } + + WorkingAsset singleEntry = *iter; + results.m_unfiltered.clear(); + results.m_unfiltered.push_back(singleEntry); + } + m_modResults = {}; m_state = State::Modifying; m_log.Activate(); m_keepEditorAlive = AZStd::make_unique(); - auto results = m_scanner->TakeResult(); + m_modifier = AZStd::make_unique(modification, AZStd::move(results.m_unfiltered), [this](){ OnModificationComplete(); }); } diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/ModelTraits.h b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/ModelTraits.h index 026748c344..01eb200542 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/ModelTraits.h +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/ModelTraits.h @@ -26,7 +26,7 @@ namespace ScriptCanvasEditor { AZStd::function)> modification; AZStd::function onReadOnlyFile; - bool modifySingleAsset = false; + AZ::Data::AssetInfo modifySingleAsset; bool backupGraphBeforeModification = false; bool successfulDependencyUpgradeRequired = true; }; diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.cpp index 5a2eb80187..c548176af5 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.cpp @@ -67,7 +67,7 @@ namespace ScriptCanvas void ReceiveScriptEvent::PopulateAsset(AZ::Data::Asset asset, SlotIdMapping& populationMapping) { - if (CreateHandler(asset)) + if (InitializeDefinition(asset)) { if (!CreateEbus()) { @@ -140,7 +140,6 @@ namespace ScriptCanvas } } - void ReceiveScriptEvent::InitializeEvent(AZ::Data::Asset asset, int eventIndex, SlotIdMapping& populationMapping) { if (!m_handler) @@ -353,6 +352,13 @@ namespace ScriptCanvas AZStd::optional ReceiveScriptEvent::GetEventIndex(AZStd::string eventName) const { + if (!m_handler) + { + const_cast(this)->InitializeDefinition(m_asset); + const_cast(this)->CreateEbus(); + } + + AZ_Error("ScriptCanvas", m_handler != nullptr, "GetEventIndex called and handler was not created"); return m_handler ? AZStd::optional(m_handler->GetFunctionIndex(eventName.c_str())) : AZStd::nullopt; } @@ -490,15 +496,10 @@ namespace ScriptCanvas return m_definition.IsAddressRequired() || (slot && slot->GetDataType().IsValid()); } - bool ReceiveScriptEvent::CreateHandler(AZ::Data::Asset asset) + bool ReceiveScriptEvent::InitializeDefinition(AZ::Data::Asset asset) { AZStd::lock_guard lock(m_mutex); - if (m_handler) - { - return true; - } - if (!asset) { return false; @@ -518,12 +519,11 @@ namespace ScriptCanvas } return true; - } void ReceiveScriptEvent::OnScriptEventReady(const AZ::Data::Asset& asset) { - if (CreateHandler(asset)) + if (InitializeDefinition(asset)) { CompleteInitialize(asset); } @@ -531,7 +531,7 @@ namespace ScriptCanvas bool ReceiveScriptEvent::CreateEbus() { - if (!m_ebus) + if (!m_ebus || !m_handler) { AZ::BehaviorContext* behaviorContext = nullptr; AZ::ComponentApplicationBus::BroadcastResult(behaviorContext, &AZ::ComponentApplicationBus::Events::GetBehaviorContext); @@ -547,13 +547,11 @@ namespace ScriptCanvas AZ_Assert(m_ebus, "Behavior Context EBus does not exist: %s", m_definition.GetName().c_str()); AZ_Assert(m_ebus->m_createHandler, "The ebus %s has no create handler!", m_definition.GetName().c_str()); AZ_Assert(m_ebus->m_destroyHandler, "The ebus %s has no destroy handler!", m_definition.GetName().c_str()); - AZ_Verify(m_ebus->m_createHandler->InvokeResult(m_handler, &m_definition), "Behavior Context EBus handler creation failed %s", m_definition.GetName().c_str()); - AZ_Assert(m_handler, "Ebus create handler failed %s", m_definition.GetName().c_str()); } - return true; + return m_ebus != nullptr && m_handler != nullptr; } bool ReceiveScriptEvent::IsOutOfDate(const VersionData& graphVersion) const @@ -597,7 +595,6 @@ namespace ScriptCanvas } } - AZStd::string ReceiveScriptEvent::GetUpdateString() const { if (m_ebus) diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.h index 0f8d673629..a41d0ba3f9 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ReceiveScriptEvent.h @@ -101,7 +101,7 @@ namespace ScriptCanvas Internal::ScriptEventEntry ConfigureEbusEntry(const ScriptEvents::Method& methodDefinition, const AZ::BehaviorEBusHandler::BusForwarderEvent& event, SlotIdMapping& populationMapping); - bool CreateHandler(AZ::Data::Asset asset); + bool InitializeDefinition(AZ::Data::Asset asset); void CompleteInitialize(AZ::Data::Asset asset); void PopulateAsset(AZ::Data::Asset asset, SlotIdMapping& populationMapping); bool m_eventInitComplete = false; @@ -120,7 +120,6 @@ namespace ScriptCanvas bool m_autoConnectToGraphOwner = true; bool m_connected; - }; } } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ScriptEventBase.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ScriptEventBase.cpp index 3b01a8fa7c..e4cbfc94f7 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ScriptEventBase.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Libraries/Core/ScriptEventBase.cpp @@ -122,11 +122,8 @@ namespace ScriptCanvas void ScriptEventBase::OnActivate() { - if (!m_asset || m_asset.GetStatus() == AZ::Data::AssetData::AssetStatus::NotLoaded) - { - m_asset = AZ::Data::AssetManager::Instance().GetAsset(m_scriptEventAssetId, AZ::Data::AssetLoadBehavior::PreLoad); - m_asset.BlockUntilLoadComplete(); - } + m_asset = AZ::Data::AssetManager::Instance().GetAsset(m_scriptEventAssetId, AZ::Data::AssetLoadBehavior::PreLoad); + m_asset.BlockUntilLoadComplete(); } void ScriptEventBase::OnAssetReady(AZ::Data::Asset asset) diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToLua.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToLua.cpp index 9a1ef73126..ee7eb704bb 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToLua.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToLua.cpp @@ -246,8 +246,7 @@ namespace ScriptCanvas } else { - ErrorList errors = { AZStd::string("provide translation failure details") }; - return AZ::Failure(errors); + return AZ::Failure(translation.MoveErrors()); } } @@ -874,7 +873,13 @@ namespace ScriptCanvas AZStd::optional eventIndex = ebusHandling->m_node->GetEventIndex(nameAndEventThread.first); if (!eventIndex) { - AddError(nullptr, aznew Internal::ParseError(ebusHandling->m_node->GetEntityId(), AZStd::string::format("EBus handler did not return a valid index for event %s", nameAndEventThread.first.c_str()))); + AddError(nullptr, + aznew Internal::ParseError( + ebusHandling->m_node->GetEntityId() + , AZStd::string::format + ( "EBus Handler %s did not return a valid index for event %s" + , ebusHandling->m_ebusName.c_str() + , nameAndEventThread.first.c_str()))); return; } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.cpp index 09315ec4b0..d21fb354b1 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.cpp @@ -107,27 +107,9 @@ namespace ScriptCanvas m_translationDuration = AZStd::chrono::microseconds(AZStd::chrono::system_clock::now() - m_translationStartTime).count(); } - AZStd::string GraphToX::ResolveScope(const AZStd::vector& namespaces) + AZStd::vector&& GraphToX::MoveErrors() { - AZStd::string resolution; - - if (!namespaces.empty()) - { - resolution = Grammar::ToIdentifier(namespaces[0]); - - for (size_t index = 1; index < namespaces.size(); ++index) - { - resolution += m_configuration.m_lexicalScopeDelimiter; - resolution += Grammar::ToIdentifier(namespaces[index]); - } - } - - return resolution; - } - - void GraphToX::SingleLineComment(Writer& writer) - { - writer.Write(m_configuration.m_singleLineComment); + return AZStd::move(m_errors); } void GraphToX::OpenBlockComment(Writer& writer) @@ -160,6 +142,29 @@ namespace ScriptCanvas writer.Indent(); } + AZStd::string GraphToX::ResolveScope(const AZStd::vector& namespaces) + { + AZStd::string resolution; + + if (!namespaces.empty()) + { + resolution = Grammar::ToIdentifier(namespaces[0]); + + for (size_t index = 1; index < namespaces.size(); ++index) + { + resolution += m_configuration.m_lexicalScopeDelimiter; + resolution += Grammar::ToIdentifier(namespaces[index]); + } + } + + return resolution; + } + + void GraphToX::SingleLineComment(Writer& writer) + { + writer.Write(m_configuration.m_singleLineComment); + } + void GraphToX::WriteCopyright(Writer& writer) { OpenBlockComment(writer); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.h index 062a58efe1..c4e189679a 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/GraphToX.h @@ -51,12 +51,13 @@ namespace ScriptCanvas AZStd::string_view GetGraphName() const; AZStd::string_view GetFullPath() const; AZStd::sys_time_t GetTranslationDuration() const; - AZStd::string ResolveScope(const AZStd::vector& namespaces); - void SingleLineComment(Writer& writer); + AZStd::vector&& MoveErrors(); void OpenBlockComment(Writer& writer); void OpenFunctionBlock(Writer& writer); void OpenNamespace(Writer& writer, AZStd::string_view ns); void OpenScope(Writer& writer); + AZStd::string ResolveScope(const AZStd::vector& namespaces); + void SingleLineComment(Writer& writer); void WriteCopyright(Writer& writer); void WriteDoNotModify(Writer& writer); void WriteLastWritten(Writer& writer); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.cpp index dd8b42730d..c39cac13a3 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.cpp @@ -72,6 +72,15 @@ namespace ScriptCanvas resultString += entry->GetDescription(); } + for (const auto& errors : m_errors) + { + for (auto& entry : errors.second) + { + resultString += "* "; + resultString += entry->GetDescription(); + } + } + return resultString; } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.h index 0cde56d5dd..bd616fbfbb 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Translation/TranslationResult.h @@ -12,10 +12,11 @@ #include #include #include -#include #include -#include +#include +#include #include +#include namespace AZ { @@ -85,7 +86,7 @@ namespace ScriptCanvas AZStd::sys_time_t m_duration; }; - using ErrorList = AZStd::vector; + using ErrorList = AZStd::vector; using Errors = AZStd::unordered_map; using Translations = AZStd::unordered_map; @@ -102,7 +103,6 @@ namespace ScriptCanvas const AZStd::sys_time_t m_translationDuration; Result(AZStd::string invalidSourceInfo); - Result(Result&& source); Result(Grammar::AbstractCodeModelConstPtr model); Result(Grammar::AbstractCodeModelConstPtr model, Translations&& translations, Errors&& errors);