From 306a7a622df6a427dbe75bb0eb38cba2b2ed18de Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Tue, 24 Aug 2021 15:29:00 -0500 Subject: [PATCH] Fixed memory stomping leading to Track View nodes with invalid labels and crash. Signed-off-by: Chris Galvan --- Code/Editor/TrackView/TrackViewDialog.cpp | 3 ++- Code/Editor/TrackView/TrackViewNodes.cpp | 21 +++++++++++-------- Code/Legacy/CryCommon/IMovieSystem.h | 2 +- .../Source/Cinematics/AnimComponentNode.h | 8 +++---- .../Code/Source/Cinematics/AnimNode.cpp | 2 +- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Code/Editor/TrackView/TrackViewDialog.cpp b/Code/Editor/TrackView/TrackViewDialog.cpp index f1701f9e20..26d844a68b 100644 --- a/Code/Editor/TrackView/TrackViewDialog.cpp +++ b/Code/Editor/TrackView/TrackViewDialog.cpp @@ -2033,7 +2033,8 @@ void CTrackViewDialog::UpdateTracksToolBar() continue; } - name = pAnimNode->GetParamName(paramType); + AZStd::string paramName = pAnimNode->GetParamName(paramType); + name = paramName.c_str(); QString sToolTipText("Add " + name + " Track"); QIcon hIcon = m_wndNodesCtrl->GetIconForTrack(pTrack); diff --git a/Code/Editor/TrackView/TrackViewNodes.cpp b/Code/Editor/TrackView/TrackViewNodes.cpp index ae38323cc3..62b134a508 100644 --- a/Code/Editor/TrackView/TrackViewNodes.cpp +++ b/Code/Editor/TrackView/TrackViewNodes.cpp @@ -616,7 +616,8 @@ CTrackViewNodesCtrl::CRecord* CTrackViewNodesCtrl::AddAnimNodeRecord(CRecord* pP { CRecord* pNewRecord = new CRecord(animNode); - pNewRecord->setText(0, animNode->GetName()); + AZStd::string nodeName = animNode->GetName(); + pNewRecord->setText(0, nodeName.c_str()); UpdateAnimNodeRecord(pNewRecord, animNode); pParentRecord->insertChild(GetInsertPosition(pParentRecord, animNode), pNewRecord); FillNodesRec(pNewRecord, animNode); @@ -629,7 +630,8 @@ CTrackViewNodesCtrl::CRecord* CTrackViewNodesCtrl::AddTrackRecord(CRecord* pPare { CRecord* pNewTrackRecord = new CRecord(pTrack); pNewTrackRecord->setSizeHint(0, QSize(30, 18)); - pNewTrackRecord->setText(0, pTrack->GetName()); + AZStd::string trackName = pTrack->GetName(); + pNewTrackRecord->setText(0, trackName.c_str()); UpdateTrackRecord(pNewTrackRecord, pTrack); pParentRecord->insertChild(GetInsertPosition(pParentRecord, pTrack), pNewTrackRecord); FillNodesRec(pNewTrackRecord, pTrack); @@ -2348,13 +2350,13 @@ bool CTrackViewNodesCtrl::FillAddTrackMenu(STrackMenuTreeNode& menuAddTrack, con continue; } } - name = animNode->GetParamName(paramType); + AZStd::string paramName = animNode->GetParamName(paramType); + name = paramName.c_str(); QStringList splittedName = name.split("/", Qt::SkipEmptyParts); STrackMenuTreeNode* pCurrentNode = &menuAddTrack; - for (int j = 0; j < splittedName.size() - 1; ++j) + for (const QString& segment : splittedName) { - const QString& segment = splittedName[j]; auto findIter = pCurrentNode->children.find(segment); if (findIter != pCurrentNode->children.end()) { @@ -2370,7 +2372,7 @@ bool CTrackViewNodesCtrl::FillAddTrackMenu(STrackMenuTreeNode& menuAddTrack, con // only add tracks to the that STrackMenuTreeNode tree that haven't already been added CTrackViewTrackBundle matchedTracks = animNode->GetTracksByParam(paramType); - if (matchedTracks.GetCount() == 0) + if (matchedTracks.GetCount() == 0 && !splittedName.isEmpty()) { STrackMenuTreeNode* pParamNode = new STrackMenuTreeNode; pCurrentNode->children[splittedName.back()] = std::unique_ptr(pParamNode); @@ -2580,10 +2582,11 @@ void CTrackViewNodesCtrl::Update() { const CTrackViewAnimNode* track = static_cast(node); if (track) - { - record->setText(0, track->GetName()); + { + AZStd::string trackName = track->GetName(); + record->setText(0, trackName.c_str()); } - } + } } } } diff --git a/Code/Legacy/CryCommon/IMovieSystem.h b/Code/Legacy/CryCommon/IMovieSystem.h index 915dc925bf..2385f3c358 100644 --- a/Code/Legacy/CryCommon/IMovieSystem.h +++ b/Code/Legacy/CryCommon/IMovieSystem.h @@ -625,7 +625,7 @@ public: , valueType(_valueType) , flags(_flags) {}; - const char* name; // parameter name. + AZStd::string name; // parameter name. CAnimParamType paramType; // parameter id. AnimValueType valueType; // value type, defines type of track to use for animating this parameter. ESupportedParamFlags flags; // combination of flags from ESupportedParamFlags. diff --git a/Gems/Maestro/Code/Source/Cinematics/AnimComponentNode.h b/Gems/Maestro/Code/Source/Cinematics/AnimComponentNode.h index 5ad3ab6f94..867afcb28e 100644 --- a/Gems/Maestro/Code/Source/Cinematics/AnimComponentNode.h +++ b/Gems/Maestro/Code/Source/Cinematics/AnimComponentNode.h @@ -150,16 +150,16 @@ private: } BehaviorPropertyInfo(const BehaviorPropertyInfo& other) { - m_displayName = other.m_displayName; - m_animNodeParamInfo.paramType = other.m_displayName; - m_animNodeParamInfo.name = &m_displayName[0]; + m_displayName = AZStd::move(other.m_displayName); + m_animNodeParamInfo.paramType = m_displayName; + m_animNodeParamInfo.name = m_displayName; } BehaviorPropertyInfo& operator=(const AZStd::string& str) { // TODO: clean this up - this weird memory sharing was copied from legacy Cry - could be better. m_displayName = str; m_animNodeParamInfo.paramType = str; // set type to AnimParamType::ByString by assigning a string - m_animNodeParamInfo.name = &m_displayName[0]; + m_animNodeParamInfo.name = m_displayName; return *this; } diff --git a/Gems/Maestro/Code/Source/Cinematics/AnimNode.cpp b/Gems/Maestro/Code/Source/Cinematics/AnimNode.cpp index 828e5ad20f..c238d40ca8 100644 --- a/Gems/Maestro/Code/Source/Cinematics/AnimNode.cpp +++ b/Gems/Maestro/Code/Source/Cinematics/AnimNode.cpp @@ -82,7 +82,7 @@ const char* CAnimNode::GetParamName(const CAnimParamType& paramType) const SParamInfo info; if (GetParamInfoFromType(paramType, info)) { - return info.name; + return info.name.c_str(); } return "Unknown";