Addresses feedback from PR review

Change DeepCopyNode utility to return a unique_ptr, fix up some
string/path usages to avoid temporaries, etc.

Signed-off-by: amzn-phist <52085794+amzn-phist@users.noreply.github.com>
This commit is contained in:
amzn-phist
2021-07-23 11:29:47 -05:00
parent 91dcbe7fed
commit d385f6ed99
6 changed files with 18 additions and 26 deletions
@@ -9,8 +9,6 @@
#include <AudioWwiseLoader.h>
#include <AzCore/StringFunc/StringFunc.h>
#include <IAudioSystemControl.h>
#include <IAudioSystemEditor.h>
#include <AudioSystemEditor_wwise.h>
@@ -77,7 +75,7 @@ namespace AudioControls
isLocalizedLoaded = true;
}
}
else if (fileName.Extension() == Audio::Wwise::BankExtension && !AZ::StringFunc::Equal(fileName.Native(), Audio::Wwise::InitBank))
else if (fileName.Extension() == Audio::Wwise::BankExtension && fileName != Audio::Wwise::InitBank)
{
m_audioSystemImpl->CreateControl(
SControlDef(AZStd::string{ fileName.Native() }, eWCT_WWISE_SOUND_BANK, isLocalized, nullptr, subPath));
@@ -41,6 +41,6 @@ namespace AudioControls
using FilepathSet = AZStd::set<AZStd::string>;
using XmlAllocator = AZ::rapidxml::memory_pool<>;
inline static XmlAllocator s_xmlAllocator;
inline XmlAllocator s_xmlAllocator;
} // namespace AudioControls
@@ -26,7 +26,7 @@ namespace Audio
AZStd::vector<AZ::IO::FixedMaxPath> foundFiles;
AZ::IO::FileIOBase::FindFilesCallbackType findFilesCallback = [&foundFiles](const char* file) -> bool
{
foundFiles.emplace_back(AZ::IO::FixedMaxPath{ file }.LexicallyNormal());
foundFiles.emplace_back(AZ::IO::PathView{ file }.LexicallyNormal());
return true;
};
@@ -345,7 +345,7 @@ namespace AudioControls
{
for (auto& connectionNode : m_connectionNodes)
{
if (TConnectionPtr connection = audioSystemImpl->CreateConnectionFromXMLNode(connectionNode.m_xmlNode, m_type))
if (TConnectionPtr connection = audioSystemImpl->CreateConnectionFromXMLNode(connectionNode.m_xmlNode.get(), m_type))
{
AddConnection(connection);
connectionNode.m_isValid = true;
@@ -25,11 +25,11 @@ namespace AudioControls
{
SRawConnectionData(AZ::rapidxml::xml_node<char>* node, bool isValid)
{
m_xmlNode = DeepCopyNode(node);
m_xmlNode = AZStd::move(DeepCopyNode(node));
m_isValid = isValid;
}
AZ::rapidxml::xml_node<char>* m_xmlNode{ nullptr };
AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> m_xmlNode{};
// indicates if the connection is valid for the currently loaded middleware
bool m_isValid{ false };
@@ -40,20 +40,20 @@ namespace AudioControls
// will be pointing into the memory pool of an xml document that has gone out of scope.
// This function is a rewritten version of 'clone_node' that does the deep copy of strings
// into the new destination tree.
static AZ::rapidxml::xml_node<char>* DeepCopyNode(AZ::rapidxml::xml_node<char>* srcNode)
[[nodiscard]] static AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> DeepCopyNode(AZ::rapidxml::xml_node<char>* srcNode)
{
AZ::rapidxml::xml_node<char>* destNode = nullptr;
AZStd::unique_ptr<AZ::rapidxml::xml_node<char>> destNode{};
if (srcNode)
{
XmlAllocator& xmlAlloc(AudioControls::s_xmlAllocator);
destNode = xmlAlloc.allocate_node(srcNode->type());
destNode.reset(xmlAlloc.allocate_node(srcNode->type()));
destNode->name(xmlAlloc.allocate_string(srcNode->name(), srcNode->name_size()), srcNode->name_size());
destNode->value(xmlAlloc.allocate_string(srcNode->value(), srcNode->value_size()), srcNode->value_size());
for (AZ::rapidxml::xml_node<char>* child = srcNode->first_node(); child != nullptr; child = child->next_sibling())
{
destNode->append_node(DeepCopyNode(child));
destNode->append_node(DeepCopyNode(child).release());
}
for (AZ::rapidxml::xml_attribute<char>* attr = srcNode->first_attribute(); attr != nullptr; attr = attr->next_attribute())
@@ -352,19 +352,13 @@ namespace AudioControls
{
if (node && control && m_audioSystemImpl)
{
TXmlNodeList otherNodes = control->m_connectionNodes;
auto end = AZStd::remove_if(
otherNodes.begin(), otherNodes.end(),
[](const SRawConnectionData& connection)
{
return connection.m_isValid;
}
);
otherNodes.erase(end, otherNodes.end());
for (auto& connectionNode : otherNodes)
for (auto& connectionNode : control->m_connectionNodes)
{
node->append_node(SRawConnectionData::DeepCopyNode(connectionNode.m_xmlNode));
if (!connectionNode.m_isValid)
{
auto nodeCopy = SRawConnectionData::DeepCopyNode(connectionNode.m_xmlNode.get());
node->append_node(nodeCopy.release());
}
}
const size_t size = control->ConnectionCount();
@@ -391,7 +385,7 @@ namespace AudioControls
IFileUtil* fileUtil = editor ? editor->GetFileUtil() : nullptr;
if (fileUtil)
{
fileUtil->CheckoutFile(filepath.data(), nullptr);
fileUtil->CheckoutFile(AZ::IO::FixedMaxPath{ filepath }.c_str(), nullptr);
}
}
@@ -402,7 +396,7 @@ namespace AudioControls
IFileUtil* fileUtil = editor ? editor->GetFileUtil() : nullptr;
if (fileUtil)
{
fileUtil->DeleteFromSourceControl(filepath.data(), nullptr);
fileUtil->DeleteFromSourceControl(AZ::IO::FixedMaxPath{ filepath }.c_str(), nullptr);
}
}