Create an annotation (e.g. @required) that checks an @export var is not null at compile time

[harryclough]

Describe the project you are working on

General Godot scripting.

Describe the problem or limitation you are having in your project

When writing scripts, I frequently use an @export var to create a reference to another node in the scene tree that I don't want to hard code the path to. I will then select the target node in the editor. However, it is very easy to forget to assign these variables, or for the variable to be un-assigned when modifying the scene tree. This will often result in a null value related error being thrown somewhere in the code, and it is not always obvious it was caused simply by the @export var being unassigned.

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

I suggest an annotation that will simply throw an error or warning at compile time if an @export variable has not been assigned by the editor (i.e. the value is null). This helps enforce the assumption made by the programmer that these variables should be set, and makes it very clear what the issue is, instead of an obscure null-value error thrown somewhere else in the code-base.

Note: This is only intended to apply to nullable variables, so for example it would not check if an integer has be changed from 0.

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

I imagine it could look something like this:

@required @export var foo

or even it's own export type

@required_export var foo

Then if the variable is not set in the editor at compile time, an error/warning along the lines of The variable 'foo' is not assigned. This only applies to variables that are nullable / null by default, such as nodes.

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

This can be handled with explicit null checking, e.g.

func _init():
    assert foo != null

However, I feel like implementing this as an annotation is much clearer and signals intent better, as well as keeping the relevant code in one place rather than spread over the script. Additionally, I find that I use this general pattern very frequently, and avoiding this boiler-plate is desirable.

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

In my experience, selecting nodes in the editor via an @export is a common pattern, and is better practice than hard coding the relation. Adding this as an annotation explicitly supports this practice, and reduces boiler-plate and debugging time on an common error.

[harryclough]

I have realised now that this is possibly a duplicate of #550 although that seems to be advocating for a warning to all variable types, this proposal only applies to nullable types, particularly Nodes. I suspect this would make the implementation much more straightforward. I'm not sure if this similarity is worth closing this issue as a duplicate or not.

[Jesusemora]

I don't see the point on a new annotation. ALL exports should be assigned at "compile time", that's the point of an export. if you assign an export in game, it can lead to problems because the instances of a scene share the same export. It happened to me, assigning an export at runtime to an instantiated scene changed it for the other instances of the scene.

my proposal: make it so that a warning is issued when ANY export is unassigned. if the export is assigned by code, the warning will not be thrown:
@export var speed : float = 0.5
@export var character_sheet : CharacterSheet = CharacterSheet.new()
if the export is of a type that can be 0, the warning will not be thrown either:
@export var speed : float#0
but if the export is null, a warning will be thrown:
@export var health_bar : Range

In my experience, selecting nodes in the editor via an @export is a common pattern

is better practice than hard coding the relation

there are no useless features of the engine. exports are for assigning nodes that exist outside a scene and for testing values. they can be used to assign nodes inside a scene. but if the node you have to reference will be created in game, an export is a bad idea. and using get_node allows you to replace a child node with another in editor.
both have their uses, neither is a magic fit-all solution.

[theraot]

We can leave an export that takes a Node or a Resource pointing to nothing. I assume - I confess I only skimmed the proposal - that is what the annotation is about.

But, the thing is, they can be unset in runtime, and nodes in particular can be freed without setting the variables that point to them to null, and NodePath variables could end up pointing to something that does not exist without the NodePath being empty.

Given that the title says "at compile time", I take you are aware of that. I bring it to the discussion to suggest the following: make it @required instead of @not_null. In doing so, it would not step on the nullability related GDScript proposals that are not focused on exported variables. In other words if we end up needed a broader @not_null it won't be an issue that it was used for this proposal (assuming it gets implemented).

[harryclough]

@theraot yes, my intention was not for it to be a guarantee that the value can never be null, it is more an indication that the variable is expected to be non-null at compile time (i.e. it should be set in the editor). I think I agree that @required is a clearer name. I will edit the proposal.

@Jesusemora I'm not completely sure what you're suggesting. My intention for this feature is that it is for the specific use case you mentioned, where you want to reference a node outside the current scene, or a node that you don't want to hard code the relation to. It is easy to accidentally leave this value unassigned (i.e. null), and get an error somewhere down the line. Since there are several ways a variable can get set to null (often intentionally), it is often not immediately obvious that it was caused by the value never being set in the first place. Particularly when editing a scene tree, reference set in the editor can get set to null automatically. The intended use-case is for nodes, but it should work for any nullable value.

This annotation would let you signal that a value is expected to be set in the editor, and the code could fail if it is not. It can then warn you at compile time about this.

[harryclough]

@Jesusemora my idea was only for it to work on nullable types, so it wouldn't check if a float was non-zero (the default value), for example. I agree with you there.

[sockeye-d]

A workaround would be to also add configuration warnings:

func _get_configuration_warnings():
	var w: PackedStringArray
	if export1_is_configured_badly:
		w.append("message")
	return w

[theraot]

Aside from the afordmentioned:

• An export variable hint which shows a warning icon if the value has not been set in the inspector #550

I found another variation on the proposal (potential duplicate):

• Add @required annotation for exported properties #8343

[h0lley]

@Jesusemora

I don't see the point on a new annotation. ALL exports should be assigned at "compile time", that's the point of an export.

I would interpret it so that unassigned here means that no custom value is serialized in the tscn/tres file.
the editor should not write default values into tscn/tres file.

I can see an use case for an @export property where the user must specify a custom value because a fallback to a default value is not applicable.

enum Options {PLEASE_SELECT, OPTION_A, OPTION_B, OPTION_C}

@export var required_choice := Options.PLEASE_SELECT

if you assign an export in game, it can lead to problems because the instances of a scene share the same export.

that is not a thing. all that @export does is to display the property for editing in the inspector and store its value inside the tscn or tres in case it is changed from the default. the value is then retrieved only once whenever an instance is created from that file. there is no magic state sharing across instances going on.

you probably have had a Resource embedded in a scene, and forgot to set that Resource to local_to_scene, which is why the scenes all shared the same resource instance and so changing a property affected all scenes, however that has nothing to do with that property having had an @export annotation.