Godot: Confusing _ (underscore) for private, virtual, signals, callbacks, lifecycle functions

[toger5]

The current function naming api is confusing in it's _ use.
In the engine source (C++) it is used for the private functions. And in godot it is obviously used for lifecycle callbacks like _ready(d) or _input(d) but it is also used when creating signal connections via the editor _on_button_pressed().

But for builtin api's this is not always the case. There are a lot of virtual functions which can be overwritten by gdscript and are treated like callbacks which don't have a _ (underscore).

Example: editorImportPlugin
all the get_... methods which are basically callbacks (virtual functions) don't use the _ syntax.

[toger5]

Also mentioned here #14466

[vnen]

When you consider that nothing is really private in GDScript, the underscore is quite pointless. It can be an internal convention for users to make in their method names, but engine provided methods don't really need to be marked this way (and considering that they are called by the engine itself, they should be considered public anyway).

As said, it's not used consistently, so much that many contributors got it wrong, which made it more inconsistent. If it were up to me I would remove the underscore prefix from everything.

For the hidden methods and properties I think it's okay though. They don't show up in docs and aren't supposed to be called from the scripts anyway.

[LikeLakers2]

The way I always understood it is that any functions that could be considered responding to a signal/event (ready, input, physics process, on button pressed, etc.) should be prefixed with a _, so as to signify that they're not supposed to be called directly by other game code; and anything else (whether it's getting data or performing an action) should not. Personally, it's not confusing to me, but I can understand how someone coming from another language might be confused.

I am curious if you have any suggestions on how to better categorize the functions, though.

[toger5]

@LikeLakers2 yea when using just the engine I got the same impression than u.
Just recently I found inconsistencies with some virtual functions like get_name() from the EditorPlugin class. (But there are a lot of others too).
And when dealing with engine source code it has no coherent structure with gdscript (which is fine. the users should not care about the source code naming)

My proposal would be to rename all virtual, (auto-called function) to _function_name()
so that it becames a consistent indicator that those functions are called from inside the engine event handlers/value getters... and you don't have to be confessed where the function is used. or why it never gets called, but only declared.

[henriiquecampos]

isn't this a python-ish convention like PEP8 or something?? When you name a function which shouldn't be called outside a class, i.e. private, you add _ to its beginning.

I don't know how it is on the source code, but in the API it seems consistent to me. I mean, if you are connecting a notification to a method, this method should be called by the notification only, shouldn't it? I mean, you can get a pretty hard time debugging a method that is called both by a built-in notification (signal) and by a custom method.

And for lifecycle functions it makes even more sense...is not recommended at all to call _process or _ready outside of the class itself, or _draw. :thinking: So it seems that these would be equivalent to privatefunctions, wouldn't they?

Since GDScript, as Python, doesn't have public and private scopes, this is a really important convention. The problem, yes, can be it consistency, but it doesn't seem to be inconsistent to me, as an user.

As for the core, Since C++ has such scopes, I think they are only using this convention for user exposed methods, is this the case?

PS: just pointing out that is not something exclusive for virtual functions or anything, it is a convention for "telling" that a method is meant to be considered private so it makes sense to not have underscores on virtual methods if they are meant to be use outside the EditorImportPlugin class

[vnen]

Well, you're not supposed to call EditorImportPlugin::import(), but it does not start with an underscore. If the engine calling it mean they're not private, then the same applied to the lifecycle callbacks. OTOH,
VisualScriptCustomNode has all virtual methods prefixed.

For _ready(). _process(), etc. I believe it's not recommended to call them at all, not even inside the class. They are more like some kind of magic methods than private ones.

In any case, there's no consistency at all. If the underscore convention is really useful, there should be a clear documented rule to apply it, otherwise each one thinks it's something different and it loses the meaning.

[vnen]

If the criteria is that virtual functions start with underscore, here's a list of violations:

doc/classes/EditorInspectorPlugin.xml
42:     <method name="can_handle" qualifiers="virtual">
50:     <method name="parse_begin" qualifiers="virtual">
58:     <method name="parse_category" qualifiers="virtual">
68:     <method name="parse_end" qualifiers="virtual">
74:     <method name="parse_property" qualifiers="virtual">

doc/classes/EditorScenePostImport.xml
12:     <method name="post_import" qualifiers="virtual">

doc/classes/EditorSpatialGizmo.xml
89:     <method name="commit_handle" qualifiers="virtual">
103:        <method name="get_handle_name" qualifiers="virtual">
113:        <method name="get_handle_value" qualifiers="virtual">
122:        <method name="redraw" qualifiers="virtual">
129:        <method name="set_handle" qualifiers="virtual">

doc/classes/EditorPlugin.xml
138:        <method name="apply_changes" qualifiers="virtual">
146:        <method name="build" qualifiers="virtual">
152:        <method name="clear" qualifiers="virtual">
159:        <method name="create_spatial_gizmo" qualifiers="virtual">
168:        <method name="edit" qualifiers="virtual">
177:        <method name="forward_canvas_gui_input" qualifiers="virtual">
185:        <method name="forward_draw_over_viewport" qualifiers="virtual">
193:        <method name="forward_force_draw_over_viewport" qualifiers="virtual">
201:        <method name="forward_spatial_gui_input" qualifiers="virtual">
213:        <method name="get_breakpoints" qualifiers="virtual">
226:        <method name="get_plugin_icon" qualifiers="virtual">
232:        <method name="get_plugin_name" qualifiers="virtual">
238:        <method name="get_state" qualifiers="virtual">
252:        <method name="get_window_layout" qualifiers="virtual">
261:        <method name="handles" qualifiers="virtual">
270:        <method name="has_main_screen" qualifiers="virtual">
291:        <method name="make_visible" qualifiers="virtual">
396:        <method name="save_external_data" qualifiers="virtual">
416:        <method name="set_state" qualifiers="virtual">
425:        <method name="set_window_layout" qualifiers="virtual">

doc/classes/Control.xml
112:        <method name="can_drop_data" qualifiers="virtual">
132:        <method name="drop_data" qualifiers="virtual">
205:        <method name="get_drag_data" qualifiers="virtual">
408:        <method name="has_point" qualifiers="virtual">

doc/classes/EditorProperty.xml
24:     <method name="update_property" qualifiers="virtual">

doc/classes/EditorResourcePreviewGenerator.xml
14:     <method name="generate" qualifiers="virtual">
25:     <method name="generate_from_path" qualifiers="virtual">
36:     <method name="handles" qualifiers="virtual">

doc/classes/EditorImportPlugin.xml
58:     <method name="get_import_options" qualifiers="virtual">
67:     <method name="get_import_order" qualifiers="virtual">
74:     <method name="get_importer_name" qualifiers="virtual">
81:     <method name="get_option_visibility" qualifiers="virtual">
91:     <method name="get_preset_count" qualifiers="virtual">
98:     <method name="get_preset_name" qualifiers="virtual">
107:        <method name="get_priority" qualifiers="virtual">
114:        <method name="get_recognized_extensions" qualifiers="virtual">
121:        <method name="get_resource_type" qualifiers="virtual">
128:        <method name="get_save_extension" qualifiers="virtual">
135:        <method name="get_visible_name" qualifiers="virtual">
142:        <method name="import" qualifiers="virtual">

---

And here's the ones that adheres to it:

doc/classes/RigidBody2D.xml
17:     <method name="_integrate_forces" qualifiers="virtual">

doc/classes/Resource.xml
14:     <method name="_setup_local_to_scene" qualifiers="virtual">

doc/classes/EditorSceneImporter.xml
12:     <method name="_get_extensions" qualifiers="virtual">
18:     <method name="_get_import_flags" qualifiers="virtual">
24:     <method name="_import_animation" qualifiers="virtual">
36:     <method name="_import_scene" qualifiers="virtual">

doc/classes/CanvasItem.xml
20:     <method name="_draw" qualifiers="virtual">

doc/classes/CollisionObject.xml
14:     <method name="_input_event" qualifiers="virtual">

doc/classes/BaseButton.xml
14:     <method name="_pressed" qualifiers="virtual">
21:     <method name="_toggled" qualifiers="virtual">

doc/classes/CollisionObject2D.xml
14:     <method name="_input_event" qualifiers="virtual">

doc/classes/Object.xml
18:     <method name="_get" qualifiers="virtual">
27:     <method name="_get_property_list" qualifiers="virtual">
34:     <method name="_init" qualifiers="virtual">
41:     <method name="_notification" qualifiers="virtual">
50:     <method name="_set" qualifiers="virtual">

