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

Using export_multiline should default to a blank string, not null #74188

Open
Miziziziz opened this issue Mar 1, 2023 · 5 comments · May be fixed by #77875
Open

Using export_multiline should default to a blank string, not null #74188

Miziziziz opened this issue Mar 1, 2023 · 5 comments · May be fixed by #77875

Comments

@Miziziziz
Copy link

Miziziziz commented Mar 1, 2023

Godot version

v4.0.stable.official [92bee43]

System information

Windows 10

Issue description

Using @export_multiline and then not defining the variable causes the inspector to show <null> when it should show nothing.

Steps to reproduce

Make a script, add this line

@export_multiline var dialog

look in inspector window

Minimal reproduction project

N/A

Bugsquad edit: Added mark up for <null> in the description, so GitHub does not show it as an empty string.

@zacryol
Copy link
Contributor

zacryol commented Mar 1, 2023

Looks like this happens with other export annotations as well
I've checked both @export_file and @export_color_no_alpha, and the var gets set to null, when it would make more sense for the default value to be of the appropriate type ("" or Color(0, 0, 0))

@dalexeev
Copy link
Member

dalexeev commented Mar 1, 2023

Export annotations do not change the default value of a variable.

@export_multiline var dialog # = null

Use an explicit initializer and/or specify a static type:

@export_multiline var dialog = ""
@export_multiline var dialog_2: String

@timothyqiu
Copy link
Member

Edited the markup in OP so '<null>' is not displayed as '' ;)

I agree that showing <null> in the TextEdit for a null value is a bit confusing, as it's different from typing <null> yourself. It's worse for a color annotation: the user gets two different blacks.

@akien-mga
Copy link
Member

akien-mga commented Mar 2, 2023

Fixing this properly might be tricky. The export annotations define which kind of inspector widget is created (EditorProperty subclass, e.g. EditorPropertyMultilineText). This widget then handles the property it was created for.

It could likely have a check added here:

void EditorPropertyMultilineText::update_property() {
        String t = get_edited_object()->get(get_edited_property());
        if (text->get_text() != t) {
                text->set_text(t);
                if (big_text && big_text->is_visible_in_tree()) {
                        big_text->set_text(t);
                }
        }
}

To handle the case where get_edited_object()->get(get_edited_property()) is not a String (it's a Variant, so the first line here is casting it to String).
But this implies having a way to represent a non-String Variant in a good way in EditorPropertyMultilineText... and then the same needs to be implemented for all other EditorProperty classes.

So that wouldn't be too hard, but it's complex in terms of UX and requires quite some added boilerplate.

If someone has a better idea, let me know :D

As a side note, we could maybe add a GDScript warning when using a "typed" annotation (an annotation which is designed for a specific type) on an untyped and uninitialized export var.

Finally, we could also change the design and make it so that typed annotations force initialize to the matching type. That's outside the scope of this discussion, and possibly not a wanted change, but it would solve this issue too.

@dalexeev
Copy link
Member

dalexeev commented Mar 2, 2023

As a side note, we could maybe add a GDScript warning when using a "typed" annotation (an annotation which is designed for a specific type) on an untyped and uninitialized export var.

I wanted to suggest this too, it seems to me that this is the right and quite simple solution to implement.

EDIT: The error "Specify a static type or initial value" is fine too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants