From 205c09e2000de5ff47c57621f59f0f8e7c66a7ac Mon Sep 17 00:00:00 2001 From: chcurran <82187351+carlitosan@users.noreply.github.com> Date: Tue, 31 Aug 2021 17:24:02 -0700 Subject: [PATCH] Better upgrade tool messaging and continuity on failure; fix for serialization Signed-off-by: chcurran <82187351+carlitosan@users.noreply.github.com> --- .../Tools/UpgradeTool/VersionExplorer.cpp | 183 ++++++++---------- .../Tools/UpgradeTool/VersionExplorer.h | 13 +- .../Serialization/DatumSerializer.cpp | 29 ++- 3 files changed, 113 insertions(+), 112 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.cpp b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.cpp index 6b396d88ba..0d7efa5e41 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.cpp @@ -188,9 +188,9 @@ namespace ScriptCanvasEditor if (!IsUpgrading()) { - OperationResult result = BackupGraph(*m_inProgressAsset); + AZStd::string errorMessage = BackupGraph(*m_inProgressAsset); // Make the backup - if (result == OperationResult::BackupSuccess || result == OperationResult::SkipBackup) + if (errorMessage.empty()) { Log("SystemTick::ProcessState::Upgrade: Backup Success %s ", m_inProgressAsset->GetHint().c_str()); QList items = m_ui->tableWidget->findItems(m_inProgressAsset->GetHint().c_str(), Qt::MatchFlag::MatchExactly); @@ -210,7 +210,7 @@ namespace ScriptCanvasEditor else { Log("SystemTick::ProcessState::Upgrade: Backup Failed %s ", m_inProgressAsset->GetHint().c_str()); - GraphUpgradeComplete(*m_inProgressAsset, result); + GraphUpgradeComplete(*m_inProgressAsset, OperationResult::Failure, errorMessage); } } @@ -241,12 +241,12 @@ namespace ScriptCanvasEditor AZ::SystemTickBus::Handler::BusConnect(); } - VersionExplorer::OperationResult VersionExplorer::BackupGraph(const AZ::Data::Asset& asset) + AZStd::string VersionExplorer::BackupGraph(const AZ::Data::Asset& asset) { bool makeBackup = m_ui->makeBackupCheckbox->isChecked(); if (!makeBackup) { - return OperationResult::SkipBackup; + return ""; } QDateTime theTime = QDateTime::currentDateTime(); @@ -262,7 +262,7 @@ namespace ScriptCanvasEditor if (AZ::IO::FileIOBase::GetInstance()->CreatePath(backupPath.c_str()) != AZ::IO::ResultCode::Success) { AZ_Error(ScriptCanvas::k_VersionExplorerWindow.data(), false, "Failed to create backup folder %s", backupPath.c_str()); - return OperationResult::BackupFail_CreateFolder; + return "Failed to create backup folder"; } } @@ -298,7 +298,7 @@ namespace ScriptCanvasEditor else { AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "VersionExplorer::BackupGraph: Failed to find file: %s", asset.GetHint().c_str()); - return OperationResult::BackupFail_FileNotFound; + return "Failed to find source file"; } devRoot = devRootCStr; @@ -317,12 +317,12 @@ namespace ScriptCanvasEditor if (AZ::IO::FileIOBase::GetInstance()->Copy(sourceFilePath.c_str(), targetFilePath.c_str()) != AZ::IO::ResultCode::Error) { Log("VersionExplorer::BackupGraph: Backed up: %s ---> %s\n", sourceFilePath.c_str(), targetFilePath.c_str()); - return OperationResult::BackupSuccess; + return ""; } else { AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "VersionExplorer::BackupGraph: Error creating backup: %s ---> %s\n", sourceFilePath.c_str(), targetFilePath.c_str()); - return OperationResult::BackupFail; + return "Failed to copy source file to backup location"; } } @@ -376,17 +376,18 @@ namespace ScriptCanvasEditor scriptCanvasEntity->Activate(); } + AZ_Assert(scriptCanvasEntity->GetState() == AZ::Entity::State::Active, "Graph entity is not active"); auto graphComponent = scriptCanvasEntity->FindComponent(); AZ_Assert(graphComponent, "The Script Canvas entity must have a Graph component"); if (graphComponent) { + m_scriptCanvasEntity = scriptCanvasEntity; + graphComponent->UpgradeGraph ( asset , m_ui->forceUpgrade->isChecked() ? Graph::UpgradeRequest::Forced : Graph::UpgradeRequest::IfOutOfDate , m_ui->verbose->isChecked()); - - m_scriptCanvasEntity = scriptCanvasEntity; } } @@ -417,85 +418,78 @@ namespace ScriptCanvasEditor { AZStd::string relativePath, fullPath; AZ::Data::AssetCatalogRequestBus::BroadcastResult(relativePath, &AZ::Data::AssetCatalogRequests::GetAssetPathById, asset.GetId()); - bool fullPathFound = false; AzToolsFramework::AssetSystemRequestBus::BroadcastResult(fullPathFound, &AzToolsFramework::AssetSystemRequestBus::Events::GetFullSourcePathFromRelativeProductPath, relativePath, fullPath); - AZStd::string tmpFileName; - bool tmpFilesaved = false; - - constexpr const size_t k_maxAttemps = 10; - // here we are saving the graph to a temp file instead of the original file and then copying the temp file to the original file. // This ensures that AP will not a get a file change notification on an incomplete graph file causing it to fail processing. Temp files are ignored by AP. - if (AZ::IO::CreateTempFileName(fullPath.c_str(), tmpFileName)) + if (!AZ::IO::CreateTempFileName(fullPath.c_str(), tmpFileName)) { - AZ::IO::FileIOStream fileStream(tmpFileName.c_str(), AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeText); + GraphUpgradeComplete(asset, OperationResult::Failure, "Failure to create temporary file name"); + return; + } - if (fileStream.IsOpen()) + bool tempSavedSucceeded = false; + AZ::IO::FileIOStream fileStream(tmpFileName.c_str(), AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeText); + if (fileStream.IsOpen()) + { + if (asset.GetType() == azrtti_typeid()) { - if (asset.GetType() == azrtti_typeid()) - { - ScriptCanvasEditor::ScriptCanvasAssetHandler handler; - tmpFilesaved = handler.SaveAssetData(asset, &fileStream); - } - - fileStream.Close(); + ScriptCanvasEditor::ScriptCanvasAssetHandler handler; + tempSavedSucceeded = handler.SaveAssetData(asset, &fileStream); } - using SCCommandBus = AzToolsFramework::SourceControlCommandBus; - SCCommandBus::Broadcast(&SCCommandBus::Events::RequestEdit, fullPath.c_str(), true, - [this, asset, fullPath, tmpFileName, tmpFilesaved](bool /*success*/, const AzToolsFramework::SourceControlFileInfo& info) - { - if (!info.IsReadOnly()) - { - if (tmpFilesaved) - { - PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); - } - } - else - { - if (m_overwriteAll) - { - AZ::IO::SystemFile::SetWritable(info.m_filePath.c_str(), true); - - if (tmpFilesaved) - { - PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); - } - } - else - { - int result = QMessageBox::No; - if (!m_overwriteAll) - { - QMessageBox mb(QMessageBox::Warning, - QObject::tr("Failed to Save Upgraded File"), - QObject::tr("The upgraded file could not be saved because the file is read only.\nDo you want to make it writeable and overwrite it?"), - QMessageBox::YesToAll | QMessageBox::Yes | QMessageBox::No, this); - - result = mb.exec(); - if (result == QMessageBox::YesToAll) - { - m_overwriteAll = true; - } - } - - if (result == QMessageBox::Yes || m_overwriteAll) - { - AZ::IO::SystemFile::SetWritable(info.m_filePath.c_str(), true); - - if (tmpFilesaved) - { - PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); - } - } - - } - } - }); + fileStream.Close(); } + + if (!tempSavedSucceeded) + { + GraphUpgradeComplete(asset, OperationResult::Failure, "Save asset data to temporary file failed"); + return; + } + + using SCCommandBus = AzToolsFramework::SourceControlCommandBus; + SCCommandBus::Broadcast(&SCCommandBus::Events::RequestEdit, fullPath.c_str(), true, + [this, asset, fullPath, tmpFileName]([[maybe_unused]] bool success, const AzToolsFramework::SourceControlFileInfo& info) + { + constexpr const size_t k_maxAttemps = 10; + + if (!info.IsReadOnly()) + { + PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); + } + else + { + if (m_overwriteAll) + { + AZ::IO::SystemFile::SetWritable(info.m_filePath.c_str(), true); + PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); + } + else + { + int result = QMessageBox::No; + if (!m_overwriteAll) + { + QMessageBox mb(QMessageBox::Warning, + QObject::tr("Failed to Save Upgraded File"), + QObject::tr("The upgraded file could not be saved because the file is read only.\nDo you want to make it writeable and overwrite it?"), + QMessageBox::YesToAll | QMessageBox::Yes | QMessageBox::No, this); + + result = mb.exec(); + if (result == QMessageBox::YesToAll) + { + m_overwriteAll = true; + } + } + + if (result == QMessageBox::Yes || m_overwriteAll) + { + AZ::IO::SystemFile::SetWritable(info.m_filePath.c_str(), true); + PerformMove(asset, tmpFileName, fullPath, k_maxAttemps); + } + } + } + }); } void VersionExplorer::PerformMove(AZ::Data::Asset asset, const AZStd::string& source, const AZStd::string& target @@ -506,10 +500,11 @@ namespace ScriptCanvasEditor if (remainingAttempts == 0) { AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "moving converted file to source destination failed: %s. giving up", target.c_str()); - GraphUpgradeComplete(asset, OperationResult::CopyFinalFailed); + GraphUpgradeComplete(asset, OperationResult::Failure, "Failed to move updated file from backup to source destination"); } else if (remainingAttempts == 2) { + AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "moving converted file to source destination failed: %s, trying again", target.c_str()); auto streamer = AZ::Interface::Get(); AZ::IO::FileRequestPtr flushRequest = streamer->FlushCaches(); streamer->SetRequestCompleteCallback(flushRequest @@ -530,11 +525,14 @@ namespace ScriptCanvasEditor AZ::IO::FileRequestPtr flushRequest = streamer->FlushCache(target.c_str()); // Bump the slice asset up in the asset processor's queue. AzFramework::AssetSystemRequestBus::Broadcast(&AzFramework::AssetSystem::AssetSystemRequests::EscalateAssetBySearchTerm, target.c_str()); - AZ::SystemTickBus::QueueFunction([this, asset]() { GraphUpgradeComplete(asset); }); + AZ::SystemTickBus::QueueFunction([this, asset]() + { + GraphUpgradeComplete(asset, OperationResult::Success, ""); + }); } else { - AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "moving converted file to source destination failed: %s. trying again", target.c_str()); + AZ_Warning(ScriptCanvas::k_VersionExplorerWindow.data(), false, "moving converted file to source destination failed: %s, trying again", target.c_str()); auto streamer = AZ::Interface::Get(); AZ::IO::FileRequestPtr flushRequest = streamer->FlushCache(target.c_str()); streamer->SetRequestCompleteCallback(flushRequest, [this, asset, &source, &target, remainingAttempts]([[maybe_unused]] AZ::IO::FileRequestHandle request) @@ -547,7 +545,8 @@ namespace ScriptCanvasEditor } } - void VersionExplorer::GraphUpgradeComplete(const AZ::Data::Asset asset, OperationResult result ) + void VersionExplorer::GraphUpgradeComplete + (const AZ::Data::Asset asset, OperationResult result, AZStd::string_view message) { m_inProgress = false; @@ -557,7 +556,7 @@ namespace ScriptCanvasEditor m_scriptCanvasEntity = nullptr; } - GraphUpgradeCompleteUIUpdate(asset, result); + GraphUpgradeCompleteUIUpdate(asset, result, message); if (!m_isUpgradingSingleGraph) { @@ -588,7 +587,8 @@ namespace ScriptCanvasEditor } } - void VersionExplorer::GraphUpgradeCompleteUIUpdate(const AZ::Data::Asset asset, OperationResult result /*= OperationResult::Success*/) + void VersionExplorer::GraphUpgradeCompleteUIUpdate + ( const AZ::Data::Asset asset, OperationResult result, AZStd::string_view message) { QString text = asset.GetHint().c_str(); QList items = m_ui->tableWidget->findItems(text, Qt::MatchFlag::MatchExactly); @@ -615,18 +615,7 @@ namespace ScriptCanvasEditor else { doneButton->setIcon(QIcon(":/stylesheet/img/UI20/titlebar-close.svg")); - if (result == OperationResult::BackupFail_FileNotFound) - { - doneButton->setToolTip("The file no longer exists"); - } - else if (result == OperationResult::BackupFail_CreateFolder) - { - doneButton->setToolTip("Failed to create the backup folder"); - } - else if (result == OperationResult::CopyFinalFailed) - { - doneButton->setToolTip("Failed to copy final file to the source destination"); - } + doneButton->setToolTip(message.data()); } m_ui->tableWidget->setCellWidget(row, ColumnStatus, doneButton); diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.h b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.h index 4dd67fef58..78f4f26e95 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.h +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/VersionExplorer.h @@ -111,15 +111,10 @@ namespace ScriptCanvasEditor enum class OperationResult { Success, - SkipBackup, - BackupSuccess, - BackupFail, - BackupFail_CreateFolder, - BackupFail_FileNotFound, - CopyFinalFailed, + Failure, }; - void GraphUpgradeComplete(const AZ::Data::Asset, OperationResult result = OperationResult::Success); + void GraphUpgradeComplete(const AZ::Data::Asset, OperationResult result, AZStd::string_view message); bool IsUpgrading() const; @@ -157,10 +152,10 @@ namespace ScriptCanvasEditor void FinalizeScan(); void BackupComplete(); - OperationResult BackupGraph(const AZ::Data::Asset&); + AZStd::string BackupGraph(const AZ::Data::Asset&); void UpgradeGraph(const AZ::Data::Asset&); - void GraphUpgradeCompleteUIUpdate(const AZ::Data::Asset asset, OperationResult result = OperationResult::Success); + void GraphUpgradeCompleteUIUpdate(const AZ::Data::Asset asset, OperationResult result, AZStd::string_view message); void OnGraphUpgradeComplete(AZ::Data::Asset&, bool skipped = false) override; void OnSourceFileReleased(AZ::Data::Asset asset); diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/DatumSerializer.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/DatumSerializer.cpp index 5bfb69b11e..37fd3b14d6 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/DatumSerializer.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Serialization/DatumSerializer.cpp @@ -6,12 +6,27 @@ * */ +#include #include #include #include using namespace ScriptCanvas; +namespace DatumSerializerCpp +{ + bool IsEventInput(const AZ::Uuid& inputType) + { + AZ::BehaviorContext* behaviorContext = nullptr; + AZ::ComponentApplicationBus::BroadcastResult(behaviorContext, &AZ::ComponentApplicationRequests::GetBehaviorContext); + AZ_Assert(behaviorContext, "Can't serialize data properly without checking the type, for which we need behavior context!"); + auto bcClassIter = behaviorContext->m_typeToClassMap.find(inputType); + return bcClassIter != behaviorContext->m_typeToClassMap.end() + && bcClassIter->second->m_azRtti + && bcClassIter->second->m_azRtti->GetGenericTypeId() == azrtti_typeid(); + } +} + namespace AZ { AZ_CLASS_ALLOCATOR_IMPL(DatumSerializer, SystemAllocator, 0); @@ -57,7 +72,7 @@ namespace AZ return context.Report ( JSR::Tasks::ReadField , JSR::Outcomes::Missing - , "DatumSerializer::Load failed to load the 'isNullPointer'' member"); + , "DatumSerializer::Load failed to load the 'isNullPointer' member"); } if (isNullPointerMember->value.GetBool()) @@ -110,7 +125,7 @@ namespace AZ { listeners->push_back(outputDatum); } - + return context.Report(result, result.GetProcessing() != JSR::Processing::Halted ? "DatumSerializer Load finished loading Datum" : "DatumSerializer Load failed to load Datum"); @@ -130,13 +145,13 @@ namespace AZ auto inputScriptDataPtr = reinterpret_cast(inputValue); auto defaultScriptDataPtr = reinterpret_cast(defaultValue); - + if (defaultScriptDataPtr) { if (*inputScriptDataPtr == *defaultScriptDataPtr) { return context.Report - ( JSR::Tasks::WriteValue, JSR::Outcomes::DefaultsUsed, "DatumSerializer Store used defaults for Datum"); + (JSR::Tasks::WriteValue, JSR::Outcomes::DefaultsUsed, "DatumSerializer Store used defaults for Datum"); } } @@ -159,11 +174,13 @@ namespace AZ , azrtti_typeidGetType())>() , context)); + // datum storage begin auto inputObjectSource = inputScriptDataPtr->GetAsDanger(); - outputValue.AddMember("isNullPointer", rapidjson::Value(inputObjectSource == nullptr), context.GetJsonAllocator()); + const bool isNullPointer = inputObjectSource == nullptr || DatumSerializerCpp::IsEventInput(inputScriptDataPtr->GetType().GetAZType()); + outputValue.AddMember("isNullPointer", rapidjson::Value(isNullPointer), context.GetJsonAllocator()); - if (inputObjectSource) + if (!isNullPointer) { rapidjson::Value typeValue; result.Combine(StoreTypeId(typeValue, inputScriptDataPtr->GetType().GetAZType(), context));