Merge pull request #79280 from raulsntos/dotnet/better-push-error

C#: Improve `GD.PushError` and `GD.PushWarning`

6 files changed, 108 insertions, 27 deletions

diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/DebuggingUtils.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/DebuggingUtils.cs
index c4161d2ded..57b292793a 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/DebuggingUtils.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/DebuggingUtils.cs
@@ -14,14 +14,46 @@ namespace Godot
     {
         private static void AppendTypeName(this StringBuilder sb, Type type)
         {
-            if (type.IsPrimitive)
-                sb.Append(type.Name);
-            else if (type == typeof(void))
+            // Use the C# type keyword for built-in types.
+            // https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/built-in-types
+            if (type == typeof(void))
                 sb.Append("void");
+            else if (type == typeof(bool))
+                sb.Append("bool");
+            else if (type == typeof(byte))
+                sb.Append("byte");
+            else if (type == typeof(sbyte))
+                sb.Append("sbyte");
+            else if (type == typeof(char))
+                sb.Append("char");
+            else if (type == typeof(decimal))
+                sb.Append("decimal");
+            else if (type == typeof(double))
+                sb.Append("double");
+            else if (type == typeof(float))
+                sb.Append("float");
+            else if (type == typeof(int))
+                sb.Append("int");
+            else if (type == typeof(uint))
+                sb.Append("uint");
+            else if (type == typeof(nint))
+                sb.Append("nint");
+            else if (type == typeof(nuint))
+                sb.Append("nuint");
+            else if (type == typeof(long))
+                sb.Append("long");
+            else if (type == typeof(ulong))
+                sb.Append("ulong");
+            else if (type == typeof(short))
+                sb.Append("short");
+            else if (type == typeof(ushort))
+                sb.Append("ushort");
+            else if (type == typeof(object))
+                sb.Append("object");
+            else if (type == typeof(string))
+                sb.Append("string");
             else
                 sb.Append(type);
-
-            sb.Append(' ');
         }
 
         internal static void InstallTraceListener()
@@ -70,13 +102,26 @@ namespace Godot
             }
         }
 
