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

Improve usability of Editable Children #3248

Open
reyma24 opened this issue Sep 3, 2021 · 29 comments · May be fixed by godotengine/godot#84018
Open

Improve usability of Editable Children #3248

reyma24 opened this issue Sep 3, 2021 · 29 comments · May be fixed by godotengine/godot#84018

Comments

@reyma24
Copy link

reyma24 commented Sep 3, 2021

Describe the project you are working on

An immersive sim.

Describe the problem or limitation you are having in your project

I believe that the editable children feature has the potential to be one of the most important part of Godot's scene editor. If improved, it would finally stop all those requests for component systems / multiple inheritance / modularity systems.
Godot already has the perfect solution for code modularity - adding nodes. This works perfectly and is increadibly easy to understand and implement:
components

The problem is with editing them after instancing the scene.
I found 3, in fact:

  1. When the scene contains many nodes, the tree becomes cluttered and shows nodes that are not needed (more in 27828):
    not_needed

  2. You have to right-click > select editable children every single time, on every single node (more in 22263).
    editable

  3. The current color of the properties (red) indicates that those properties should not be edited (see this).
    red

There might be more problems with editable children, these are the ones that I found and think are important things to change / implement.

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

Improve editing children's properties in instanced scenes.

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

Solutions:

  1. Select which nodes will be editable - there should be an option for setting a node's visiblity when parent node is instanced:
    editable_in_parent

  2. Show editable nodes by default - there should be an option in the Editor Settings for showing editable children by default (automatically, when node enters tree):
    new_editor_setting

  3. Remove the red coloring from childrens' properties.

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

The alternative is to create an addon that shows the children attributes in the parent's inspector, however that also comes with many problems (editor doesn't save changes to non-editable children and you can't attach metadata to instanced scenes).

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

The feature is already implemented, the only thing it needs are improvements listed above.

The end result:
final

@Calinou
Copy link
Member

Calinou commented Sep 3, 2021

  1. Remove the red coloring from childrens' properties.

I wouldn't remove this coloring as it indicates that your changes may be lost in the future. We can change it to the warning color (yellow) instead to make it less scary.

Edit: Done in godotengine/godot#53260.

@Calinou Calinou changed the title Improve Editable Children Improve usability of Editable Children Sep 3, 2021
@NathanLovato
Copy link

I agree with Calinou here, the issue with Editable Children isn't the presentation right now: as with inherited scenes, you have to be extremely careful not to lose your changes.

At GDQuest, we very rarely use Editable Children because of that: it's too easy to change the parent scene or refactor a class and introduce game-breaking bugs you won't detect until days or weeks later.

With that limitation fixed somehow, sure, it'd be a great feature. There was also a proposal to improve inherited scenes and limit or prevent data losses. To me, that'd be the starting point.

@Wavesonics
Copy link

Maybe the data loss is really what we should focus on finding a solution for. At the very least, knowing that the data loss has happened (like a bunch of changes visible in your source control) so that it doesn't silently break things.

Can someone explain the root cause of the data loss?

@Calinou
Copy link
Member

Calinou commented Sep 4, 2021

Can someone explain the root cause of the data loss?

One common cause for this is when you rename a property. There's already an open proposal about adding an annotation to handle property renames, but I can't find it right now.

@raulsntos
Copy link
Member

raulsntos commented Sep 4, 2021

Can someone explain the root cause of the data loss?

One common cause for this is when you rename a property. There's already an open proposal about adding an annotation to handle property renames, but I can't find it right now.

I think you are referring to this one: #3152

@TackerTacker
Copy link

I would like a solution that avoids showing the children completely, but it would still be better than what we have right now.

In your the end result image you show the "Attacker" child as selected and the properties you want from that child, the "Script Variables".
The Scene tab can become quite cluttered if there are a lot of nodes. Instanced scenes help fight this by creating bigger building blocks out of smaller pieces which get hidden to de-clutter the scene graph, so this would counteract this by partially bringing back hidden away nodes, even though you don't actually want the hidden nodes back, you want to be able to change the values found under "Script Variables" and nothing else.
I think it would be much better to make only selected properties, or property groups available directly on the instanced scene itself, without cluttering up the scene dock with child nodes.

