godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.17k stars 98 forks source link

Make script paths in Editor Output error messages clickable #1628

Open me2beats opened 4 years ago

me2beats commented 4 years ago

Describe the project you are working on: Godot editor plugins (gdscript)

Describe the problem or limitation you are having in your project: clickable

I often get errors in Output, which point to scripts with errors, but I need to manually go to these scripts (for example in this case I need to manually find x.gd and script list or filesystem and scroll to line 2), which is not convenient.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: Making scripts paths clickable in error messages in Output would be more convenient than open the script manually.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: One could click on a script path in Output and then the script would be opened and scrolled to line specified in the error message. This could be implemented with adding clickable metas in case MSG_TYPE_ERROR: { block of EditorLog.cpp, maybe in MSG_TYPE_WARNING as well

If this enhancement will not be used often, can it be worked around with a few lines of script?: No it can't

Is there a reason why this should be core and not an add-on in the asset library?: I tried to make a gdscript plugin that does this. But this always looked ugly since there are some missing API methods for that. There is no get_editor_log() method There are no signals to find out if an error is printed. etc

Calinou commented 4 years ago

The Output panel uses RichTextLabel, which makes it possible to implement this by using [url] tags and defining custom meta actions. See editor/editor_help.cpp for an example of this: signal connection, method definition

me2beats commented 4 years ago

But EditorLog doesn't use metas It just uses push_color() and add_image() and I couldn't find a way to know for example which line is an error and which one is a warning. I just have a plain text.

Or maybe I simply don't know how to work with RichTextLabel in this case

Calinou commented 4 years ago

@me2beats You can push URL tags manually using the push_meta() method. It's just another way to RichTextLabel syntax. BBCode isn't the only way to do it :slightly_smiling_face:

me2beats commented 4 years ago

@Calinou Do I understand it right that

from this it turns out that I can not just add meta to all places in the Output text where the script links should be, and still keep the same "info" about each line (color, etc).

Calinou commented 4 years ago

@me2beats I was referring to implementing this as a built-in feature, not an add-on. In this case, you can call push_meta() anywhere you need :wink:

Calinou commented 2 years ago

Note that this was worked on in https://github.com/godotengine/godot/pull/57896, but it needs to be salvaged and rebased on the latest master branch.

awardell commented 2 years ago

I've brought https://github.com/godotengine/godot/pull/57896 up to date with Master, and the functionality is working, but it is not yet ready for a new pull request. There are some issues:

1.

Somehow the resource parser and the rich text parser are fighting with each other. Rich text logs with [url] tags will correctly link to script files, and logs with visible resource addresses will correctly link to script files, but if the Output panel contains BOTH rich text logs and regular logs with visible resource addresses, the visible resource addresses will not create hyperlinks. For example, if my gdscript contains:

print("res://Player.gd") #0
print("res://Player.gd:24") #1
print_rich("res://Player.gd") #2
print_rich("[url=res://Player.gd]Click me![/url]") #3
print_rich("[url]res://Player.gd[/url]") #4
print_debug("This is print_debug") #5
print_stack() #6

then # 0, 1, and 2 will not correctly hyperlink, but the rest will. If all of the print_rich lines are removed, then # 0 and 1 will correctly hyperlink. It appears impossible for # 2 to hyperlink, but perhaps that is desired behavior.

2.

The functionality as written requires an include of "editor/plugins/script_editor_plugin.h" from within "editor/editor_log.cpp". I'm new to Godot, but I presume that dependencies on plugins from within core features need to be avoided. Am I correct?

3.

I am wary of the performance impact of running regex on every single line that is logged to the Output. That seems as though it could be unnecessarily heavy in situations with lots of logs. What is a good way to measure that performance impact? If it does end up being an issue, we could relegate resource-path detection to only print_rich, print_debug, print_stack, and any others which naturally may contain resource paths.

Summary

The updated version can be found at https://github.com/awardell/godot_contributing/commit/cc197f142c43dc535fca70f33b85be23a238873f

If @jordigcs is interested in taking this up again, then maybe the original pull request can be reopened and modified? Or perhaps there should be a new one. I don't think I'm going to dive into solving the issue right away myself if someone else would like to take it. Otherwise I may get to it in the near future.

Calinou commented 2 years ago

What is a good way to measure that performance impact?

Using C++ profilers or the microbenchmarking snippet.