Add a casting option for @onready variables generated by dragging and dropping nodes when the editor setting "text_editor/completion/add_type_hints" is true.

[cosmic0256]

Describe the project you are working on

A third person shooter with lots of @onready variables used on scripts.

Describe the problem or limitation you are having in your project

Variables declared as @onready var foo: Node = $Foo don't satisfy Godot's Safe Lines tool, and will be marked as "unsafe".
Declaring them as @onready var foo := $Foo as Node is enough to make the tool happy.
I try to reduce "unsafe" lines as much as possible, and when you have a lot of nodes that you want to make into variables it becomes cumbersome to rewrite them all from form 1 to form 2.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Add a setting to EditorSettings called "text_editor/completion/cast_dropped_nodes" (or a better name if anyone thinks about one) to enable variables to be generated using type casting.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

1. In 'editor_settings.cpp' , add _initial_set("text_editor/completion/cast_dropped_nodes", false); to the //Completion section [line 664].
2. In 'script_text_editor.cpp', at the function ScriptTextEditor::drop_data_fw(const Point2 &p_point, const Variant &p_data, Control *p_from) [line 1817] add const bool use_cast = EDITOR_GET("text_editor/completion/cast_dropped_nodes"); under if (use_type) { [line 1940].
3. In 'script_text_editor.cpp', refactor [line 1949] from:
   text_to_drop += vformat("@onready var %s: %s = %c%s\n", variable_name, class_name, is_unique ? '%' : '$', path);
   to:

if (use_cast) {
    text_to_drop += vformat("@onready var %s := %c%s as %s\n", variable_name, is_unique ? '%' : '$', path, class_name);
} else {
    text_to_drop += vformat("@onready var %s: %s = %c%s\n", variable_name, class_name, is_unique ? '%' : '$', path);
}

4. In EditorSettings.xml add

<member name="text_editor/completion/cast_dropped_nodes" type="bool" setter="" getter=""> 
If [code]true[/code], adds casting to [code] @onready [/code] variables generated by dropping nodes into the code editor, allowing these to be considered safe lines by the text editor.
</member>

End Result:

editor_settings.cpp's changes would end up looking like this:

_initial_set("text_editor/completion/add_type_hints", true);
_initial_set("text_editor/completion/cast_dropped_nodes", false);
_initial_set("text_editor/completion/add_string_name_literals", false);

script_text_editor.cpp's changes would end up looking like this:

if (use_type) {
        const bool use_cast = EDITOR_GET("text_editor/completion/cast_dropped_nodes");

	StringName class_name = node->get_class_name();
	Ref<Script> node_script = node->get_script();
	if (node_script.is_valid()) {
		StringName global_node_script_name = node_script->get_global_name();
		if (global_node_script_name != StringName()) {
			class_name = global_node_script_name;
		}
	}
	if (use_cast) {
		text_to_drop += vformat("@onready var %s := %c%s as %s\n", variable_name, is_unique ? '%' : '$', path, class_name);
	} else {
		text_to_drop += vformat("@onready var %s: %s = %c%s\n", variable_name, class_name, is_unique ? '%' : '$', path);
	}
} else {
	text_to_drop += vformat("@onready var %s = %c%s\n", variable_name, is_unique ? '%' : '$', path);
}

EditorSettings.xml's changes would end up looking like this:

<member name="text_editor/completion/add_type_hints" type="bool" setter="" getter="">
If [code]true[/code], adds [url=$DOCS_URL/tutorials/scripting/gdscript/static_typing.html]GDScript static typing[/url] hints such as [code]-&gt; void[/code] and [code]: int[/code] when using code autocompletion or when creating onready variables by drag and dropping nodes into the script editor while pressing the [kbd]Ctrl[/kbd] key. If [code]true[/code], newly created scripts will also automatically have type hints added to their method parameters and return types.
</member>
<member name="text_editor/completion/cast_dropped_nodes" type="bool" setter="" getter="">
If [code]true[/code], adds casting to [code] @onready [/code] variables generated by dropping nodes into the code editor, allowing these to be considered safe lines by the text editor.
</member>
<member name="text_editor/completion/auto_brace_complete" type="bool" setter="" getter="">
If [code]true[/code], automatically completes braces when making use of code completion.
</member>

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

In theory one could write a script that

1. Reads the currently open script.
2. Searches for @onready variables that include type hints and no casting.
3. Copies the type hint into the end of the line.
4. Removes the type hint.
5. Moves the colon so it creates a walrus operator and leaves a space between itself and the variable.

This still needs to be run either every time a drag-and-drop variable is created, or manually by the user at their own discretion. The performance of the script would also rely on the length of the target script.

Is there a reason why this should be core and not an add-on in the asset library?

The drag-and-drop variable generation functionality and the string template used for it seem to be hard coded into the engine's source code. I don't see another way of doing it without making bigger changes to the relevant code.

[dalexeev]

I try to reduce "unsafe" lines as much as possible

See:

• Make onready variables created from dropping nodes include custom types godot#79198
• Static typing in GDScript

Safe lines do not always mean better or more reliable code. See the note above about the as keyword. For example:

@onready var node_1 := $Node1 as Type1 # Safe line.
@onready var node_2: Type2 = $Node2 # Unsafe line.

Even though node_2 declaration is marked as an unsafe line, it is more reliable than node_1 declaration. Because if you change the node type in the scene and accidentally forget to change it in the script, the error will be detected immediately when the scene is loaded. Unlike node_1, which will be silently cast to null and the error will be detected later.

[cosmic0256]

Hmm. I never thought about the error detection being better with form 1, but yeah it does sound better. This could be changed to not remove the type hints, but then variable declarations would become obnoxiously long.

Is the possibility of marking form 1 declarations as "safe" realistic? It's just really annoying that the engine provides such a tool but not a way to make it happy in every context, specially when the tool gives the impression that making it happy is the the intended way to write better code (particularly in the case of people less experienced with coding).

[theraot]

We can't make everybody happy, not even Godot's editor o,.,o

Godot is highlighting the lines where the types can't lead to a runtime error. On one hand, we don't want runtime errors. On the other hand, errors are useful.

So, I think it would be better if in this case it wasn't a runtime error. For example we can imagine Godot checking if the type is correct (for any form of the @onready declaration) and having an error or warning if it isn't... With the caveat that the script could be used in multiple scenes, and Godot only uses the current scene for this kind of analysis.

I believe in that case considering both safe could be more palatable.

And yes, we can make the argument that these are different kinds of safety. On a similar note, remember that both lines would fail if the Node does not even exist. And also that neither will save you from a freed Node (i.e. you might still need to check with is_instance_valid).

Meanwhile, have you considered using Node @exports?

[cosmic0256]

Taking into account the fact that nodes might or might not be there when the script expects it now makes me see how messy something like this could be.

As for @exports, I just don't like exporting things if I don't want to see them in the property editor. But I agree that these changes might not fit into Godot so I'll just do that I guess.

[theraot]

As for @exports, I just don't like exporting things if I don't want to see them in the property editor.

It is possible to have them collapse with an export group.

It is also possible to hide them dynamically, for example in _validate_property you could check if the Node is the current edited scene (requires tool), so these exported variables are visible when editing the scene, but not on instances.

---

I have to accept lines that are not marked as type safe anyway (e.g. when using a Dictionary), so I can live with @onready not being marked safe.

The reason I've been moving to @export is because I want to make reusable script, so they might work in completely different scenes. And I wish we had scene templates, but that is another topic.

---

Addendum: If we could annotate that a script is for a scene and only that scene, so that when we instantiate the script we get an instance of the scene (there was a proposal for that), then the static analysis could benefit from that too.

[dalexeev]

Is the possibility of marking form 1 declarations as "safe" realistic? It's just really annoying that the engine provides such a tool but not a way to make it happy in every context, specially when the tool gives the impression that making it happy is the the intended way to write better code

The problem is a misunderstanding of the tool, not how it works in this case. We can't make a tool that can reliably determine whether your code is good or not. So I don't think this is a good idea. This will only make things even more confusing if we try to highlight better code rather than type-safe code. But I understand how it can be annoying for a perfectionist.

Godot Contributors Chat

the tool gives the impression that making it happy is the the intended way to write better code

This is why we added a note to the documentation.

I think that the problem is that the as operator combines different cast types:

1. You know/expect a specific type in advance and want to tell the compiler about it (static cast). For example @onready variables or if you have already checked the type using the is operator. If the expectation is violated, it should produce an error, at least in debug builds. In release builds this could be undefined behavior, but as a bonus you would remove unnecessary runtime code.
2. You don't know the type in advance, and want to check it at runtime (dynamic cast). For example, InputEvent's or physical bodies in area signal callbacks. In this case, silently casting to null is the intended behavior.

If we introduced new cast operators (since as has issues), this might solve the type safety problem. However, the question is how well this fits into GDScript's design.

# Don't hides an error in debug builds.
@onready var label := static_cast[Label]($Label) # Safe line.

func _on_body_entered(body: Node2D) -> void:
    if body is Player:
        # The type is already checked at runtime.
        var player := static_cast[Player](body) # Safe line.
        ...

    # The type is not yet checked at runtime.
    var enemy := dynamic_cast[Enemy](body) # Safe line.
    if enemy: # No error above, but you should check that it is not `null`.
        ...

Also, if we improve the static analysis of local variables in the future, then the following code will also be considered safe:

if body is Player:
    var player: Player = body # Safe line at this point, no unnecessary runtime code.
    ...