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

Ability to declare exported properties that are not shown (aka hidden) in inspector #7794

Closed
limbonaut opened this issue Sep 22, 2023 · 13 comments · Fixed by godotengine/godot#82122
Assignees
Milestone

Comments

@limbonaut
Copy link

limbonaut commented Sep 22, 2023

Describe the project you are working on

I used _get_property_list to hide properties in inspector on multiple occasions. It is especially useful for data serialization in the resource files when making editor plugins. However, the dictionary syntax is not easy to use, and it is quite easy to make a mistake.

Describe the problem or limitation you are having in your project

It's cumbersome and error-prone to implement _get_property_list to hide a property in inspector. There has to be a better way!

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

Similar to @onready, there could be a @hidden annotation to hide a property in the inspector.

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

@hidden @export var data: MyData

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

It can be worked around using _get_property_list:

extends Node

var data: Array

func _get_property_list():
	# Export `data` property, but don't show it in inspector.
	# (`data` property will be saved in the scene file.)
	var serializable = [
		{
			name = "data",
			usage = PROPERTY_USAGE_NO_EDITOR,
			type = TYPE_ARRAY,
		},
	]
	return serializable

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

N/A

@dalexeev
Copy link
Member

Given that hidden properties are not visible in the editor, there is no point in combining @hidden with @export_* annotations, only with @export. Therefore, I suggest adding a single annotation and name it @export_hidden, @export_storage or @export_no_editor.

@Kakiroi
Copy link

Kakiroi commented Sep 22, 2023

This can be useful for animation player to get property, or marking duplicatable properties in resource files without cluttering editor.

@Jeremi360
Copy link

It would also useful for custom classes that want to overwrite some exported var by class after it inherits from.

For example I develop AdvancedTextLabel that inherits from RichTextLabel:
so I would hide text and bbcode as my class it to be enabled by default

@Maran23
Copy link

Maran23 commented Sep 22, 2023

Another idea could be @export(false), so @export accepting a visibility parameter which defaults to true

@dalexeev
Copy link
Member

dalexeev commented Sep 22, 2023

Another idea could be @export(false)

Interesting idea, but we already have many specialized export annotations instead of just one annotation with parameters (like export keyword in 3.x). In addition, autocompletion will add parentheses and place the cursor between them, which will annoy users. It is logical that the basic and most often used export annotation has no parameters.

@limbonaut
Copy link
Author

@export_hidden, @export_storage and @export_no_editor are all good fits imho. And the main question right now is, as discussed in the PR, which one would stand the test of time. I think @export_hidden is a little disconnected from internal implementation, but sounds better than @export_no_editor version and less typing. And @export_storage fits best to most common intention for this functionality, which is to serialize some variables with a scene/resource file.

@Jeremi360
Copy link

I think @export_hidden and any other variant with prefix @export for this usage would be confusing as other @export make var visable in editor.

Also @export make it sound like it would be implented in way:

@export_hidden var new_var

Which make unuseable, in example I gave above I was thinking it would be this way:

For example I develop AdvancedTextLabel that inherits from RichTextLabel:
so I would hide text and bbcode as my class it to be enabled by default

@hidden("property/path")
# or
@on_editor("property/path")

@dalexeev
Copy link
Member

I think @export_hidden and any other variant with prefix @export for this usage would be confusing as other @export make var visable in editor.

"Export" does not mean "display in inspector", it means "get property value from scene/resource file". That is, serialization is primary. The fact that the property is editable in the inspector is secondary, although from the user’s point of view it seems the opposite. See also #3906.

Therefore, I would prefer that the annotation name start with @export_ rather than be something else entirely. It would be consistent and clear. In addition, we have a check that multiple export annotations cannot be applied to one variable. The naming exception would be confusing in my opinion.

@export_hidden looks pretty good, it means making the property serializable (exported) without displaying it in the editor. But the question is whether @export_hidden should correspond to PROPERTY_USAGE_STORAGE or PROPERTY_USAGE_NO_EDITOR. See godotengine/godot#82122 for details.