doc/classes/EditorResourceConversionPlugin.xml
12:     <method name="_convert" qualifiers="virtual">
20:     <method name="_converts_to" qualifiers="virtual">

doc/classes/AStar.xml
15:     <method name="_compute_cost" qualifiers="virtual">
26:     <method name="_estimate_cost" qualifiers="virtual">

doc/classes/Node.xml
25:     <method name="_enter_tree" qualifiers="virtual">
33:     <method name="_exit_tree" qualifiers="virtual">
41:     <method name="_input" qualifiers="virtual">
53:     <method name="_physics_process" qualifiers="virtual">
64:     <method name="_process" qualifiers="virtual">
75:     <method name="_ready" qualifiers="virtual">
84:     <method name="_unhandled_input" qualifiers="virtual">
96:     <method name="_unhandled_key_input" qualifiers="virtual">

doc/classes/EditorExportPlugin.xml
12:     <method name="_export_begin" qualifiers="virtual">
26:     <method name="_export_file" qualifiers="virtual">

doc/classes/Control.xml
22:     <method name="_get_minimum_size" qualifiers="virtual">
29:     <method name="_gui_input" qualifiers="virtual">

doc/classes/EditorScript.xml
23:     <method name="_run" qualifiers="virtual">

doc/classes/MainLoop.xml
14:     <method name="_drop_files" qualifiers="virtual">
24:     <method name="_finalize" qualifiers="virtual">
31:     <method name="_idle" qualifiers="virtual">
40:     <method name="_initialize" qualifiers="virtual">
47:     <method name="_input_event" qualifiers="virtual">
55:     <method name="_input_text" qualifiers="virtual">
63:     <method name="_iteration" qualifiers="virtual">

doc/classes/RigidBody.xml
18:     <method name="_integrate_forces" qualifiers="virtual">

doc/classes/TileSet.xml
15:     <method name="_forward_subtile_selection" qualifiers="virtual">
29:     <method name="_is_tile_bound" qualifiers="virtual">

modules/visual_script/doc_classes/VisualScriptCustomNode.xml
14:     <method name="_get_caption" qualifiers="virtual">
21:     <method name="_get_category" qualifiers="virtual">
28:     <method name="_get_input_value_port_count" qualifiers="virtual">
35:     <method name="_get_input_value_port_name" qualifiers="virtual">
44:     <method name="_get_input_value_port_type" qualifiers="virtual">
53:     <method name="_get_output_sequence_port_count" qualifiers="virtual">
60:     <method name="_get_output_sequence_port_text" qualifiers="virtual">
69:     <method name="_get_output_value_port_count" qualifiers="virtual">
76:     <method name="_get_output_value_port_name" qualifiers="virtual">
85:     <method name="_get_output_value_port_type" qualifiers="virtual">
94:     <method name="_get_text" qualifiers="virtual">
101:        <method name="_get_working_memory_size" qualifiers="virtual">
108:        <method name="_has_input_sequence_port" qualifiers="virtual">
115:        <method name="_step" qualifiers="virtual">

modules/visual_script/doc_classes/VisualScriptSubCall.xml
12:     <method name="_subcall" qualifiers="virtual">

[toger5]

So should we do something about it? add a underscore to all of them?

[neikeq]

I think it's worth considering removing them entirely (including methods like _ready and _process). Is there a real utility for the underscore?
This change would require fixing conflicts (like get and _get) and it would greatly break compatibility.

[toger5]

I definitely advocate going through those compatibility breaking changes.
I like the _ since it allows me to type it and let autocomplete show me all the virtual functions.

I still prefer giving virtual functions (those which are bound with: BIND_VMETHOD(MethodInfo("_process", PropertyInfo(Variant::REAL, "delta")));) a naming hint. on_functions_name, or _function_name.

[willnationsdev]

I also prefer having the underscores present for "virtual" concepts, but the inconsistencies need to be cleared, so I'd rather just add underscores to everything virtual that doesn't yet have it. If a method with an underscore is exposed to the scripting API, but isn't supposed to be called (like the import thing vnen mentioned), then why don't we just not make them visible in the scripting API?

It would also be nice if we could specify whether a method is virtual (intended to be overridden) vs. private (intended not to be called externally) in user-defined script functions, but I suspect that will be a task for #20318.

[Calinou]

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!