C#: Improve `GD.PushError` and `GD.PushWarning`
- Use the name, file path and line number of the caller that invokes
GD.PushErrorandGD.PushWarninginstead of the location in the C++runtime_interop.cppfile.- See https://github.com/godotengine/godot/pull/71943#issuecomment-1415809223.
- ~Deprecate
GD.PushErrorandGD.PushWarningoverloads that take aparamsparameter, since those can't be used with [Caller] attributes and the reported error location will be theGD.csfile.~ - ~Remove
GD.PushErrorandGD.PushWarningoverloads that take a single string parameter. This breaks compatibility but otherwise overload resolution will prefer them over the new overloads, which defeats the purpose of this PR.~- ~Interestingly, I'm the one that added those overloads as performance optimization in https://github.com/godotengine/godot/pull/71946.~
- ~This breaks binary compatibility but not source compatibility, meaning that it only breaks for users that are calling these methods from a library that was built for 4.0 - 4.1. Users calling these methods directly in their game code will be unaffected.~
- 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.
- In
ScriptEditorDebuggeravoid overriding error metadata when the source is inside the project file.- Fixes https://github.com/godotengine/godot/issues/72662.
- May fix https://github.com/godotengine/godot/issues/76415.
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.
| 4.1.stable | This PR |
|---|---|
Removed the breaks compat label, since I changed this to avoid breaking compatibility.
If the a stack from is from source generated code attempting to open its source will fail because the source doesn't exist
There is a check to avoid the source function if it's not part of the project. It only checks the path, is that check not enough?
Can we pretend that we don't know about the source file if it doesn't exist and avoid these errors?
That sounds reasonable, I'm a little worried about how this would affect the script debugger since I think it assumes the entire stack trace contains methods defined in the user project. Maybe we should filter everything that is not part of the user project from the stack trace, but that feels wrong since it may be useful information.
There is a check to avoid the source function if it's not part of the project. It only checks the path, is that check not enough?
What check do you mean?
https://github.com/godotengine/godot/blob/202e4b2c1e7f8b25738b93d0e4d5066453d3edf3/core/io/resource_loader.cpp#L276-L279
Could only trigger if the previous errors didn't happen.
I would add a single error to this function: (tho I suppose we could do that in a different PR as well) https://github.com/godotengine/godot/blob/202e4b2c1e7f8b25738b93d0e4d5066453d3edf3/editor/debugger/editor_debugger_node.cpp#L142-L145
What check do you mean?
This one:
https://github.com/godotengine/godot/blob/13ab2b6f4f61dbfb4f90c6602f126c247d4c38c5/editor/debugger/script_editor_debugger.cpp#L495
https://github.com/godotengine/godot/blob/13ab2b6f4f61dbfb4f90c6602f126c247d4c38c5/editor/debugger/script_editor_debugger.cpp#L608-L611
The metadata is then used to determine which file to open:
https://github.com/godotengine/godot/blob/13ab2b6f4f61dbfb4f90c6602f126c247d4c38c5/editor/debugger/script_editor_debugger.cpp#L1486-L1491
I would add a single error to this function: (tho I suppose we could do that in a different PR as well)
I'd prefer to do it in a different PR.
Thanks!