Added support to skip the output of a window in the Trace system (#16969

) * Added support skip the output of a window in the Trace system A special "<no-window>" window constant as been added to the Trace system via the `AZ::Debug::Trace::NoWindow` constant which allows the output of a trace message to skip printing both the window name parameter and the adjoining ":" separator which is used to split the trace message from the window. Signed-off-by: lumberyard-employee-dm <[email protected]> * Updated the NoWindow constant to be an empty string. The Trace `Output`, `RawOutput` and `Printf` will not default to the NoWindow constant if a nullptr window value is supplied via a `const char*`. In addition Trace fnuction overloads which accept a `std::nullptr_t` type for the window parameter have been added to cause a compile error when `nullptr` has been explicitly passed for the window parameter. The window parameter is expected to be valid c-string value. Signed-off-by: lumberyard-employee-dm <[email protected]> * Removed unneccessary AZ_Warning that never could fail Furthermore the message associated with the warning about using `aznew` over regular C++ `new` has no additional benefits for memory tracking anymore. Signed-off-by: lumberyard-employee-dm <[email protected]> --------- Signed-off-by: lumberyard-employee-dm <[email protected]>
o3de · Oct 30, 2023 · 67763ca · 67763ca
1 parent 31d43f5
commit 67763ca
Show file tree

Hide file tree

Showing 11 changed files with 81 additions and 42 deletions.
diff --git a/Code/Editor/Lib/Tests/test_EditorUtils.cpp b/Code/Editor/Lib/Tests/test_EditorUtils.cpp
@@ -61,7 +61,7 @@ namespace EditorUtilsTest
         WarningDetector detector;
         EditorUtils::AzWarningAbsorber absorber("ignore this");
 
-        AZ_Warning(nullptr, false, "This warning should occur and not be absorbed by the absorber since the window name is nullptr");
+        AZ_Warning(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "This warning should occur and not be absorbed by the absorber since the window name is nullptr");
         ASSERT_TRUE(detector.m_gotWarning);
     }
 }
diff --git a/Code/Editor/Objects/DisplayContextShared.inl b/Code/Editor/Objects/DisplayContextShared.inl
@@ -1086,7 +1086,7 @@ void DisplayContext::DrawTerrainLine(Vec3 worldPos1, Vec3 worldPos2)
 //////////////////////////////////////////////////////////////////////////
 void DisplayContext::DrawTextLabel(const Vec3& pos, float size, const char* text, const bool bCenter, [[maybe_unused]] int srcOffsetX, [[maybe_unused]] int scrOffsetY)
 {
-    AZ_ErrorOnce(nullptr, false, "DisplayContext::DrawTextLabel needs to be removed/ported to use Atom");
+    AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "DisplayContext::DrawTextLabel needs to be removed/ported to use Atom");
 
 #if 0
       ColorF col(m_color4b.r * (1.0f / 255.0f), m_color4b.g * (1.0f / 255.0f), m_color4b.b * (1.0f / 255.0f), m_color4b.a * (1.0f / 255.0f));
@@ -1113,7 +1113,7 @@ void DisplayContext::DrawTextLabel(const Vec3& pos, float size, const char* text
 //////////////////////////////////////////////////////////////////////////
 void DisplayContext::Draw2dTextLabel(float x, float y, float size, const char* text, bool bCenter)
 {
-    AZ_ErrorOnce(nullptr, false, "DisplayContext::Draw2dTextLabel needs to be removed/ported to use Atom");
+    AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "DisplayContext::Draw2dTextLabel needs to be removed/ported to use Atom");
 #if 0
     float col[4] = { m_color4b.r * (1.0f / 255.0f), m_color4b.g * (1.0f / 255.0f), m_color4b.b * (1.0f / 255.0f), m_color4b.a * (1.0f / 255.0f) };
     renderer->Draw2dLabel(x, y, size, col, bCenter, "%s", text);
@@ -1286,7 +1286,7 @@ void DisplayContext::Flush2D()
     int rcw, rch;
     view->GetDimensions(&rcw, &rch);
 
-    AZ_ErrorOnce(nullptr, false, "DisplayContext::Flush2D needs to be removed/ported to use Atom");
+    AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "DisplayContext::Flush2D needs to be removed/ported to use Atom");
 #if 0
 
     TransformationMatrices backupSceneMatrices;