@justinbarrett
Copy link

I am not a contributor, but I have never had a case where I needed to edit the children.. I have always just reopened the scene, fixed the issue and moved on...This is another reason there are exported variables. That is where I do any changes needed....if someone implements this...it's, ofc, fine...but I feel like this is the wrong path and might teach bad habits. Feels very "corner case".

@mark-rafter
Copy link

I would prefer a variant of this where you can selectively override children (by right clicking the node and checking "Override Child").. Currently I'm doing this in code by exporting NodePath but it's a bit tedious and pollutes the code/scene tree, and is susceptible to bugs (e.g. no static type checking).

Current behaviour

// Character.cs
[Export] public NodePath CharacterAnimationTreePath { get; set; } = nameof(CharacterAnimationTree);

public CharacterAnimationTree CharacterAnimationTree { get; private set; }

public override void _Ready()
{
    CharacterAnimationTree = GetNode<CharacterAnimationTree>(CharacterAnimationTreePath);
}

override-current

Proposed behaviour

// Character.cs
public CharacterAnimationTree CharacterAnimationTree { get; private set; }

public override void _Ready()
{
    CharacterAnimationTree = GetNode<CharacterAnimationTree>(nameof(CharacterAnimationTree));
}

override-new

@reyma24
Copy link
Author

reyma24 commented Sep 6, 2021

I have never had a case where I needed to edit the children.. I have always just reopened the scene, fixed the issue and moved on...This is another reason there are exported variables. That is where I do any changes needed....if someone implements this...it's, ofc, fine...but I feel like this is the wrong path and might teach bad habits. Feels very "corner case".

Here's my thinking, visualized with an example:
You can't add the same Destructible script to, for example, both Static and Kinematic bodies - they extend different classes.
However, this way you will get all the properties needed in the inspector.

The much better solution is to "extend" them by adding the same node:

the_same_script

Great! But now you can't see and edit those new properties after instancing this scene because the inspector shows only the properties from the classes above.

The solution: Editable Children, hence this proposal.


Edit: I created an alternative proposal for those prefering to change nested nodes with their own, custom scripts: #3264

@AttackButton
Copy link

AttackButton commented Dec 13, 2021

I am not a contributor, but I have never had a case where I needed to edit the children.. I have always just reopened the scene, fixed the issue and moved on...This is another reason there are exported variables. That is where I do any changes needed....if someone implements this...it's, ofc, fine...but I feel like this is the wrong path and might teach bad habits. Feels very "corner case".

This is not a "corner case", look at the likes in the op. I'm using this a lot in my project, in the level design. I'm using nodes from the base scene as a container for some other scenes that will be manipulated via script. The number and type of objects in the containers changes from instance to instance, as the level design requires by the context.

This feature is very important.

@Z3R0PTR
Copy link

Z3R0PTR commented Dec 30, 2021

Hi.

I think, this will be a nice feature. In my projects, I have thousands of Models (FBX-Files). All of them are represented as inherited scenes, where the top node is a simple spatial (3D) that contains one ore more (hierarchical) mesh instances.
So if I want to instance a model multiple times within a scene and want to change the material for some of them, i have to edit the children and set the new material for all the mesh instances. This is my biggest problem in level design.

@eerbin13
Copy link

I am currently using a component based code architecture model. Improved Editable Children would be magnificent.

I stumbled across this thread looking for a way to turn Editable Children on by default.

Perhaps a method exposed to GDscript where you could use it in an _on_ready() with the tool keyword at the top of the script?

I was looking for a function inside Node called set_editable_children(state: bool) before I did my Google deep dive.

@MatthijsMud
Copy link

This could also be useful for creating "slots" if the various ancestor nodes aren't shown in the hierarchy.