+        internal static unsafe StackFrame? GetCurrentStackFrame(int skipFrames = 0)
+        {
+            // We skip 2 frames:
+            // The first skipped frame is the current method.
+            // The second skipped frame is a method in NativeInterop.NativeFuncs.
+            var stackTrace = new StackTrace(skipFrames: 2 + skipFrames, fNeedFileInfo: true);
+            return stackTrace.GetFrame(0);
+        }
+
         [UnmanagedCallersOnly]
         internal static unsafe void GetCurrentStackInfo(void* destVector)
         {
             try
             {
                 var vector = (godot_stack_info_vector*)destVector;
-                var stackTrace = new StackTrace(skipFrames: 1, fNeedFileInfo: true);
+
+                // We skip 2 frames:
+                // The first skipped frame is the current method.
+                // The second skipped frame is a method in NativeInterop.NativeFuncs.
+                var stackTrace = new StackTrace(skipFrames: 2, fNeedFileInfo: true);
                 int frameCount = stackTrace.FrameCount;
 
                 if (frameCount == 0)
@@ -87,6 +132,14 @@ namespace Godot
                 int i = 0;
                 foreach (StackFrame frame in stackTrace.GetFrames())
                 {
+                    var method = frame.GetMethod();
+
+                    if (method is MethodInfo methodInfo && methodInfo.IsDefined(typeof(StackTraceHiddenAttribute)))
+                    {
+                        // Skip methods marked hidden from the stack trace.
+                        continue;
+                    }
+
                     string? fileName = frame.GetFileName();
                     int fileLineNumber = frame.GetFileLineNumber();
 
@@ -102,6 +155,9 @@ namespace Godot
 
                     i++;
                 }
+
+                // Resize the vector again in case we skipped some frames.
+                vector->Resize(i);
             }
             catch (Exception e)
             {
@@ -122,7 +178,10 @@ namespace Godot
             var sb = new StringBuilder();
 
             if (methodBase is MethodInfo methodInfo)
+            {
                 sb.AppendTypeName(methodInfo.ReturnType);
+                sb.Append(' ');
+            }
 
             sb.Append(methodBase.DeclaringType?.FullName ?? "<unknown>");
             sb.Append('.');
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs
index 9425b7424c..33ebb8171e 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Text;
 using Godot.NativeInterop;
 
@@ -334,6 +335,21 @@ namespace Godot
             NativeFuncs.godotsharp_printt(godotStr);
         }
 
+        [StackTraceHidden]
+        private static void ErrPrintError(string message, godot_error_handler_type type = godot_error_handler_type.ERR_HANDLER_ERROR)
+        {
+            // Skip 1 frame to avoid current method.
+            var stackFrame = DebuggingUtils.GetCurrentStackFrame(skipFrames: 1);
+            string callerFilePath = ProjectSettings.LocalizePath(stackFrame.GetFileName());
+            DebuggingUtils.GetStackFrameMethodDecl(stackFrame, out string callerName);
+            int callerLineNumber = stackFrame.GetFileLineNumber();
+
+            using godot_string messageStr = Marshaling.ConvertStringToNative(message);
+            using godot_string callerNameStr = Marshaling.ConvertStringToNative(callerName);
+            using godot_string callerFilePathStr = Marshaling.ConvertStringToNative(callerFilePath);
+            NativeFuncs.godotsharp_err_print_error(callerNameStr, callerFilePathStr, callerLineNumber, messageStr, p_type: type);
+        }
+
         /// <summary>
         /// Pushes an error message to Godot's built-in debugger and to the OS terminal.
         ///
@@ -347,8 +363,7 @@ namespace Godot
         /// <param name="message">Error message.</param>
         public static void PushError(string message)
         {
-            using var godotStr = Marshaling.ConvertStringToNative(message);
-            NativeFuncs.godotsharp_pusherror(godotStr);
+            ErrPrintError(message);
         }
 
         /// <summary>
@@ -364,7 +379,7 @@ namespace Godot
         /// <param name="what">Arguments that form the error message.</param>
         public static void PushError(params object[] what)
         {
-            PushError(AppendPrintParams(what));
+            ErrPrintError(AppendPrintParams(what));
         }
 
         /// <summary>
@@ -378,8 +393,7 @@ namespace Godot
         /// <param name="message">Warning message.</param>
         public static void PushWarning(string message)
         {
-            using var godotStr = Marshaling.ConvertStringToNative(message);
-            NativeFuncs.godotsharp_pushwarning(godotStr);
+            ErrPrintError(message, type: godot_error_handler_type.ERR_HANDLER_WARNING);
         }
 
         /// <summary>
@@ -393,7 +407,7 @@ namespace Godot
         /// <param name="what">Arguments that form the warning message.</param>
         public static void PushWarning(params object[] what)
         {
-            PushWarning(AppendPrintParams(what));
+            ErrPrintError(AppendPrintParams(what), type: godot_error_handler_type.ERR_HANDLER_WARNING);
         }
 
         /// <summary>
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs
index ba2c232580..a656c5de90 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs
@@ -95,7 +95,7 @@ namespace Godot.NativeInterop
                 }
 
                 NativeFuncs.godotsharp_internal_script_debugger_send_error(nFunc, nFile, line,
-                    nErrorMsg, nExcMsg, p_warning: godot_bool.False, stackInfoVector);
+                    nErrorMsg, nExcMsg, godot_error_handler_type.ERR_HANDLER_ERROR, stackInfoVector);
             }
         }
 
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs
index 1dddc82e85..d5d9404ed1 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs
@@ -1134,4 +1134,13 @@ namespace Godot.NativeInterop
             get => _ptr != null ? *((int*)_ptr - 1) : 0;
         }
     }
