From 606de5427b161d7cad7e1abfc0b5f10c7f7938c8 Mon Sep 17 00:00:00 2001 From: ffarahmand-DPS Date: Fri, 15 Oct 2021 02:30:48 -0700 Subject: [PATCH] Fixes debug console autocomplete issues (#4223) * Fixed a crash caused by large autocomplete results in the debug console. A fixed vector was growing beyond its allocated size. Signed-off-by: ffarahmand-DPS * Fixes printing duplicate autocomplete results, caused by looping over multiple CVARs registered with the same name. Also adds an erase to prevent undefined behavior. Signed-off-by: ffarahmand-DPS * Adds a test case for autocomplete duplication in the event of multiple cvars existing under the same name. Two matching cvars are created and checked against the number of matches produced by autocomplete. Signed-off-by: ffarahmand-DPS * Added two safety checks and made a pointer const as per reviewer feedback. Signed-off-by: ffarahmand-DPS --- .../AzCore/AzCore/Console/Console.cpp | 27 ++++++++++++++++--- .../AzCore/Tests/Console/ConsoleTests.cpp | 15 +++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Console/Console.cpp b/Code/Framework/AzCore/AzCore/Console/Console.cpp index 9f207d9afd..79e1f79697 100644 --- a/Code/Framework/AzCore/AzCore/Console/Console.cpp +++ b/Code/Framework/AzCore/AzCore/Console/Console.cpp @@ -225,8 +225,16 @@ namespace AZ ConsoleCommandContainer commandSubset; - for (ConsoleFunctorBase* curr = m_head; curr != nullptr; curr = curr->m_next) + for (const auto& functor : m_commands) { + if (functor.second.empty()) + { + continue; + } + + // Filter functors registered with the same name + const ConsoleFunctorBase* curr = functor.second.front(); + if ((curr->GetFlags() & ConsoleFunctorFlags::IsInvisible) == ConsoleFunctorFlags::IsInvisible) { // Filter functors marked as invisible @@ -236,7 +244,12 @@ namespace AZ if (StringFunc::StartsWith(curr->m_name, command, false)) { AZLOG_INFO("- %s : %s\n", curr->m_name, curr->m_desc); - commandSubset.push_back(curr->m_name); + + if (commandSubset.size() < MaxConsoleCommandPlusArgsLength) + { + commandSubset.push_back(curr->m_name); + } + if (matches) { matches->push_back(curr->m_name); @@ -271,7 +284,10 @@ namespace AZ { for (auto& curr : m_commands) { - visitor(curr.second.front()); + if (!curr.second.empty()) + { + visitor(curr.second.front()); + } } } @@ -336,6 +352,11 @@ namespace AZ { iter->second.erase(iter2); } + + if (iter->second.empty()) + { + m_commands.erase(iter); + } } functor->Unlink(m_head); functor->m_console = nullptr; diff --git a/Code/Framework/AzCore/Tests/Console/ConsoleTests.cpp b/Code/Framework/AzCore/Tests/Console/ConsoleTests.cpp index d91ec5ba58..e01129a7bc 100644 --- a/Code/Framework/AzCore/Tests/Console/ConsoleTests.cpp +++ b/Code/Framework/AzCore/Tests/Console/ConsoleTests.cpp @@ -288,6 +288,21 @@ namespace AZ AZStd::string completeCommand = console->AutoCompleteCommand("testVec3"); AZ_TEST_ASSERT(completeCommand == "testVec3"); } + + // Duplicate names + { + // Register two cvars with the same name + auto id = AZ::TypeId(); + auto flag = AZ::ConsoleFunctorFlags::Null; + auto signature = AZ::ConsoleFunctor::FunctorSignature(); + AZ::ConsoleFunctor cvarOne(*console, "testAutoCompleteDuplication", "", flag, id, signature); + AZ::ConsoleFunctor cvarTwo(*console, "testAutoCompleteDuplication", "", flag, id, signature); + + // Autocomplete given name expecting one match (not two) + AZStd::vector matches; + AZStd::string completeCommand = console->AutoCompleteCommand("testAutoCompleteD", &matches); + AZ_TEST_ASSERT(matches.size() == 1 && completeCommand == "testAutoCompleteDuplication"); + } } TEST_F(ConsoleTests, ConsoleFunctor_FreeFunctorExecutionTest)