Granted, in case of Spatial and Node2D this could be worked around by creating nodes in the root that are targeted by RemoteTransform and RemoteTransform2D respectively.

For Control nodes there currently exists no such alternative (that I'm aware of). Yet one might be interested in adding items to a "deeply" nested Container representing the content (of a custom container built out of different nodes). An accordion could be an example of such a component.

This would allow viewing the results directly in the editor rather than having to rely on runtime (or Tool scripts) that move nodes around/create scene instances.

Current editable children situation With improvements to editable children
image image

@BlockyDK
Copy link

BlockyDK commented Sep 5, 2022

I'm not entirely sure if this fits here, but I've just run into a use case that I think touches upon this discussion.

Considering the node based structure of Godot, it seems most intuitive to me to give quick and easy control over which properties from children nodes will be visible from the root node when instanced in another scene. And of course while not actually exposing the children to the scene tree in order to keep everything modular and compact. While this functionality can be achieved using @tool, there might be a better way.

Consider this prefab I put together, it is made up of several parts, a stand, a rotating brace, a tilting lamp and finally a spotlight.

EditableChildren_01

in the tool script I allow the user to rotate and tilt the head of the lamp with a simple slider, and as this is custom behavior, the tool script made sense here. And it is all in one tidy little node.

EditableChildren_03

EditableChildren_02

However, I also wanted the ability to control the properties of the spotlight. This ended up resulting in a rather long tool script just exposing all of the properties from the spotlight. At first I only exposed a few (as seen above), but as time went on I found that I continually needed access to more and more of the light's properties.

In this scenario, I found myself wishing for the ability to easily select properties from the child and have them auto expose in the root when instantiated in another scene. Or even better, a one click solution to have all of the selected child's properties exposed. Two approaches came to mind, one where the user would right click on the property in the child and the option would pop up to expose that property in the root when instantiated.

EditableChildren_04

Another would be to add a checkbox in the properties for the child (so basically a property inherited from a basic node) that would just expose everything from that child in the root when instantiated.

Then ideally, in the case of this prefab for instance, when I select the instantiated prefab, over in the inspector, I should just see a dropdown for the spotlight child and all of it's properties (provided the box is checked under the spotlights properties from within the actual prefab's scene. Obviously this property would have to be excluded when exposing all the others.

That is just my two cents, I think it would work well with Godot's node based workflow.

@CookieBadger
Copy link

As a level editor, this is one of my main pain points of the Godot Engine. I have so many cases where I need to edit children, when making a level.

  1. There is no way to make children editable per default when instancing a scene (either global or per scene resource)
  2. There is no hotkey to make the children of the current node editable
  3. There is no way to hide any children, that you don't want to be exposed

@justinbarrett You don't have to be a contributor to come across many use cases for this. The moment you need two instances of a StaticBody with different Collision Shape sizes, you need to make children editable.

When you constantly have to instance new scenes, make their children editable, expand an abysmal hierarchy and find the one thing you are supposed to edit, this is detrimental for the workflow. More clicks, more searching, more typing on the level editor's end, which could be avoided, by the person who designed the feature, is always bad.

As an improvement, I think the easiest improvement could be adding the hotkey. Then, the solutions from @reyma24 's proposal would be a boon for level editing. Finally, @BlockyDK 's selectively exposed properties can be nice, but that would limit it to properties in the inspector, compared to using editor viewport gizmos for e.g. transformations.

@Vukbo
Copy link

Vukbo commented Dec 21, 2022

I totally agree with the proposal of @ExquisiterEmil.
This would be a tremendous level up regarding level design in the editor...

@realkotob
Copy link

I made a PR that partially addresses this, let me know what you think

godotengine/godot#70947

@CookieBadger
Copy link

@realkotob cool, from a level design standpoint, I can see this being very useful already! As a system designer, you often don't want the entire child node hierarchy visible to the level designer, so I am still of the opinion that something like solution 1 from the original proposal should be implemented as well.

@Huraqan
Copy link

Huraqan commented Jan 11, 2023

Noone seems to talk about the loss of data... (EDIT: you guys did talk about it, my bad.) I lost half a level because of editable children. I saved a scene of my level to load it in a new project and most of the polygon2D nodes got reset. Well, actually the saved scene lost most of the polygon data, kept only one PoolVector2Array and put it in every Polygon2D. This happened only the moment I saved the scene and reimported it...

@realkotob
Copy link

@ExquisiterEmil I fully agree with you that adding a system to expose properties selectively would be useful, I can work on that as a second step after the first PR is accepted :)

