From ca771f3adc3ef4b71cf76c84c66bc93173cfbf89 Mon Sep 17 00:00:00 2001 From: chcurran <82187351+carlitosan@users.noreply.github.com> Date: Wed, 1 Dec 2021 22:18:32 -0800 Subject: [PATCH] version upgrader in a presumed working story Signed-off-by: chcurran <82187351+carlitosan@users.noreply.github.com> --- .../Editor/Assets/ScriptCanvasFileHandling.cpp | 5 ++--- .../Code/Editor/Components/EditorGraph.cpp | 4 ++-- .../Code/Editor/Components/GraphUpgrade.cpp | 6 ++++-- .../ScriptCanvas/Components/EditorGraph.h | 2 +- .../Windows/Tools/UpgradeTool/FileSaver.cpp | 13 ++++++++++++- .../Windows/Tools/UpgradeTool/Modifier.cpp | 18 ++++++++++-------- .../Code/Include/ScriptCanvas/Core/Core.h | 6 +++--- .../ScriptCanvas/Grammar/ParsingMetaData.cpp | 8 ++++---- 8 files changed, 38 insertions(+), 24 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Editor/Assets/ScriptCanvasFileHandling.cpp b/Gems/ScriptCanvas/Code/Editor/Assets/ScriptCanvasFileHandling.cpp index 706c46fa75..be909e5043 100644 --- a/Gems/ScriptCanvas/Code/Editor/Assets/ScriptCanvasFileHandling.cpp +++ b/Gems/ScriptCanvas/Code/Editor/Assets/ScriptCanvasFileHandling.cpp @@ -209,7 +209,6 @@ namespace ScriptCanvasEditor AZ::Outcome LoadFromFile(AZStd::string_view path) { namespace JSRU = AZ::JsonSerializationUtils; - using namespace ScriptCanvas; auto fileStringOutcome = AZ::Utils::ReadFile(path); if (!fileStringOutcome) @@ -218,7 +217,7 @@ namespace ScriptCanvasEditor } const auto& asString = fileStringOutcome.GetValue(); - DataPtr scriptCanvasData = AZStd::make_shared(); + ScriptCanvas::DataPtr scriptCanvasData = aznew ScriptCanvas::ScriptCanvasData(); if (!scriptCanvasData) { return AZ::Failure(AZStd::string("failed to allocate ScriptCanvas::ScriptCanvasData after loading source file")); @@ -314,7 +313,7 @@ namespace ScriptCanvasEditor listener->OnSerialize(); } - auto saveOutcome = JSRU::SaveObjectToStream(graphData, stream, nullptr, &settings); + auto saveOutcome = JSRU::SaveObjectToStream(graphData.get(), stream, nullptr, &settings); if (!saveOutcome.IsSuccess()) { return AZ::Failure(AZStd::string::format("JSON serialization failed to save source: %s", saveOutcome.GetError().c_str())); diff --git a/Gems/ScriptCanvas/Code/Editor/Components/EditorGraph.cpp b/Gems/ScriptCanvas/Code/Editor/Components/EditorGraph.cpp index 8d02e61b7d..f032bfe692 100644 --- a/Gems/ScriptCanvas/Code/Editor/Components/EditorGraph.cpp +++ b/Gems/ScriptCanvas/Code/Editor/Components/EditorGraph.cpp @@ -1069,7 +1069,7 @@ namespace ScriptCanvasEditor m_owner = &owner; } - ScriptCanvas::ScriptCanvasData* Graph::GetOwnership() const + ScriptCanvas::DataPtr Graph::GetOwnership() const { return const_cast(this)->m_owner; } @@ -1354,7 +1354,7 @@ namespace ScriptCanvasEditor void Graph::SignalDirty() { - SourceHandle handle(m_owner->shared_from_this(), {}, {}); + SourceHandle handle(m_owner, {}, {}); GeneralRequestBus::Broadcast(&GeneralRequests::SignalSceneDirty, handle); } diff --git a/Gems/ScriptCanvas/Code/Editor/Components/GraphUpgrade.cpp b/Gems/ScriptCanvas/Code/Editor/Components/GraphUpgrade.cpp index e64a47a753..fec813b0c1 100644 --- a/Gems/ScriptCanvas/Code/Editor/Components/GraphUpgrade.cpp +++ b/Gems/ScriptCanvas/Code/Editor/Components/GraphUpgrade.cpp @@ -438,10 +438,11 @@ namespace ScriptCanvasEditor if (nodeConfig.IsValid()) { ScriptCanvas::NodeUpdateSlotReport nodeUpdateSlotReport; + auto nodeEntity = node->GetEntityId(); auto nodeOutcome = graph->ReplaceNodeByConfig(node, nodeConfig, nodeUpdateSlotReport); if (nodeOutcome.IsSuccess()) { - ScriptCanvas::MergeUpdateSlotReport(node->GetEntityId(), sm->m_updateReport, nodeUpdateSlotReport); + ScriptCanvas::MergeUpdateSlotReport(nodeEntity, sm->m_updateReport, nodeUpdateSlotReport); sm->m_allNodes.erase(node); sm->m_outOfDateNodes.erase(node); @@ -687,7 +688,8 @@ namespace ScriptCanvasEditor void EditorGraphUpgradeMachine::OnComplete(IState::ExitStatus exitStatus) { UpgradeNotificationsBus::Broadcast(&UpgradeNotifications::OnGraphUpgradeComplete, m_asset, exitStatus == IState::ExitStatus::Skipped); - m_asset = {}; + // releasing the asset at this stage of the system tick causes a memory crash + // m_asset = {}; } ////////////////////////////////////////////////////////////////////// diff --git a/Gems/ScriptCanvas/Code/Editor/Include/ScriptCanvas/Components/EditorGraph.h b/Gems/ScriptCanvas/Code/Editor/Include/ScriptCanvas/Components/EditorGraph.h index 319a835ba5..be8f274cd6 100644 --- a/Gems/ScriptCanvas/Code/Editor/Include/ScriptCanvas/Components/EditorGraph.h +++ b/Gems/ScriptCanvas/Code/Editor/Include/ScriptCanvas/Components/EditorGraph.h @@ -302,7 +302,7 @@ namespace ScriptCanvasEditor const GraphStatisticsHelper& GetNodeUsageStatistics() const; void MarkOwnership(ScriptCanvas::ScriptCanvasData& owner); - ScriptCanvas::ScriptCanvasData* GetOwnership() const; + ScriptCanvas::DataPtr GetOwnership() const; // Finds and returns all nodes within the graph that are of the specified type template diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/FileSaver.cpp b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/FileSaver.cpp index c23a3d915b..47e43b19d3 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/FileSaver.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/FileSaver.cpp @@ -76,7 +76,7 @@ namespace ScriptCanvasEditor AZ::SystemTickBus::QueueFunction([this, tmpFileName]() { FileSaveResult result; - result.fileSaveError = "Failed to move updated file from temporary location to tmpFileName destination"; + result.fileSaveError = "Failed to move updated file from temporary location to original destination."; result.tempFileRemovalError = RemoveTempFile(tmpFileName); m_onComplete(result); }); @@ -97,7 +97,16 @@ namespace ScriptCanvasEditor } else { + // #sc_editor_asset - skip save, narrow down the memory corruption + AZ::SystemTickBus::QueueFunction([this, tmpFileName]() + { + FileSaveResult result; + result.tempFileRemovalError = RemoveTempFile(tmpFileName); + m_onComplete(result); + }); + // the actual move attempt + /* auto moveResult = AZ::IO::SmartMove(tmpFileName.c_str(), target.c_str()); if (moveResult.GetResultCode() == AZ::IO::ResultCode::Success) { @@ -105,6 +114,7 @@ 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, tmpFileName]() { FileSaveResult result; @@ -124,6 +134,7 @@ namespace ScriptCanvasEditor }); streamer->QueueRequest(flushRequest); } + **/ } } diff --git a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Modifier.cpp b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Modifier.cpp index 242aea355e..3c8605e915 100644 --- a/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Modifier.cpp +++ b/Gems/ScriptCanvas/Code/Editor/View/Windows/Tools/UpgradeTool/Modifier.cpp @@ -138,15 +138,17 @@ namespace ScriptCanvasEditor void Modifier::ModificationComplete(const ModificationResult& result) { - m_result = result; - - if (result.errorMessage.empty()) + if (!result.errorMessage.empty()) { - SaveModifiedGraph(result); + ReportModificationError(result.errorMessage); + } + else if (m_result.asset.Describe() != result.asset.Describe()) + { + ReportModificationError("Received modifiction complete notification for different result"); } else { - ReportModificationError(result.errorMessage); + SaveModifiedGraph(result); } } @@ -187,15 +189,15 @@ namespace ScriptCanvasEditor void Modifier::ReportModificationError(AZStd::string_view report) { - m_result.asset = {}; m_result.errorMessage = report; - m_results.m_failures.push_back(m_result); + m_results.m_failures.push_back({ m_result.asset.Describe(), report }); ModifyNextAsset(); } void Modifier::ReportModificationSuccess() { - m_results.m_successes.push_back(m_result.asset); + m_result.asset = m_result.asset.Describe(); + m_results.m_successes.push_back({ m_result.asset.Describe(), {} }); ModifyNextAsset(); } diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Core.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Core.h index 297ee70500..215041e1fd 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Core.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Core/Core.h @@ -306,8 +306,8 @@ namespace ScriptCanvas { class ScriptCanvasData; - using DataPtr = AZStd::shared_ptr; - using DataPtrConst = AZStd::shared_ptr; + using DataPtr = AZStd::intrusive_ptr; + using DataPtrConst = AZStd::intrusive_ptr; } namespace ScriptCanvasEditor @@ -372,7 +372,7 @@ namespace ScriptCanvasEditor namespace ScriptCanvas { class ScriptCanvasData - : public AZStd::enable_shared_from_this + : public AZStd::intrusive_refcount { public: diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/ParsingMetaData.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/ParsingMetaData.cpp index 476b21d0d6..681bd3b0e4 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/ParsingMetaData.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Grammar/ParsingMetaData.cpp @@ -152,19 +152,19 @@ namespace ScriptCanvas { if (azrtti_istypeof(execution->GetId().m_node)) { - return MetaDataPtr(aznew FormatStringMetaData()); + return AZStd::make_shared(); } else if (azrtti_istypeof(execution->GetId().m_node)) { - return MetaDataPtr(aznew PrintMetaData()); + return AZStd::make_shared(); } else if (azrtti_istypeof(execution->GetId().m_node)) { - return MetaDataPtr(aznew MathExpressionMetaData()); + return AZStd::make_shared(); } else if (execution->GetSymbol() == Symbol::FunctionCall) { - return MetaDataPtr(aznew FunctionCallDefaultMetaData()); + return AZStd::make_shared(); } }