Debugger Plugins in Godot

[simpuid]

Purpose

Godot's debugger uses Profiler and Capture to communicate with the running game. This PR exposes them to gdscript as debugger plugins.

How to use

To make a debugger plugin, we need 3 scripts and a scene (optional ).

• First script is the plugin script that is used to register the debugger plugin script and autoload script.

• Second script is the debugger plugin script. It is an extension of EditorDebuggerPlugin class. It can register/unregister message captures and send messages to the game using member functions. The script will run inside the editor.

• Third script is an autoload script . It use EngineDebugger functions to register/unregister profilers and captures. It can also use EngineDebugger.send_message to send messages to the Editor. The script will run inside the game.

Capture system

Capture system is used to relay the messages to the correct recipient (Callables in this case). A message like my_profiler:my_message will be passed to the capture registered with name my_profiler and the message would be my_message. The debugger uses some captures like performance, scene, memory, visual, servers, network etc internally, so don't use them.

Sending message

Message should be in the format [capture name]:[message]. Use the argument array to send additional data with messages.

Demo

This is the demo used for testing.

The right panel displays the mouse position relative to the running game. The left text field and button can be used to execute Expression inside the game. (It's a profiler but the usage is not limited to profiling)

Action

[Zylann]

Sorry if that sounds confused, because I am: from my last looks at this system, the only thing that nitpicks me is that's called "profilers", while I had associated that word with "measuring execution time" so far, which is a significant part of development.
This feature isn't really about measuring time at low overhead, but more general states of the game, which I had associated with "monitoring", or "capture" (perhaps even "debugging"). It's not necessarily terms mapping how things are actually called in engine tho.
How do we distinguish between that and actual time profiling then, which is much more focused and may even have specific UIs and implementation?
For example, I had a side project consisting in implementing this in Godot, to improve what I call the "existing profiler", the one where you see how long things take on CPU. So that an API allows to further instrument scripts or the engine itself. But the "profiler" term would be taken already to denote something else now...
So maybe "profiler" is a broader term for the whole thing that takes measures on a running game (so each thing need a more specific name than just "profiler"), but then that overlaps "capture" which apparently is something else, and as you said you can do things that aren't even profiling... so the word might not be well chosen?

[Zireael07]

offtop: side project is <3, @Zylann
and yep, the naming here seems a bit confused

[simpuid]

Yes "profiler" seems like a broader term because "network profiler" and "visual profiler" exists in godot. "Capture" is not about measuring general states of the game (atleast not the EngineDebugger::Capture class which is being exposed here as "capture") It is used to capture messages exchanged between the editor and the running game. "Profiler" is used to represent EngineDebugger::Profiler which is used by "network profiler" and "visual profiler" to process and send profiling data.

[Faless]

The functionalities are overall good. There are 2 things that I don't really like about the implemention right now:

• First, there is no need for all those changes to Profiler and Capture. Wrapping around them is less code and I think it's clearer (see Faless@c17bcbc which you should be able to apply on top of your branch ).
• Second, for which I don't have a clear solution, it that there is now clean way of get_profiler_scene_debugger without instancing an EditorPlugin. Maybe the root node should be a new class (EditorDebuggerModule?) which then expose that method? Should we have an exposed singleton instead? I'm still unsure about that.

About the naming issues raised by @Zylann should we call them DebuggerModule instead of Profiler. Does it make more sense?

[simpuid]

@Faless I added the EditorDebuggerModule implementation.
Notable changes in this approach:

• No need to expose EditorDebuggerNode and ScriptEditorDebugger to gdscript.
• EditorDebuggerModule exposes the function for addition/removal of captures and message sending instead of ScriptEditorDebugger. I would add additional helpful functions later (if this approach looks good)
• A PackedScene was required to register the GUI but now there is no need. A script extending EditorDebuggerModule is required instead. Users can instantiate the required nodes as its child to add UI (using a packed scene or make them through script).
• EditorDebuggerNode was only used to register/unregister GUI node which is now exposed through EditorPlugin. This is similar to addition/removal of import plugin, export plugin, inspector plugin, spatial gizmos plugin, etc.

[Faless]

See comments below, overall looking much better! Great job.

[simpuid]

@Faless What are your thoughts about renaming EditorDebuggerModule to EditorDebuggerPlugin?
I think it would match with the classes of similar functionality like EditorInspectorPlugin,EditorExportPlugin, EditorImportPlugin, EditorInspectorPlugin and EditorSpatialGizmoPlugin.

Should I rename register/unregister functions to add/remove to make them similar to other in-built functions?

Planned Functions/Signals for EditorDebuggerModule or EditorDebuggerPlugin:

• Add, remove and has message capture
• Sending message
• Is breaked (from ScriptEditorDebugger)
• Is debuggable (from ScriptEditorDebugger)
• Is session active (from ScriptEditorDebugger)
• A signal that gets called whenever the game is started/stopped from the respective debugger (Unsure about the name, maybe session_toggle?)
• Should I expose a function to stop the execution of the running game? (ScriptEditorDebugger::stop)

Are there any other functions/signals that would be useful?

User can queue_free the node of EditorDebuggerModule/EditorDebuggerPlugin which would cause an error message when the plugin is removed/unregistered. Is there any way to restrict this? Should I care about this case?

[Faless]

EditorDebuggerPlugin sounds good.

Should I rename register/unregister functions to add/remove to make them similar to other in-built functions?

Not sure, does this have the same behaviour as other add/remove? Register is used in other places (like ScriptEditor), but no sure. I don't have a strong opinion on this.

* A signal that gets called whenever the game is started/stopped from the respective debugger (Unsure about the name, maybe `session_toggle`?)

Are there any other functions/signals that would be useful?

Check out the signals in ScriptEditorDebugger.
Probably:

• breaked
• stopped
• Add a new one: started.

* Should I expose a function to stop the execution of the running game? (`ScriptEditorDebugger::stop`)

That's probably not needed.

User can queue_free the node of EditorDebuggerModule/EditorDebuggerPlugin which would cause an error message when the plugin is removed/unregistered. Is there any way to restrict this?

No.

Should I care about this case?

Maybe we can add some logic to the destructor, or the _notification for EXIT_TREE to remove it from the modules.

[simpuid]

@Faless The implementation is complete. Things added/changed:

• All the above mentioned signals of ScriptEditorDebugger as signal and virtual functions. Signal "breaked" of ScriptEditorDebugger was performing 2 function so I broke it into two parts in EditorDebuggerPlugin, "breaked" and "continued".
• We can't extend a virtual class (changed recently in master because earlier it was working) so now EditorDebuggerPlugin is a normal class, It can be constructed using "new" but that instance would be useless.
• Since we are using Script for editor side of communication, so addition/removal of plugin uses Ref<Script> as index instead of StringName hence the argument count is reduced.
• Plugin nodes use the attached script as argument to call ScriptEditorDebugger::remove_debugger_plugin at exit_tree and destructor.

[Faless]

This looks great.
I don't think we need to call breaked/continued/etc in the script (i.e. if (get_script_instance() && get_script_instance()->has_method("breaked")) [...] ). The signals should be enough.
Would be nice to add some base documentation for EditorDebuggerPlugin (see https://docs.godotengine.org/en/stable/community/contributing/updating_the_class_reference.html#updating-the-documentation-template for generating, but please only add the new EditorDebuggerPlugin.xml file, not all other doc changes).

[simpuid]

@Faless I added the documentation for all the new classes/functions implemented in this PR (not limited to EditorDebuggerPlugin.xml but EditorPlugin.xml and EngineDebugger.xml too as they contain new functions, I will revert it if required). And I removed those virtual functions.

[Faless]

Almost there 👍 , please see my comments about documentation and the mutex.

[Faless]

Great job 🏅

[akien-mga]

Looks great, excellent work!

[akien-mga]

Thanks!