diff --git a/Code/Framework/AzCore/AzCore/Debug/Trace.cpp b/Code/Framework/AzCore/AzCore/Debug/Trace.cpp
@@ -449,9 +449,9 @@ namespace AZ::Debug
         }
 
         using namespace DebugInternal;
-        if (!window)
+        if (window == nullptr)
         {
-            window = g_dbgSystemWnd;
+            window = NoWindow;
         }
 
         char message[g_maxMessageLength];
@@ -465,7 +465,7 @@ namespace AZ::Debug
 
         va_list mark;
         va_start(mark, format);
-        azvsnprintf(message, g_maxMessageLength-1, format, mark); // -1 to make room for the "/n" that will be appended below
+        azvsnprintf(message, g_maxMessageLength - 1, format, mark); // -1 to make room for the "/n" that will be appended below
         va_end(mark);
 
         TraceMessageResult result;
@@ -496,8 +496,7 @@ namespace AZ::Debug
     // Warning
     // [8/3/2009]
     //=========================================================================
-    void
-    Trace::Warning(const char* fileName, int line, const char* funcName, const char* window, const char* format, ...)
+    void Trace::Warning(const char* fileName, int line, const char* funcName, const char* window, const char* format, ...)
     {
         if (!IsTraceLoggingEnabledForLevel(LogLevel::Warnings))
         {
@@ -533,12 +532,11 @@ namespace AZ::Debug
     // Printf
     // [8/3/2009]
     //=========================================================================
-    void
-    Trace::Printf(const char* window, const char* format, ...)
+    void Trace::Printf(const char* window, const char* format, ...)
     {
-        if (!window)
+        if (window == nullptr)
         {
-            window = g_dbgSystemWnd;
+            window = NoWindow;
         }
 
         char message[g_maxMessageLength];
@@ -564,9 +562,9 @@ namespace AZ::Debug
     //=========================================================================
     void Trace::Output(const char* window, const char* message)
     {
-        if (!window)
+        if (window == nullptr)
         {
-            window = g_dbgSystemWnd;
+            window = NoWindow;
         }
 
         Platform::OutputToDebugger(window, message);
@@ -589,14 +587,15 @@ namespace AZ::Debug
 
     void Trace::RawOutput(const char* window, const char* message)
     {
-        if (!window)
+        if (window == nullptr)
         {
-            window = g_dbgSystemWnd;
+            window = NoWindow;
         }
 
-
         // printf on Windows platforms seem to have a buffer length limit of 4096 characters
         // Therefore fwrite is used directly to write the window and message to stdout or stderr
+
+        // Wrapping the NoWindow constant in a string_view to allow use of string_view::operator== for string compare
         AZStd::string_view windowView{ window };
         AZStd::string_view messageView{ message };
         constexpr AZStd::string_view windowMessageSeparator{ ": " };
@@ -606,8 +605,11 @@ namespace AZ::Debug
         FILE* stdoutStream = stdout;
         if (FILE* rawOutputStream = s_fileStream ? *s_fileStream : stdoutStream; rawOutputStream != nullptr)
         {
-            fwrite(windowView.data(), 1, windowView.size(), rawOutputStream);
-            fwrite(windowMessageSeparator.data(), 1, windowMessageSeparator.size(), rawOutputStream);
+            if (!windowView.empty())
+            {
+                fwrite(windowView.data(), 1, windowView.size(), rawOutputStream);
+                fwrite(windowMessageSeparator.data(), 1, windowMessageSeparator.size(), rawOutputStream);
+            }
             fwrite(messageView.data(), 1, messageView.size(), rawOutputStream);
         }
     }

diff --git a/Code/Framework/AzCore/AzCore/Debug/Trace.h b/Code/Framework/AzCore/AzCore/Debug/Trace.h
@@ -25,6 +25,9 @@ namespace AZ
 {
     namespace Debug
     {
+        // The NoWindow constant contains a special case string that can skip outputting the
+        // "<window-name> :" portion of a trace message of the form "<window-name>: <message>"
+        inline constexpr const char* NoWindow = "";
         namespace Platform
         {
             void OutputToDebugger(AZStd::basic_string_view<char, AZStd::char_traits<char>> window, AZStd::basic_string_view<char, AZStd::char_traits<char>> message);
@@ -121,8 +124,18 @@ namespace AZ
             {
                 fprintf(stdout, "%s: %s\n", window, message);
             }
+
             virtual void PrintCallstack(const char* /*window*/, unsigned int /*suppressCount*/ = 0, void* /*nativeContext*/ = nullptr) {}
 
+            // Catch the cases when an explicit nullptr has been passed in to the Trace window parameter
+            // A valid c-string parameter must at least be passed in
+            void Error(const char* fileName, int line, const char* funcName, std::nullptr_t window, const char* format, ...) = delete;
+            void Warning(const char* fileName, int line, const char* funcName, std::nullptr_t window, const char* format, ...) = delete;
+            void Printf(std::nullptr_t, const char* format, ...) = delete;
+            void Output(std::nullptr_t, const char* message) = delete;
+            void RawOutput(std::nullptr_t, const char* message) = delete;
+            void PrintCallstack(std::nullptr_t, unsigned int suppressCount = 0, void* nativeContext = nullptr) = delete;
+
         private:
             inline static constexpr size_t s_maxMessageLength = 4096;
             inline static ITrace* s_tracer{};
@@ -147,6 +160,10 @@ namespace AZ
             * or to force a Trace message bus handler to do special processing by using a known, consistent char*
             */
             static const char* GetDefaultSystemWindow();
+
+            //! Returns a Window string that indicates that the window
+            //! parameter and the separating ":" not be part of the output
+            static constexpr const char* GetNoWindow() { return NoWindow; }
             bool IsDebuggerPresent() override;
             static bool AttachDebugger();
             static bool WaitForDebugger(float timeoutSeconds = -1.f);

diff --git a/Code/Framework/AzCore/AzCore/Memory/Memory_fwd.h b/Code/Framework/AzCore/AzCore/Memory/Memory_fwd.h
@@ -166,7 +166,6 @@
     /* ========== standard operator new/delete ========== */                                                                                                                        \
     AZ_FORCE_INLINE void* operator new(std::size_t size) {                      /* default operator new (called with "new _Class()") */                                             \
         AZ_Assert(size == sizeof(_Class), "Size mismatch! Did you forget to declare the macro in derived class? Size: %d sizeof(%s): %d", size, #_Class, sizeof(_Class));           \
-        AZ_Warning(0, true/*false*/, "Make sure you use aznew, offers better tracking! (%s)", #_Class /*Warning temporarily disabled until engine is using AZ allocators.*/);       \
         return AZ::AllocatorInstance< _Allocator >::Get().allocate(size, alignof( _Class ));                                                                                        \
     }                                                                                                                                                                               \
     AZ_FORCE_INLINE void  operator delete(void* p, std::size_t size) {    /* default operator delete */                                                                             \

diff --git a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp
@@ -109,9 +109,9 @@ namespace AZ::Debug
     {
         char message[MaxMessageLength];
         // Trace::RawOutput
-        Debug::Trace::Instance().RawOutput(nullptr, "==================================================================\n");
+        Debug::Trace::Instance().RawOutput(AZ::Debug::Trace::GetDefaultSystemWindow(), "==================================================================\n");
         azsnprintf(message, MaxMessageLength, "Error: signal %s: \n", strsignal(signal));
-        Debug::Trace::Instance().RawOutput(nullptr, message);
+        Debug::Trace::Instance().RawOutput(AZ::Debug::Trace::GetDefaultSystemWindow(), message);
 
         StackFrame frames[MaxStackLines];
         SymbolStorage::StackLine stackLines[MaxStackLines];
@@ -121,9 +121,9 @@ namespace AZ::Debug
         for (int i = 0; i < numberOfFrames; ++i)
         {
             azsnprintf(message, MaxMessageLength, "%s \n", stackLines[i]);
-            Debug::Trace::Instance().RawOutput(nullptr, message);
+            Debug::Trace::Instance().RawOutput(AZ::Debug::Trace::GetDefaultSystemWindow(), message);
         }
-        Debug::Trace::Instance().RawOutput(nullptr, "==================================================================\n");
+        Debug::Trace::Instance().RawOutput(AZ::Debug::Trace::GetDefaultSystemWindow(), "==================================================================\n");
 
         // Continue to exit the process when a signal is caught by this signal handler. 
         // The exit code will conform to https://tldp.org/LDP/abs/html/exitcodes.html.

diff --git a/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/Debug/Trace_WinAPI.cpp b/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/Debug/Trace_WinAPI.cpp
@@ -99,6 +99,7 @@ namespace AZ::Debug
         void OutputToDebugger(AZStd::string_view window, AZStd::string_view message)
         {
             AZStd::fixed_wstring<g_maxMessageLength> tmpW;
+            // Only print the window if it is not an empty string
             if(!window.empty())
             {
                 while (window.size() > tmpW.max_size())
@@ -182,8 +183,8 @@ namespace AZ::Debug
         {
             // prevent g_tracer from calling the tracebus:
             DebugInternal::g_suppressEBusCalls = true;
-            Debug::Trace::Instance().Output(nullptr, "Exception handler loop!");
-            Debug::Trace::Instance().PrintCallstack(nullptr, 0, ExceptionInfo->ContextRecord);
+            Debug::Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "Exception handler loop!");
+            Debug::Trace::Instance().PrintCallstack(AZ::Debug::Trace::GetDefaultSystemWindow(), 0, ExceptionInfo->ContextRecord);
             DebugInternal::g_suppressEBusCalls = false;
 
             if (Debug::Trace::Instance().IsDebuggerPresent())
@@ -200,23 +201,23 @@ namespace AZ::Debug
         g_exceptionInfo = (void*)ExceptionInfo;
 
         char message[g_maxMessageLength];
-        Debug::Trace::Instance().Output(nullptr, "==================================================================\n");
+        Debug::Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "==================================================================\n");
         azsnprintf(message, g_maxMessageLength, "Exception : 0x%lX - '%s' [%p]\n", ExceptionInfo->ExceptionRecord->ExceptionCode, GetExeptionName(ExceptionInfo->ExceptionRecord->ExceptionCode), ExceptionInfo->ExceptionRecord->ExceptionAddress);
-        Debug::Trace::Instance().Output(nullptr, message);
+        Debug::Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), message);
 
-        Debug::Trace::Instance().PrintCallstack(nullptr, 0, ExceptionInfo->ContextRecord);
+        Debug::Trace::Instance().PrintCallstack(AZ::Debug::Trace::GetDefaultSystemWindow(), 0, ExceptionInfo->ContextRecord);
 
         bool result = false;
         Debug::TraceMessageBus::BroadcastResult(result, &Debug::TraceMessageBus::Events::OnException, message);
         if (result)
         {
-            Debug::Trace::Instance().Output(nullptr, "==================================================================\n");
+            Debug::Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "==================================================================\n");
             g_exceptionInfo = NULL;
             // if someone ever returns TRUE we assume that they somehow handled this exception and continue.
             return EXCEPTION_CONTINUE_EXECUTION;
         }
-        
-        Debug::Trace::Instance().Output(nullptr, "==================================================================\n");
+
+        Debug::Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "==================================================================\n");
 
         // allowing continue of execution is not valid here.  This handler gets called for serious exceptions.
         // programs wanting things like a message box can implement them on a case-by-case basis, but we want no such 

diff --git a/Code/Legacy/CrySystem/AZCoreLogSink.h b/Code/Legacy/CrySystem/AZCoreLogSink.h
@@ -94,6 +94,15 @@ class AZCoreLogSink
             return false; // allow AZCore to do its default behavior.
         }
         gEnv->pLog->LogError("(%s) - %s", window, message);
+        AZStd::string_view windowView = window;
+        if (!windowView.empty())
+        {
+            gEnv->pLog->LogError("(%s) - %s", window, message);
+        }
+        else
+        {
+            gEnv->pLog->LogError("%s", message);
+        }
         return m_suppressSystemOutput;
     }
 
@@ -108,7 +117,15 @@ class AZCoreLogSink
             return false; // allow AZCore to do its default behavior.
         }
 
-        CryWarning(VALIDATOR_MODULE_UNKNOWN, VALIDATOR_WARNING, "(%s) - %s", window, message);
+        AZStd::string_view windowView = window;
+        if (!windowView.empty())
+        {
+            CryWarning(VALIDATOR_MODULE_UNKNOWN, VALIDATOR_WARNING, "(%s) - %s", window, message);
+        }
+        else
+        {
+            CryWarning(VALIDATOR_MODULE_UNKNOWN, VALIDATOR_WARNING, "%s", message);
+        }
         return m_suppressSystemOutput;
     }
 
@@ -119,7 +136,10 @@ class AZCoreLogSink
             return false; // allow AZCore to do its default behavior.
         }
 
-        if (window == AZ::Debug::Trace::GetDefaultSystemWindow())
+        AZStd::string_view windowView = window;
+
+        // Only print out the window if it is not equal to the NoWindow or the DefaultSystemWindow value
+        if (windowView == AZ::Debug::Trace::GetNoWindow() || windowView == AZ::Debug::Trace::GetDefaultSystemWindow())
         {
             [[maybe_unused]] auto WriteToStream = [message = AZStd::string_view(message)]
             (AZ::IO::GenericStream& stream)

diff --git a/Code/Legacy/CrySystem/AZCrySystemInitLogSink.cpp b/Code/Legacy/CrySystem/AZCrySystemInitLogSink.cpp
@@ -39,9 +39,9 @@ namespace AZ
                 msgBoxMessage.append(errMsg.c_str());
             }
 
-            Trace::Instance().Output(nullptr, "\n==================================================================\n");
-            Trace::Instance().Output(nullptr, msgBoxMessage.c_str());
-            Trace::Instance().Output(nullptr, "\n==================================================================\n");
+            Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "\n==================================================================\n");
+            Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), msgBoxMessage.c_str());
+            Trace::Instance().Output(AZ::Debug::Trace::GetDefaultSystemWindow(), "\n==================================================================\n");
 
             EBUS_EVENT(AZ::NativeUI::NativeUIRequestBus, DisplayOkDialog, "O3DE Initialization Failed", msgBoxMessage.c_str(), false);
         }

diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp
@@ -173,7 +173,7 @@ namespace ShaderManagementConsole
             sourceAssetFileName.c_str(),
             assetInfo,
             watchFolder);
-        AZ_Error(nullptr, result, "Failed to get the asset info for the file: %s.", sourceAssetFileName.c_str());
+        AZ_Error(AZ::Debug::Trace::GetDefaultSystemWindow(), result, "Failed to get the asset info for the file: %s.", sourceAssetFileName.c_str());
 
         return assetInfo;
     }

diff --git a/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp b/Gems/LyShine/Code/Source/LyShineLoadScreen.cpp
@@ -61,7 +61,7 @@ namespace LyShine
             return false;
         }
         //TODO: gEnv->pRenderer is always null, fix the logic below
-        AZ_ErrorOnce(nullptr, false, "NotifyGameLoadStart needs to be removed/ported to use Atom");
+        AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "NotifyGameLoadStart needs to be removed/ported to use Atom");
         return false;
 #if 0
         if (!gEnv || gEnv->pRenderer || !AZ::Interface<ILyShine>::Get())
@@ -100,7 +100,7 @@ namespace LyShine
             return false;
         }
 
-        AZ_ErrorOnce(nullptr, false, "NotifyLevelLoadStart needs to be removed/ported to use Atom");
+        AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), false, "NotifyLevelLoadStart needs to be removed/ported to use Atom");
         return false;
         //TODO: gEnv->pRenderer is always null, fix the logic below
 #if 0
@@ -141,7 +141,7 @@ namespace LyShine
     void LyShineLoadScreenComponent::UpdateAndRender([[maybe_unused]] float deltaTimeInSeconds)
     {
         AZ_Assert(m_isPlaying, "LyShineLoadScreenComponent should not be connected to LoadScreenUpdateNotificationBus while not playing");
-        AZ_ErrorOnce(nullptr, m_isPlaying && AZ::Interface<ILyShine>::Get(), "UpdateAndRender needs to be removed/ported to use Atom");
+        AZ_ErrorOnce(AZ::Debug::Trace::GetDefaultSystemWindow(), m_isPlaying && AZ::Interface<ILyShine>::Get(), "UpdateAndRender needs to be removed/ported to use Atom");
 
         //TODO: gEnv->pRenderer is always null, fix the logic below
 #if 0