+
+    [SuppressMessage("ReSharper", "InconsistentNaming")]
+    public enum godot_error_handler_type
+    {
+        ERR_HANDLER_ERROR = 0,
+        ERR_HANDLER_WARNING,
+        ERR_HANDLER_SCRIPT,
+        ERR_HANDLER_SHADER,
+    }
 }
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs
index 3ec3d1e530..d42ee15657 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs
@@ -58,7 +58,7 @@ namespace Godot.NativeInterop
 
         internal static partial void godotsharp_internal_script_debugger_send_error(in godot_string p_func,
             in godot_string p_file, int p_line, in godot_string p_err, in godot_string p_descr,
-            godot_bool p_warning, in DebuggingUtils.godot_stack_info_vector p_stack_info_vector);
+            godot_error_handler_type p_type, in DebuggingUtils.godot_stack_info_vector p_stack_info_vector);
 
         internal static partial godot_bool godotsharp_internal_script_debugger_is_active();
 
@@ -540,9 +540,7 @@ namespace Godot.NativeInterop
 
         internal static partial void godotsharp_var_to_str(in godot_variant p_var, out godot_string r_ret);
 
-        internal static partial void godotsharp_pusherror(in godot_string p_str);
-
-        internal static partial void godotsharp_pushwarning(in godot_string p_str);
+        internal static partial void godotsharp_err_print_error(in godot_string p_function, in godot_string p_file, int p_line, in godot_string p_error, in godot_string p_message = default, godot_bool p_editor_notify = godot_bool.False, godot_error_handler_type p_type = godot_error_handler_type.ERR_HANDLER_ERROR);
 
         // Object
 
diff --git a/modules/mono/glue/runtime_interop.cpp b/modules/mono/glue/runtime_interop.cpp
index ee4de4e9f5..24a9d4030a 100644
--- a/modules/mono/glue/runtime_interop.cpp
+++ b/modules/mono/glue/runtime_interop.cpp
@@ -92,10 +92,10 @@ void godotsharp_stack_info_vector_destroy(
 
 void godotsharp_internal_script_debugger_send_error(const String *p_func,
 		const String *p_file, int32_t p_line, const String *p_err, const String *p_descr,
-		bool p_warning, const Vector<ScriptLanguage::StackInfo> *p_stack_info_vector) {
+		ErrorHandlerType p_type, const Vector<ScriptLanguage::StackInfo> *p_stack_info_vector) {
 	const String file = ProjectSettings::get_singleton()->localize_path(p_file->simplify_path());
 	EngineDebugger::get_script_debugger()->send_error(*p_func, file, p_line, *p_err, *p_descr,
-			true, p_warning ? ERR_HANDLER_WARNING : ERR_HANDLER_ERROR, *p_stack_info_vector);
+			true, p_type, *p_stack_info_vector);
 }
 
 bool godotsharp_internal_script_debugger_is_active() {
@@ -1320,12 +1320,14 @@ void godotsharp_printraw(const godot_string *p_what) {
 	OS::get_singleton()->print("%s", reinterpret_cast<const String *>(p_what)->utf8().get_data());
 }
 
-void godotsharp_pusherror(const godot_string *p_str) {
-	ERR_PRINT(*reinterpret_cast<const String *>(p_str));
-}
-
-void godotsharp_pushwarning(const godot_string *p_str) {
-	WARN_PRINT(*reinterpret_cast<const String *>(p_str));
+void godotsharp_err_print_error(const godot_string *p_function, const godot_string *p_file, int32_t p_line, const godot_string *p_error, const godot_string *p_message, bool p_editor_notify, ErrorHandlerType p_type) {
+	_err_print_error(
+			reinterpret_cast<const String *>(p_function)->utf8().get_data(),
+			reinterpret_cast<const String *>(p_file)->utf8().get_data(),
+			p_line,
+			reinterpret_cast<const String *>(p_error)->utf8().get_data(),
+			reinterpret_cast<const String *>(p_message)->utf8().get_data(),
+			p_editor_notify, p_type);
 }
 
 void godotsharp_var_to_str(const godot_variant *p_var, godot_string *r_ret) {
@@ -1611,8 +1613,7 @@ static const void *unmanaged_callbacks[]{
 	(void *)godotsharp_str_to_var,
 	(void *)godotsharp_var_to_bytes,
 	(void *)godotsharp_var_to_str,
-	(void *)godotsharp_pusherror,
-	(void *)godotsharp_pushwarning,
+	(void *)godotsharp_err_print_error,
 	(void *)godotsharp_object_to_string,
 };