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.
This commit is contained in:
Raul Santos 2023-07-09 23:25:20 +02:00
parent 83cc5d4914
commit 13ab2b6f4f
No known key found for this signature in database
GPG key ID: B532473AE3A803E4
7 changed files with 117 additions and 30 deletions

View file

@ -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";
}

View file

@ -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('.');

View file

@ -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>

View file

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

View file

@ -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,
}
}

View file

@ -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

View file

@ -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,
};