Merge pull request #100899 from dalexeev/core-unify-display-of-error-type-prefixes

Core: Unify display of error type prefixes

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() {

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, "%s: %s\n", _error_handler_type_string(p_type), err_details);
-		fprintf(stderr, "ERROR: %s\n", 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;
 	}
 

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);

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";
+	const char *err_type = error_type_string(p_type);
-	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_details;
 	if (p_rationale && *p_rationale) {

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;

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)",
+			"%{public}s: %{public}s\nat: %{public}s (%{public}s:%i)",
-					err_details, p_function, p_file, p_line);
+			error_type_string(p_type), 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;
-	}
 
 	for (const Ref<ScriptBacktrace> &backtrace : p_script_backtraces) {
 		if (!backtrace->is_empty()) {

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 = "     ";
+			bold_color = yellow_bold;
-			logf_error("%sWARNING:%s %s\n", yellow_bold, yellow, err_details);
+			normal_color = yellow;
 			break;
 		case ERR_SCRIPT:
-			indent = "          ";
+			bold_color = magenta_bold;
-			logf_error("%sSCRIPT ERROR:%s %s\n", magenta_bold, magenta, err_details);
+			normal_color = magenta;
 			break;
 		case ERR_SHADER:
-			indent = "          ";
+			bold_color = cyan_bold;
-			logf_error("%sSHADER ERROR:%s %s\n", cyan_bold, cyan, err_details);
+			normal_color = cyan;
 			break;
 		case ERR_ERROR:
 		default:
-			indent = "   ";
+			bold_color = red_bold;
-			logf_error("%sERROR:%s %s\n", red_bold, red, err_details);
+			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<ScriptBacktrace> &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);
 		}
 	}
 }

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**";
+	const char *err_type = error_type_string(p_type);
-	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_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() {

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) {

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<ScriptLanguage::StackInfo> 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

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;
+	if (p_type == ERR_HANDLER_SCRIPT) {
-	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) {
 		header += vformat(" at %s:%d on %s()",
 				String::utf8(p_file).trim_prefix(self->base_dir).replace_char('\\', '/'),
 				p_line,

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 = "     ";
+			bold_color = "\E[1;33m";
-			os_log_error(OS_LOG_DEFAULT,
+			normal_color = "\E[0;93m";
-					"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);
 			break;
 		case ERR_SCRIPT:
-			indent = "          ";
+			bold_color = "\E[1;35m";
-			os_log_error(OS_LOG_DEFAULT,
+			normal_color = "\E[0;95m";
-					"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);
 			break;
 		case ERR_SHADER:
-			indent = "          ";
+			bold_color = "\E[1;36m";
-			os_log_error(OS_LOG_DEFAULT,
+			normal_color = "\E[0;96m";
-					"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);
 			break;
 		case ERR_ERROR:
 		default:
-			indent = "   ";
+			bold_color = "\E[1;31m";
-			os_log_error(OS_LOG_DEFAULT,
+			normal_color = "\E[0;91m";
-					"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);
 			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<ScriptBacktrace> &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());
 		}
 	}
 }

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 = "";
+		logf_error("%s:", error_type_string(p_type));
-		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;
-		}
 
 		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<ScriptBacktrace> &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());
 			}
 		}