@limbonaut
Copy link
Author

As I see it, there are two quite distinct use cases:

  1. Serialization: Export GDScript properties without exposing them to the Inspector with the intention of serialization (aka save some variables in a scene/resource file).
  2. Shadow/hide already existing core/script class properties in the inspector.

I'm not sure how much demand is for the second use-case. If you exported a property with the same name as a core class property, you should effectively shadow it for any script (but not the core C++ code). However, the property of the core class will likely show up in the inspector anyway, since it is exported in the C++ code.

As for the first use case, I also proposed using @serialize instead of @export_* to narrow down the purpose of the annotation. In my opinion, it is the clearest option when it comes to the first use-case (aka serialization). Reasons against it are: a) there is a code path in the engine that checks if only one @export* annotation used for any given property (it is considered an error); b) there is an established pattern of using @export_* for such things.

@limbonaut
Copy link
Author

limbonaut commented Sep 23, 2023

What are your opinions 👍 👎 on renaming PROPERTY_USAGE_NO_EDITOR -> PROPERTY_USAGE_HIDDEN? Could be also utilized in other places like documentation parsing.

@dalexeev
Copy link
Member

dalexeev commented Sep 23, 2023

a) there is a code path in the engine that checks if only one @export* annotation used for any given property (it is considered an error)

No, that's not the problem. The parser does not use begins_with() for the check. All export annotations1 are processed in the GDScriptParser::export_annotations() method. The problem is that the error text will be confusing. For example:

@serialize
@export # <-- Error: Annotation "@export" cannot be used with another "@export" annotation.
var a = 1

This error is confusing because there are no other annotations applied to the variable that begin with @export and it would not be obvious to the user that @serialize is also an export annotation.

I also believe that the word "export" in our terminology already means serialization. The exported (serialized) property may or may not show up in the inspector, it doesn't matter. But introducing a new term, "serialization", which means the same thing as "export", will only confuse users more. Users may think that there is some difference between the two, that exported properties are not serialized, etc.

b) there is an established pattern of using @export_* for such things.

Please see the discussion in #3906 first. Even if the term "export" is not the best (there was a proposal to rename it to "expose", "storage", "serialize", etc.), this is established terminology and in any case cannot be renamed in 4.x due to compatibility.

What are your opinions 👍 👎 on renaming PROPERTY_USAGE_NO_EDITOR -> PROPERTY_USAGE_HIDDEN?

We can't really rename a constant, only deprecate it and add a new constant with the same value. But I don't think it makes sense now. Note that PROPERTY_USAGE_NO_EDITOR exists mainly for historical reasons.

3.x:

PROPERTY_USAGE_DEFAULT = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_NETWORK,
PROPERTY_USAGE_NOEDITOR = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_NETWORK,

4.x:

PROPERTY_USAGE_DEFAULT = PROPERTY_USAGE_STORAGE | PROPERTY_USAGE_EDITOR,
PROPERTY_USAGE_NO_EDITOR = PROPERTY_USAGE_STORAGE,

Now PROPERTY_USAGE_NO_EDITOR == PROPERTY_USAGE_STORAGE, but in the future we may add other default flags.

Footnotes

  1. Except @export_category, @export_group and @export_subgroup which do not apply to variables, but are themselves "pseudo-members".

@dalexeev
Copy link
Member

I thought about it from a different perspective. What if the user instead wants to display the field in the editor, but not store the value in a file? In this case, a modifier annotation makes more sense than duplicating all the export annotations. That is, export annotations by default use storage + editor, and we could add two modifier annotations @no_storage and @no_editor (the latter, however, still makes little sense to combine with anything other than @export).

@dalexeev
Copy link
Member

I thought about it from a different perspective. What if the user instead wants to display the field in the editor, but not store the value in a file?

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

Successfully merging a pull request may close this issue.

7 participants