summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRaul Santos <raulsntos@gmail.com>2023-07-09 23:25:20 +0200
committerRaul Santos <raulsntos@gmail.com>2023-07-11 11:19:11 +0200
commit13ab2b6f4f61dbfb4f90c6602f126c247d4c38c5 (patch)
treeea866407ff98adfdf01631ffff71403f7b759a14
parent83cc5d4914a6bff76069ac19191192337e4df3de (diff)
downloadredot-engine-13ab2b6f4f61dbfb4f90c6602f126c247d4c38c5.tar.gz
C#: Improve `GD.PushError` and `GD.PushWarning`
- Use the name, file path and line number of the caller that invokes `GD.PushError` and `GD.PushWarning` instead of the location in the C++ `runtime_interop.cpp` file. - Improvements to getting the C# stack trace. - Use C# type keywords for built-in types in method declarations. - Remove extra space before each parameter in method declarations. - Skip one more frame to avoid `NativeInterop.NativeFuncs`. - Skip methods annotated with the `[StackTraceHidden]` attribute. - Improvements to `ScriptEditorDebugger` when source is in project. - Avoid overriding error metadata when the source is inside the project file. - Use the source function in the title when the source is inside the project file. Users that use these methods would expect the reported location printed by these methods to correspond to a location in their project source files. Specifically, they'd expect to see the file path and line number at which they call these methods, and not the location of the C++ code (which is always the same). Now, these methods are a lot more useful since users can know which line in their source code printed the error/warning.
-rw-r--r--editor/debugger/script_editor_debugger.cpp12
-rw-r--r--modules/mono/glue/GodotSharp/GodotSharp/Core/DebuggingUtils.cs71
-rw-r--r--modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs26
-rw-r--r--modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/ExceptionUtils.cs2
-rw-r--r--modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs9
-rw-r--r--modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/NativeFuncs.cs6
-rw-r--r--modules/mono/glue/runtime_interop.cpp21
7 files changed, 117 insertions, 30 deletions
diff --git a/editor/debugger/script_editor_debugger.cpp b/editor/debugger/script_editor_debugger.cpp
index 8985387043..2e41cd709d 100644
--- a/editor/debugger/script_editor_debugger.cpp
+++ b/editor/debugger/script_editor_debugger.cpp
@@ -526,8 +526,11 @@ void ScriptEditorDebugger::_parse_message(const String &p_msg, const Array &p_da
error->set_custom_color(1, color);
String error_title;
- if (oe.callstack.size() > 0) {
- // If available, use the script's stack in the error title.
+ if (!oe.source_func.is_empty() && source_is_project_file) {
+ // If source function is inside the project file.
+ error_title += oe.source_func + ": ";
+ } else if (oe.callstack.size() > 0) {
+ // Otherwise, if available, use the script's stack in the error title.
error_title = _format_frame_text(&oe.callstack[0]) + ": ";
} else if (!oe.source_func.is_empty()) {
// Otherwise try to use the C++ source function.
@@ -602,7 +605,10 @@ void ScriptEditorDebugger::_parse_message(const String &p_msg, const Array &p_da
if (i == 0) {
stack_trace->set_text(0, "<" + TTR("Stack Trace") + ">");
stack_trace->set_text_alignment(0, HORIZONTAL_ALIGNMENT_LEFT);
- error->set_metadata(0, meta);
+ if (!source_is_project_file) {
+ // Only override metadata if the source is not inside the project.
+ error->set_metadata(0, meta);
+ }
tooltip += TTR("Stack Trace:") + "\n";
}
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 2d8067d300..bb510cee7e 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 43e7c7eb9a..23b9228a93 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/InteropStructs.cs
@@ -1133,4 +1133,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,
};