Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exported NodePath has inconsistent empty values #21556

Closed
QbieShay opened this issue Aug 29, 2018 · 10 comments
Closed

Exported NodePath has inconsistent empty values #21556

QbieShay opened this issue Aug 29, 2018 · 10 comments

Comments

@QbieShay
Copy link
Contributor

Godot version: 3.1 custom

Issue description:
If a NodePath is exported and never assigned, its value will be null. If it is assigned and then cleared, the value will be an empty string.

Proposal:
Make the default value either be null or "" and document it

@akien-mga
Copy link
Member

As far as I know that's by design, any identifier with no value assigned is Null initially. The export hint is just that, a hint, for the editor, so it doesn't impact the type of the variable (it's still dynamically typed).

So for GDScript, export(NodePath) var node_path and export var node_path are the same things, identifiers which are not yet pointing to any data.

On the other hand, once you've assigned data to the identifier, it has a type, and clearing it will give you an empty object of that type (unless you clear it by assigning to Null again).

Consider this example which is valid GDScript:

export(NodePath) var node_path
export(Dictionary) var dict
export(Vector3) var vec3

func _ready():
	print(node_path)
	print(dict)
	print(vec3)
	node_path = 42
	dict = "Hello world"
	vec3 = ["It's a duck"]
	print(node_path)
	print(dict)
	print(vec3)

Prints:

Null
Null
Null
42
Hello world
[It's a duck]

@akien-mga
Copy link
Member

akien-mga commented Aug 29, 2018

Now with type hints:

export(NodePath) var node_path: NodePath
export(Dictionary) var dict: Dictionary
export(Vector3) var vec3: Vector3

func _ready():
	print(node_path)
	print(dict)
	print(vec3)

Prints:

{}
(0, 0, 0)

So it works fine, apart from the fact that empty NodePaths don't seem to have a proper print representation. Edit: Actually empty NodePath is properly printed as "\n" in the terminal, just not in the Output panel.

@QbieShay
Copy link
Contributor Author

I see. I still think that it should be discussed because it can represent a problem.
I had this code:

if nodepath == null:
   my_thing = get_parent()
else:
   my_thing = get_node(nodepath)

This worked until I opened a scene, assigned the nodepath and cleared it. What happened then is that the object was trying to reference itself as the my_thing and my game crashed. Tracking down this bug wasn't very hard but i think this should be addressed in a more elegant way.

@akien-mga
Copy link
Member

akien-mga commented Aug 29, 2018

IMO in your use case you should either use type hints, or initialize your export vars to make sure that they're not null, so:

export(NodePath) var nodepath = NodePath()
# or
export(NodePath) var nodepath: NodePath

...
    if nodepath == NodePath():
        my_thing = get_parent()
    else:
        my_thing = get_node(nodepath)

Or of course use belt and braces with if !nodepath or nodepath == NodePath():.

@akien-mga
Copy link
Member

I don't think we can change the fact that export(NodePath) var nodepath is null until assigned, as I mentioned the export() keyword is just defining a hint for the inspector, but it doesn't imply typing of the variable.

Within a script,

export(NodePath) var node_path
export(Dictionary) var dict
export(Vector3) var vec3

is exactly the same as

var node_path
var dict
var vec3

What could be done however is to fully deprecate the export(<Type>) syntax now that we have optional typing, and use export var nodepath: NodePath instead to provide both editor hints and typing.

CC @bojidar-bg @vnen

@vnen
Copy link
Member

vnen commented Aug 29, 2018

IINM if you use export var nodepath: NodePath the default will be an empty NodePath instead of null, since typed variables are given a default value (null is used only for objects).

Technically would be possible to do the same with export hints, but I'm not sure if it's wanted.

@ghost
Copy link

ghost commented Sep 28, 2018

It might be a generous thing to add a warning or error on export vars with hints and null values. I think the first impressions of it led me to expect empty NodePath to compare as null, because of how this was initially working. I understand everything much better now after bumbling around with it.

@akien-mga I will add I found some issues (as of 2893b5a) with export vars currently that can make these things a bit more confusing.

For example, a null export var with a float hint.

extends Node2D

export (float, 0.1, 1.0, 0.1) var something

In the inspector you'll see this:

as null

If you print it out you get:

godot_master_2018-09-28_14-05-27

I posted some issues detailing it a bit better, for them for anyone following here.
#22507
#22505

It's all minor stuff, but there are some rough edges to this.

@name-here
Copy link

Isn't this fixed by the new @export annotation?

@akien-mga akien-mga added this to the 4.0 milestone Oct 23, 2020
@vnen
Copy link
Member

vnen commented Nov 10, 2020

Isn't this fixed by the new @export annotation?

I guess so, because now you can only use the type hint to set the export type, so it should always default to an empty default value instead of null (if it's not an object type).

@Calinou Calinou changed the title Exported NodePath has inconsistant empty values Exported NodePath has inconsistent empty values Aug 21, 2021
@Calinou
Copy link
Member

Calinou commented Aug 21, 2021

Closing per @vnen's comment.

@Calinou Calinou closed this as completed Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants