From 24494d840e06402c26c03db36695b63c5322d624 Mon Sep 17 00:00:00 2001 From: Danil Alexeev Date: Mon, 5 May 2025 19:08:19 +0300 Subject: [PATCH] Core: Unify display of error type prefixes --- core/debugger/local_debugger.cpp | 2 +- core/error/error_macros.cpp | 13 +++--- core/error/error_macros.h | 14 ++++++ core/io/logger.cpp | 19 +------- core/io/logger.h | 28 ++++++++++++ .../terminal_logger_apple_embedded.mm | 26 ++--------- drivers/unix/os_unix.cpp | 24 ++++++----- drivers/unix/syslog_logger.cpp | 21 +-------- editor/import/3d/collada.cpp | 2 +- modules/gdscript/gdscript.cpp | 4 +- .../gdscript/tests/gdscript_test_runner.cpp | 25 ++--------- platform/macos/macos_terminal_logger.mm | 43 ++++++++----------- platform/windows/windows_terminal_logger.cpp | 33 ++++---------- 13 files changed, 102 insertions(+), 152 deletions(-) diff --git a/core/debugger/local_debugger.cpp b/core/debugger/local_debugger.cpp index 47a6d15df41..fb09c39c2d0 100644 --- a/core/debugger/local_debugger.cpp +++ b/core/debugger/local_debugger.cpp @@ -362,7 +362,7 @@ void LocalDebugger::send_message(const String &p_message, const Array &p_args) { } void LocalDebugger::send_error(const String &p_func, const String &p_file, int p_line, const String &p_err, const String &p_descr, bool p_editor_notify, ErrorHandlerType p_type) { - print_line("ERROR: '" + (p_descr.is_empty() ? p_err : p_descr) + "'"); + _err_print_error(p_func.utf8().get_data(), p_file.utf8().get_data(), p_line, p_err, p_descr, p_editor_notify, p_type); } LocalDebugger::LocalDebugger() { diff --git a/core/error/error_macros.cpp b/core/error/error_macros.cpp index 220a5741a6d..ad53d13344f 100644 --- a/core/error/error_macros.cpp +++ b/core/error/error_macros.cpp @@ -96,10 +96,11 @@ void _err_print_error(const char *p_function, const char *p_file, int p_line, co } else { // Fallback if errors happen before OS init or after it's destroyed. const char *err_details = (p_message && *p_message) ? p_message : p_error; - fprintf(stderr, "ERROR: %s\n at: %s (%s:%i)\n", err_details, p_function, p_file, p_line); + fprintf(stderr, "%s: %s\n at: %s (%s:%i)\n", _error_handler_type_string(p_type), err_details, p_function, p_file, p_line); } _global_lock(); + ErrorHandlerList *l = error_handler_list; while (l) { l->errfunc(l->userdata, p_function, p_file, p_line, p_error, p_message, p_editor_notify, p_type); @@ -113,18 +114,20 @@ void _err_print_error(const char *p_function, const char *p_file, int p_line, co // but we don't want to make it noisy by printing lots of file & line info (because it's already // been printing by a preceding _err_print_error). void _err_print_error_asap(const String &p_error, ErrorHandlerType p_type) { + const char *err_details = p_error.utf8().get_data(); + if (OS::get_singleton()) { - OS::get_singleton()->printerr("ERROR: %s\n", p_error.utf8().get_data()); + OS::get_singleton()->printerr("%s: %s\n", _error_handler_type_string(p_type), err_details); } else { // Fallback if errors happen before OS init or after it's destroyed. - const char *err_details = p_error.utf8().get_data(); - fprintf(stderr, "ERROR: %s\n", err_details); + fprintf(stderr, "%s: %s\n", _error_handler_type_string(p_type), err_details); } _global_lock(); + ErrorHandlerList *l = error_handler_list; while (l) { - l->errfunc(l->userdata, "", "", 0, p_error.utf8().get_data(), "", false, p_type); + l->errfunc(l->userdata, "", "", 0, err_details, "", false, p_type); l = l->next; } diff --git a/core/error/error_macros.h b/core/error/error_macros.h index aa2ed4f7bb2..6ddcce3506e 100644 --- a/core/error/error_macros.h +++ b/core/error/error_macros.h @@ -46,6 +46,20 @@ enum ErrorHandlerType { ERR_HANDLER_SHADER, }; +constexpr const char *_error_handler_type_string(ErrorHandlerType p_type) { + switch (p_type) { + case ERR_HANDLER_ERROR: + return "ERROR"; + case ERR_HANDLER_WARNING: + return "WARNING"; + case ERR_HANDLER_SCRIPT: + return "SCRIPT ERROR"; + case ERR_HANDLER_SHADER: + return "SHADER ERROR"; + } + return "UNKNOWN ERROR"; +} + // Pointer to the error handler printing function. Reassign to any function to have errors printed. // Parameters: userdata, function, file, line, error, explanation, type. typedef void (*ErrorHandlerFunc)(void *, const char *, const char *, int p_line, const char *, const char *, bool p_editor_notify, ErrorHandlerType p_type); diff --git a/core/io/logger.cpp b/core/io/logger.cpp index 4de48829d49..e15a61f7eed 100644 --- a/core/io/logger.cpp +++ b/core/io/logger.cpp @@ -58,24 +58,7 @@ void Logger::log_error(const char *p_function, const char *p_file, int p_line, c return; } - const char *err_type = "ERROR"; - switch (p_type) { - case ERR_ERROR: - err_type = "ERROR"; - break; - case ERR_WARNING: - err_type = "WARNING"; - break; - case ERR_SCRIPT: - err_type = "SCRIPT ERROR"; - break; - case ERR_SHADER: - err_type = "SHADER ERROR"; - break; - default: - ERR_PRINT("Unknown error type"); - break; - } + const char *err_type = error_type_string(p_type); const char *err_details; if (p_rationale && *p_rationale) { diff --git a/core/io/logger.h b/core/io/logger.h index ae8884d8110..4acd17df442 100644 --- a/core/io/logger.h +++ b/core/io/logger.h @@ -53,6 +53,34 @@ public: ERR_SHADER }; + static constexpr const char *error_type_string(ErrorType p_type) { + switch (p_type) { + case ERR_ERROR: + return "ERROR"; + case ERR_WARNING: + return "WARNING"; + case ERR_SCRIPT: + return "SCRIPT ERROR"; + case ERR_SHADER: + return "SHADER ERROR"; + } + return "UNKNOWN ERROR"; + } + + static constexpr const char *error_type_indent(ErrorType p_type) { + switch (p_type) { + case ERR_ERROR: + return " "; + case ERR_WARNING: + return " "; + case ERR_SCRIPT: + return " "; + case ERR_SHADER: + return " "; + } + return " "; + } + static void set_flush_stdout_on_print(bool value); virtual void logv(const char *p_format, va_list p_list, bool p_err) _PRINTF_FORMAT_ATTRIBUTE_2_0 = 0; diff --git a/drivers/apple_embedded/terminal_logger_apple_embedded.mm b/drivers/apple_embedded/terminal_logger_apple_embedded.mm index 647d371c119..4889b244f25 100644 --- a/drivers/apple_embedded/terminal_logger_apple_embedded.mm +++ b/drivers/apple_embedded/terminal_logger_apple_embedded.mm @@ -46,29 +46,9 @@ void TerminalLoggerAppleEmbedded::log_error(const char *p_function, const char * err_details = p_code; } - switch (p_type) { - case ERR_WARNING: - os_log_error(OS_LOG_DEFAULT, - "WARNING: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - break; - case ERR_SCRIPT: - os_log_error(OS_LOG_DEFAULT, - "SCRIPT ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - break; - case ERR_SHADER: - os_log_error(OS_LOG_DEFAULT, - "SHADER ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - break; - case ERR_ERROR: - default: - os_log_error(OS_LOG_DEFAULT, - "ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - break; - } + os_log_error(OS_LOG_DEFAULT, + "%{public}s: %{public}s\nat: %{public}s (%{public}s:%i)", + error_type_string(p_type), err_details, p_function, p_file, p_line); for (const Ref &backtrace : p_script_backtraces) { if (!backtrace->is_empty()) { diff --git a/drivers/unix/os_unix.cpp b/drivers/unix/os_unix.cpp index 5cf50b92ed4..31f7392d9b4 100644 --- a/drivers/unix/os_unix.cpp +++ b/drivers/unix/os_unix.cpp @@ -1221,32 +1221,34 @@ void UnixTerminalLogger::log_error(const char *p_function, const char *p_file, i const char *cyan_bold = tty ? "\E[1;36m" : ""; const char *reset = tty ? "\E[0m" : ""; - const char *indent = ""; + const char *bold_color; + const char *normal_color; switch (p_type) { case ERR_WARNING: - indent = " "; - logf_error("%sWARNING:%s %s\n", yellow_bold, yellow, err_details); + bold_color = yellow_bold; + normal_color = yellow; break; case ERR_SCRIPT: - indent = " "; - logf_error("%sSCRIPT ERROR:%s %s\n", magenta_bold, magenta, err_details); + bold_color = magenta_bold; + normal_color = magenta; break; case ERR_SHADER: - indent = " "; - logf_error("%sSHADER ERROR:%s %s\n", cyan_bold, cyan, err_details); + bold_color = cyan_bold; + normal_color = cyan; break; case ERR_ERROR: default: - indent = " "; - logf_error("%sERROR:%s %s\n", red_bold, red, err_details); + bold_color = red_bold; + normal_color = red; break; } - logf_error("%s%sat: %s (%s:%i)%s\n", gray, indent, p_function, p_file, p_line, reset); + logf_error("%s%s:%s %s\n", bold_color, error_type_string(p_type), normal_color, err_details); + logf_error("%s%sat: %s (%s:%i)%s\n", gray, error_type_indent(p_type), p_function, p_file, p_line, reset); for (const Ref &backtrace : p_script_backtraces) { if (!backtrace->is_empty()) { - logf_error("%s%s%s\n", gray, backtrace->format(strlen(indent)).utf8().get_data(), reset); + logf_error("%s%s%s\n", gray, backtrace->format(strlen(error_type_indent(p_type))).utf8().get_data(), reset); } } } diff --git a/drivers/unix/syslog_logger.cpp b/drivers/unix/syslog_logger.cpp index 60ade043e00..2efe4d8f208 100644 --- a/drivers/unix/syslog_logger.cpp +++ b/drivers/unix/syslog_logger.cpp @@ -49,24 +49,7 @@ void SyslogLogger::print_error(const char *p_function, const char *p_file, int p return; } - const char *err_type = "**ERROR**"; - switch (p_type) { - case ERR_ERROR: - err_type = "**ERROR**"; - break; - case ERR_WARNING: - err_type = "**WARNING**"; - break; - case ERR_SCRIPT: - err_type = "**SCRIPT ERROR**"; - break; - case ERR_SHADER: - err_type = "**SHADER ERROR**"; - break; - default: - ERR_PRINT("Unknown error type"); - break; - } + const char *err_type = error_type_string(p_type); const char *err_details; if (p_rationale && *p_rationale) { @@ -75,7 +58,7 @@ void SyslogLogger::print_error(const char *p_function, const char *p_file, int p err_details = p_code; } - syslog(p_type == ERR_WARNING ? LOG_WARNING : LOG_ERR, "%s: %s\n At: %s:%i:%s() - %s", err_type, err_details, p_file, p_line, p_function, p_code); + syslog(p_type == ERR_WARNING ? LOG_WARNING : LOG_ERR, "**%s**: %s\n At: %s:%i:%s() - %s", err_type, err_details, p_file, p_line, p_function, p_code); } SyslogLogger::~SyslogLogger() { diff --git a/editor/import/3d/collada.cpp b/editor/import/3d/collada.cpp index 0f6b0a3d7d1..9c7085c3097 100644 --- a/editor/import/3d/collada.cpp +++ b/editor/import/3d/collada.cpp @@ -2022,7 +2022,7 @@ void Collada::_remove_node(VisualScene *p_vscene, Node *p_node) { } } - ERR_PRINT("ERROR: Not found node to remove?"); + ERR_PRINT("Not found node to remove?"); } void Collada::_merge_skeletons(VisualScene *p_vscene, Node *p_node) { diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 8eceb4a0e08..5da5c9e458e 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -852,6 +852,7 @@ Error GDScript::reload(bool p_keep_state) { err = compiler.compile(&parser, this, p_keep_state); if (err) { + // TODO: Provide the script function as the first argument. _err_print_error("GDScript::reload", path.is_empty() ? "built-in" : (const char *)path.utf8().get_data(), compiler.get_error_line(), ("Compile Error: " + compiler.get_error()).utf8().get_data(), false, ERR_HANDLER_SCRIPT); if (can_run) { if (EngineDebugger::is_active()) { @@ -875,7 +876,8 @@ Error GDScript::reload(bool p_keep_state) { for (const GDScriptWarning &warning : parser.get_warnings()) { if (EngineDebugger::is_active()) { Vector si; - EngineDebugger::get_script_debugger()->send_error("", get_script_path(), warning.start_line, warning.get_name(), warning.get_message(), false, ERR_HANDLER_WARNING, si); + // TODO: Provide the script function as the first argument. + EngineDebugger::get_script_debugger()->send_error("GDScript::reload", get_script_path(), warning.start_line, warning.get_name(), warning.get_message(), false, ERR_HANDLER_WARNING, si); } } #endif diff --git a/modules/gdscript/tests/gdscript_test_runner.cpp b/modules/gdscript/tests/gdscript_test_runner.cpp index 045aa308d0d..f46f7b4dfbc 100644 --- a/modules/gdscript/tests/gdscript_test_runner.cpp +++ b/modules/gdscript/tests/gdscript_test_runner.cpp @@ -453,30 +453,11 @@ void GDScriptTest::error_handler(void *p_this, const char *p_function, const cha result->status = GDTEST_RUNTIME_ERROR; + String header = _error_handler_type_string(p_type); + // Only include the file, line, and function for script errors, // otherwise the test outputs changes based on the platform/compiler. - String header; - bool include_source_info = false; - switch (p_type) { - case ERR_HANDLER_ERROR: - header = "ERROR"; - break; - case ERR_HANDLER_WARNING: - header = "WARNING"; - break; - case ERR_HANDLER_SCRIPT: - header = "SCRIPT ERROR"; - include_source_info = true; - break; - case ERR_HANDLER_SHADER: - header = "SHADER ERROR"; - break; - default: - header = "UNKNOWN ERROR"; - break; - } - - if (include_source_info) { + if (p_type == ERR_HANDLER_SCRIPT) { header += vformat(" at %s:%d on %s()", String::utf8(p_file).trim_prefix(self->base_dir).replace_char('\\', '/'), p_line, diff --git a/platform/macos/macos_terminal_logger.mm b/platform/macos/macos_terminal_logger.mm index f7251e63490..d93aed1b288 100644 --- a/platform/macos/macos_terminal_logger.mm +++ b/platform/macos/macos_terminal_logger.mm @@ -46,47 +46,38 @@ void MacOSTerminalLogger::log_error(const char *p_function, const char *p_file, err_details = p_code; } - const char *indent = ""; + const char *bold_color; + const char *normal_color; switch (p_type) { case ERR_WARNING: - indent = " "; - os_log_error(OS_LOG_DEFAULT, - "WARNING: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - logf_error("\E[1;33mWARNING:\E[0;93m %s\n", err_details); - logf_error("\E[0;90m%sat: %s (%s:%i)\E[0m\n", indent, p_function, p_file, p_line); + bold_color = "\E[1;33m"; + normal_color = "\E[0;93m"; break; case ERR_SCRIPT: - indent = " "; - os_log_error(OS_LOG_DEFAULT, - "SCRIPT ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - logf_error("\E[1;35mSCRIPT ERROR:\E[0;95m %s\n", err_details); - logf_error("\E[0;90m%sat: %s (%s:%i)\E[0m\n", indent, p_function, p_file, p_line); + bold_color = "\E[1;35m"; + normal_color = "\E[0;95m"; break; case ERR_SHADER: - indent = " "; - os_log_error(OS_LOG_DEFAULT, - "SHADER ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - logf_error("\E[1;36mSHADER ERROR:\E[0;96m %s\n", err_details); - logf_error("\E[0;90m%sat: %s (%s:%i)\E[0m\n", indent, p_function, p_file, p_line); + bold_color = "\E[1;36m"; + normal_color = "\E[0;96m"; break; case ERR_ERROR: default: - indent = " "; - os_log_error(OS_LOG_DEFAULT, - "ERROR: %{public}s\nat: %{public}s (%{public}s:%i)", - err_details, p_function, p_file, p_line); - logf_error("\E[1;31mERROR:\E[0;91m %s\n", err_details); - logf_error("\E[0;90m%sat: %s (%s:%i)\E[0m\n", indent, p_function, p_file, p_line); + bold_color = "\E[1;31m"; + normal_color = "\E[0;91m"; break; } + os_log_error(OS_LOG_DEFAULT, + "%{public}s: %{public}s\nat: %{public}s (%{public}s:%i)", + error_type_string(p_type), err_details, p_function, p_file, p_line); + logf_error("%s%s:%s %s\n", bold_color, error_type_string(p_type), normal_color, err_details); + logf_error("\E[0;90m%sat: %s (%s:%i)\E[0m\n", error_type_indent(p_type), p_function, p_file, p_line); + for (const Ref &backtrace : p_script_backtraces) { if (!backtrace->is_empty()) { os_log_error(OS_LOG_DEFAULT, "%{public}s", backtrace->format().utf8().get_data()); - logf_error("\E[0;90m%s\E[0m\n", backtrace->format(strlen(indent)).utf8().get_data()); + logf_error("\E[0;90m%s\E[0m\n", backtrace->format(strlen(error_type_indent(p_type))).utf8().get_data()); } } } diff --git a/platform/windows/windows_terminal_logger.cpp b/platform/windows/windows_terminal_logger.cpp index 716d7344944..a28c9c4610c 100644 --- a/platform/windows/windows_terminal_logger.cpp +++ b/platform/windows/windows_terminal_logger.cpp @@ -90,9 +90,6 @@ void WindowsTerminalLogger::log_error(const char *p_function, const char *p_file uint32_t basecol = 0; switch (p_type) { - case ERR_ERROR: - basecol = FOREGROUND_RED; - break; case ERR_WARNING: basecol = FOREGROUND_RED | FOREGROUND_GREEN; break; @@ -102,30 +99,16 @@ void WindowsTerminalLogger::log_error(const char *p_function, const char *p_file case ERR_SHADER: basecol = FOREGROUND_GREEN | FOREGROUND_BLUE; break; + case ERR_ERROR: + default: + basecol = FOREGROUND_RED; + break; } basecol |= current_bg; SetConsoleTextAttribute(hCon, basecol | FOREGROUND_INTENSITY); - const char *indent = ""; - switch (p_type) { - case ERR_ERROR: - indent = " "; - logf_error("ERROR:"); - break; - case ERR_WARNING: - indent = " "; - logf_error("WARNING:"); - break; - case ERR_SCRIPT: - indent = " "; - logf_error("SCRIPT ERROR:"); - break; - case ERR_SHADER: - indent = " "; - logf_error("SHADER ERROR:"); - break; - } + logf_error("%s:", error_type_string(p_type)); SetConsoleTextAttribute(hCon, basecol); if (p_rationale && p_rationale[0]) { @@ -137,14 +120,14 @@ void WindowsTerminalLogger::log_error(const char *p_function, const char *p_file // `FOREGROUND_INTENSITY` alone results in gray text. SetConsoleTextAttribute(hCon, FOREGROUND_INTENSITY); if (p_rationale && p_rationale[0]) { - logf_error("%sat: (%s:%i)\n", indent, p_file, p_line); + logf_error("%sat: (%s:%i)\n", error_type_indent(p_type), p_file, p_line); } else { - logf_error("%sat: %s (%s:%i)\n", indent, p_function, p_file, p_line); + logf_error("%sat: %s (%s:%i)\n", error_type_indent(p_type), p_function, p_file, p_line); } for (const Ref &backtrace : p_script_backtraces) { if (!backtrace->is_empty()) { - logf_error("%s\n", backtrace->format(strlen(indent)).utf8().get_data()); + logf_error("%s\n", backtrace->format(strlen(error_type_indent(p_type))).utf8().get_data()); } }