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

Make child nodes export variables be editable in the scene root inspector #7803

Open
TushieSushies opened this issue Sep 22, 2023 · 17 comments

Comments

@TushieSushies
Copy link

Describe the project you are working on

A 2.5D RPG where the player can interact with objects and play dialogue by interacting with npc's / objects

Describe the problem or limitation you are having in your project

If I instance a scene, and I want to access or edit an exported variable of a child node in the inspector, currently the only way is to make a setget variable in the root object, this is not ideal and is prone to mistakes.
We want to edit out what line of text/asset is played when interacting and we want to keep the project structure consistent with Godot's node approach, such as keeping the dialogue logic in a dialogue component, but editing what asset is used in the variable in the dialogue component is not available on instanced scenes.

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

If child node exported variables appeared, we wouldn't need to use setget functions and it streamline the work greatly.

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

Say we have a component that plays dialogue, we called it DialogueComponent, and is a child of the node TestCube:
image

The DialogueComponent uses an Exported variable to know what asset to play when ordered from a function inside the node.
image

For exported variables, they could be visible to the root node like in this mockup here:
Untitled-2

Resulting in them being accessible in instanced scenes like this where they otherwise wouldn't be:
Untitled-3

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

Yes, with a setget function in the root node script, however it's prone to typos and null variables crashing the game

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

This will greatly improve the flow of instancing scenes where editing exported variables of child nodes is required

@Zireael07
Copy link

Counterpoint: I have scenes where child nodes have a TON of exported variables. Exposing them all on the root would be UNMANAGEABLE

I use editable scenes instead

@TushieSushies
Copy link
Author

Then I think it could be another type of variable attribute if not exported

@3lis4
Copy link

3lis4 commented Sep 23, 2023

I think this proposal might be interesting as a syntactic sugar to limit the amount of "useless" setters when many exported variables of the child nodes need to be exported as well on the parent node.

It could be an annotation exposing a child node's exported variable. Something like @export_child(path_to_child_node:var_name) in parent node's script.

In our project we have many of these setters and such an annotation would greatly improve readability and be easier/faster to write. Also, auto-completion could help to write the child node's path (on the same way it is for $ syntax).

@TushieSushies
Copy link
Author

I agree, @export might not be a good idea due to the points made by other users, but another way to export to root node will surely come in handy instead of relying on setget

@ywmaa
Copy link

ywmaa commented Sep 24, 2023

I really need this feature too, but please the idea of exporting everything manually to the root is not good.

It could be an annotation exposing a child node's exported variable. Something like @export_child(path_to_child_node:var_name) in parent node's script.

This sounds much better.

@TushieSushies
Copy link
Author

I vote their for their proposal as well!
Now we just need someone kind enough to implement it ^_^

@ywmaa
Copy link

ywmaa commented Sep 25, 2023

I may try to do it myself, but I guess we need to have full understanding of what it will do, because I will already try tweaking GD script which is very new to me, so I don't want to also go in it blind.

@export_child(path_to_child_node:var_name)
this works as both NodePath and @export_category ?

so it will create a new category in the root of the node containing the name/class of the exported child with its exported categories too.

do we need custom naming ?
should it be in the root, or at the top like normal @export ?
what happens when you set the scene to editable, and edit the children values, will they reflect to the parent ?
what happens when we use this recursively (child also exported another child) ?

Edit : Typos

@ywmaa
Copy link

ywmaa commented Sep 25, 2023

Here we go, I made a draft PR, you can test it, and/or provide feedback.

@AttackButton
Copy link

AttackButton commented Sep 29, 2023

Edit: this can be done with editable_children.

@ywmaa
Copy link

ywmaa commented Sep 29, 2023

Edit: this can be done with editable_children.

Yes, but that's not Out of the box workflow, like you need to do it for each new inherited scene.

Unlike this method which will expose the values by default to the parent node without the editable_children .

@AttackButton
Copy link

AttackButton commented Sep 29, 2023

There is already a discussion about this enhancement in the editable_children: #3248.

Furthermore, there are two prs (apparently incomplete) trying to implement that proposal: godotengine/godot#63307 and godotengine/godot#60974.

I think that the ideal would be to implement a better version than these prs for editable_children, perhaps creating an annotation or property allowing editable_children selecting which resources to expose.

@ywmaa
Copy link

ywmaa commented Sep 29, 2023

@AttackButton

I am not sure, but did you look at my PR ?

Since you are mentioning an annotation ?
Is the @export_child annotation not what you mean ?

@AttackButton
Copy link

AttackButton commented Sep 29, 2023

I mentioned an annotation or a property to enable editable_children by default for the instantiated scenes from a given base scene.

The only question about this proposal is whether it is not covered by what editable_children already does or could do with an enhancement.

@TushieSushies
Copy link
Author

I agree the features in the proposal #3248 should be implemented instead, it’s a more elegant and less clumsy implementation of my idea

@ywmaa
Copy link

ywmaa commented Sep 30, 2023

Yeah it looks better to me, too.

Is there any PR for it ?

@Shadowblitz16
Copy link

Shadowblitz16 commented May 3, 2024

Why not treat scene roots as packages instead of the root node?
The scene package by default could have nothing exported and user's could choose what properties on what node's are exported.

I think this is kinda what unreal engine does.

@AThousandShips
Copy link
Member

@MrModez Please don't bump without contributing significant new information. Use the 👍 reaction button on the first post instead.

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

7 participants