@realkotob
Copy link

@Huraqan The data loss is an important point but it is a completely different issue.

There is a different proposal for how to fix the data loss issue that I will try to start working on soon. #3152

@Huraqan
Copy link

Huraqan commented Jan 16, 2023 via email

@realkotob
Copy link

@Huraqan That issue is part of the losing data problem with editable children in instances, but yes you are right it is not related to saving and loading a scene using editable children.

Is there an existing issue for that? Otherwise we should create one

@Huraqan
Copy link

Huraqan commented Jan 18, 2023 via email

@FeldrinH
Copy link

If @reyma24's 'Editable In Parent' checkbox is ever implemented then I would propose that the data loss risk could be reduced by showing a warning/confirmation dialog whenever the user attempts to change a child node that is marked as editable in a way that would result in data loss. So for example if you attempt to rename an editable child the editor could show a popup that says 'Renaming this node will lose any changes made to its properties in instances of this scene' with 'Ok' and 'Cancel' buttons.

@frandmb
Copy link

frandmb commented May 20, 2023

This would be extremely useful for creating reusable components without cluttering the interface or needing the manual step of exposing the children when they have required configuration. Either exposing the children directly or displaying selected children's properties on the inspector itself for the parent node/scene view

@3lis4
Copy link

3lis4 commented Oct 10, 2023

I found myself wishing for the ability to easily select properties from the child and have them auto expose in the root when instantiated in another scene.

Being able, in a scene, to choose on every node which property should be shown on the root when this scene is instantiated in another scene seems very useful.

We could also add an annotation to be able to specify in the root script which child properties we want to expose. This would be the script equivalent of the editor UI "expose in parent" option.

In a similar issue we imagined a syntactic sugar to help with exposing child vars : #7803 (comment)

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.

This @export_child was supposed to be just a syntactic sugar replacing an export var and its setter in the root script, which has two drawbacks :

  • You have to make your root script a tool script if you want to see in-editor updates of the child export var
  • You loose the special edition tools of the child export var (like for creating a polygon for a CollisionPolygon2D)
  • You have to duplicate or loose the special export annotation, if you used one for the child variable (for instance, @export_multiline)

But @export_child could be great as the script equivalent of the editor "expose in parent" option.

There is also this interesting discussion on reddit: https://www.reddit.com/r/godot/comments/173zn9j/how_do_you_handle_encapsulation_and_passing_data/

@foolmoron
Copy link

Here's another use case in support of reyma's proposal 1 to mark nodes as editable in parent. I have a CollisionPolygon2D as part of a scene and I want to edit the polygon in each instance. If I just exposed the polygon property, I would have to edit the array manually, but if I can expose the entire node, then I get the nice editor controls.

Of course, the option to expose specific properties would be very useful too. But I think exposing nodes is more important since it covers all use cases.

image

@cursegate
Copy link

cursegate commented Jan 13, 2024

I agree that exposing child nodes to edit is the true solution to this problem.

We can use the editor to compose scenes without writing code, which is awesome. The problem is that we can't give a scene an intuitive API without code. A designer can't even tell if a scene's children require dependencies without enabling Editable Children and scrolling through.

I don't think the solution should be script-level on children, because a child node without a script attached can still have dependencies. Also, you might want to enable this feature for one node, but disable it for another node with the same script.

I don't think "Editable in Parent" is technically accurate, though. Something like "Editable in Instance" or "Exposed Outside Scene" sounds better to me.

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.