Ability to show exported variables only when viewed within their scene

[Meorge]

This PR adds a new annotation to GDScript, @export_if_root, which when placed after an @export annotation, causes the annotated property to only show in the inspector if the node is the root of the current scene. This is useful for hiding irrelevant information when editing nested scenes.

There is a discussion thread at godotengine/godot-proposals#10017 and a formal proposal at godotengine/godot-proposals#10024. More details about the reasoning behind the feature are provided there (especially in the proposal). Before merging anything, I think it may be wise for us to refine the syntax.

• Bugsquad edit, closes: Add an annotation to only display property if node is root of current scene godot-proposals#10024

[theraot]

This is technically not a review, but I can tell you right now that you should add documentation for PROPERTY_USAGE_SHOW_IF_ROOT. But perhaps you want to wait until you iron out the other details. I'll also make a post in the proposal shortly. Edit: I have done the post.

[AThousandShips]

There's some formatting errors, please run formatting and add to documentation before we can run CI on this

[Meorge]

I can certainly do that! For the documentation specifically, should I go ahead and write up the documentation for the implementation as it is currently, even if the details (such as the name of the annotation) are likely to change before being merged in?

[AThousandShips]

You should at least generate them, but you can leave a temporary description until things are decided on

[Meorge]

Alrighty, thanks! Will try to get those done today then 😄

[AThousandShips]

I feel this annotation is far too specific, I can think of few cases when I'd want something to show up only when the node is the root

Honestly I'd personally be more interested in when it isn't the root but instead is in an instanced scene, that makes far more sense to me

But I feel this is far too specific and also far too easy to work around in custom code to warrant a custom annotation, you can simply do this with _validate_property and EditorInterface.get_edited_scene_root() (though that might not work easily in exported projects, but that's something to solve with other features like editor only compiled code, not something against that workaround)

[Meorge]

I looked into the _validate_property approach, and used it to implement the behavior before I tried making the @export_if_root annotation. I wasn't a big fan of it: godotengine/godot-proposals#10024 Overall I felt like requiring @tool, and thus having to check Engine.is_editor_hint in functions like _process before running the gameplay code, along with the somewhat verbose code inside of _validate_property itself, bloated the script.

# With the custom annotation
extends Node2D

@export @export_if_root var sprite: Sprite2D
@export @export_if_root var sound_player: AudioStreamPlayer2D
@export @export_if_root var health: int = 10

func _process(_delta):
    # `_process` function can be dedicated to gameplay code, just as it would be if we weren't
    # worried about exposing/hiding the properties
    print("Sprite = %s, sound player = %s, health = %s" % [sprite, sound_player, health])

# Without the custom annotation
@tool
extends Node2D

@export var sprite: Sprite2D
@export var sound_player: AudioStreamPlayer2D
@export var health: int = 10

func _process(_delta):
    # Have to check for editor hint, to avoid gameplay code executing in editor
    if Engine.is_editor_hint():
        return
    print("Sprite = %s, sound player = %s, health = %s" % [sprite, sound_player, health])

func _validate_property(property):
    # - Property names are written here as strings, so if we change property names later on we have
    #   to remember to return here and change them here as well.
    # - Other conditions for enabling `PROPERTY_USAGE_NO_EDITOR` aren't very intuitive.
    var scene_root = EditorInterface.get_edited_scene_root()
    if property.name in ["sprite", "sound_player"] and scene_root.is_ancestor_of(self) and not scene_root.is_editable_instance(self):
        property.usage = PROPERTY_USAGE_NO_EDITOR

That being said, Mickeon commented in the proposal issue about a plugin that accomplishes very similar behavior: godotengine/godot-proposals#10024 (comment)

The upside to the plugin is that the user scripts wouldn't have to use @tool or _validate_property, and thus the script attached to the gameplay node could be more focused on doing the gameplay-specific things rather than editor boilerplate. Additionally, it ties the visibility to GDScript's convention of prefixing "private" variables with an underscore, rather than having them be completely separate as the @export_if_root annotation would do.

Some kind of functionality like this still feels to me like it'd be nice to have built into the engine, rather than having to search around for plugins that accomplish it or write it into each script manually using @tool. There seemed to be a lot of support for a @private (or similar) access modifier in godotengine/godot-proposals#641, so perhaps if that is eventually worked on, this feature could be rolled into that.

I'm not sure at which point it's acceptable to take a proposal and try to implement it, and my experience with Godot's source code is very limited (this PR would be my first major interaction with it). Implementing some kind of enforced access control with a @private annotation might be something I would be interested in taking a shot at and then opening a new PR for when it has some progress, though! 😄

[AThousandShips]

I'm not sure at which point it's acceptable to take a proposal and try to implement it

There really isn't any limit for that, the only thing to consider is that you might be wasting time if you try to implement a proposal before it's gotten good support and ends up rejected because it's not in high enough demand, or having to redesign it multiple times because the discussion changes and other details are focused on

So beyond that letting a proposal sit until some stronger consensus has been reached it's up to you, but jumping on a proposal before it's been discussed deeply enough can risk making a PR when the discussion would otherwise have led to dismissing the idea or some workaround being found or some different solution being decided on (though to be clear: support or lack of support for a proposal isn't an absolute decision making consideration, it's ultimately up to the area maintainers to decide)

[Meorge]

Closing this as there's not a lot of interest in it, and/or if the private access modifier ends up going forward, a similar function could potentially be a part of that.