godotengine
Add the ability to expose nodes for direct access in instantiated scenes
Description
This pull request implements a feature that significantly enhances Godot's scene editing capabilities. It allows specific nodes within a scene to be exposed, making them visible and allowing their properties to be overridden when the scene is instantiated elsewhere. I believe it has the following benefits over "Editable Children".
Pros:
- Improved Scene Organization: By exposing only relevant nodes, the scene tree remains uncluttered, which is especially useful for creating reusable scenes (Custom containers, Generic windows).
- Property Overrides: Users can easily override node properties without modifying the base scene or creating a script specifically to modify child node properties.
- Reliability on Change: Child node placements and property overrides are preserved even when the instanced scene is updated, ensuring stability across scene changes.
- Simpler .tscn File: Only the overridden properties are saved in the .tscn file instead of the entire node, reducing file bloat.
- Imports: This PR adds the ability to expose nodes on import (or via -exp within Blender), for use when wanting to expose parts of a scene (such as the hand bone of a character for equipping).
Cons:
- Use of Unique Names: The current implementation references nodes by unique names, which may not be ideal. A future solution, like referencing nodes by unique ID (https://github.com/godotengine/godot/pull/86960), would be preferable for more robust and conflict-free node identification.
Example
For a simple scene like the following, we expose the Mesh node, the resulting tscn looks like this:
[node name="Scene0" type="Node3D"]
[node name="Mesh" type="MeshInstance3D" parent="."]
unique_name_in_owner = true
+ exposed_in_owner = true
mesh = SubResource("BoxMesh_vuc28")
In another scene we instantiate this simple scene and override the exposed node's mesh property to be a prism instead of a box:
.tscn with editable children and doing the same thing:
[node name="MainScene" type="Node3D"]
[node name="Scene0" parent="." instance=ExtResource("1_mbyq7")]
- [node name="Mesh" parent="Scene0" index="0"]
- mesh = SubResource("PrismMesh_g1soo")
- [node name="Child1" type="Node3D" parent="Scene0/Mesh" index="0"]
- [editable path="Scene0"]
.tscn with this pr and using exposed nodes:
[node name="MainScene" type="Node3D"]
[node name="Scene0" parent="." instance=ExtResource("1_mbyq7")]
+ [node name="Child1" type="Node3D" parent="Scene0/%Mesh" index="0"]
+ [override path="Scene0/%Mesh"]
+ mesh = SubResource("PrismMesh_g1soo")
The .tscn is simpler and more straightforward with whats happening. We only save the information relevant to this scene, no need to save the Mesh node from the instantiated scene just to override the mesh. We only save whats overridden/changed.
Since we reference the parent of Child1 and the override via the unique name of the exposed node we gain the following benefits if the Exposed node is moved/modified within the subscene:
- Child1 will not be orphaned and will remain a child of the exposed Mesh node
- Exposed Mesh node will retain the prism mesh override regardless of the base scene's mesh property
TODO
- Continue to test
I think that this pr addresses the following proposals:
- Closes https://github.com/godotengine/godot-proposals/issues/3248
- Closes https://github.com/godotengine/godot-proposals/issues/4265
- Closes https://github.com/godotengine/godot-proposals/issues/9660
- Closes https://github.com/godotengine/godot-proposals/issues/5475
- Closes https://github.com/godotengine/godot-proposals/issues/8266
- Closes (?) https://github.com/godotengine/godot-proposals/issues/7803
- Closes (?) https://github.com/godotengine/godot-proposals/issues/9536
This will work with imported gltf scenes?
I havn't looked into that aspect, but would be a nice feature to add to this as well
EDIT 11/2/2024:
Yes!
Also please update your branch by rebasing instead of merging, important skill to get used to with contributing, see the pr workflow for details
You have reset your branch and this closes the PR, if you update your branch this can be reopened
Sorry, still trying to get a grip on correct way of updating my repo from main while keeping my changes. I've pushed my new changes up. This includes a somewhat functional version of this pr.
Some notes from testing:
- enabling the Expose option will create 2 undo actions. You should make it single action (it can be achieved by nesting actions)
- when you have exposed node and click
%, it will disable exposition and you can't undo it - why is even Expose tied to Unique Name? exposing a node does not mean the user wants to access it in code
- when you add nodes directly under instance, they go into void and only appear when you enable editable children. This is wrong. Looks like they get wrong owner assigned
- when you duplicate exposed node, the duplicate is also exposed and still belongs to the instance. The exposition should be stripped and correct owner assigned (see above point)
- instead of saying "Hide Node in Instantiated Scenes", using "Unexpose" could be more clear
Some notes from testing:
* enabling the Expose option will create 2 undo actions. You should make it single action (it can be achieved by nesting actions) * when you have exposed node and click `%`, it will disable exposition and you can't undo it * why is even Expose tied to Unique Name? exposing a node does not mean the user wants to access it in code * when you add nodes directly under instance, they go into void and only appear when you enable editable children. This is wrong. Looks like they get wrong owner assigned * when you duplicate exposed node, the duplicate is also exposed and still belongs to the instance. The exposition should be stripped and correct owner assigned (see above point) * instead of saying "Hide Node in Instantiated Scenes", using "Unexpose" could be more clear
Hi KoBeWi, thanks so much for doing some testing! Here's some thoughts on your notes.
- I'll definitely look into this, makes sense.
- I'll look into this as well :)
- This is just the way I approached this issue, it makes for an easy way to save nodes under a unique name path. Definitely could be a different way to approach this problem.
- Hmm, I'm not certain what situation you mean by this, but ill play around and see if i cant reproduce.
- Will look into this as well.
- Agreed, will make those changes.
Really appreciate the feedback!
You broke your rebase, to fix please do:
git reset --hard 910afbc
And then rebase again
You're run the wrong version of clang-format (or need to run it in the first place) as you have a lot of format errors (including unrelated changes to style that break the style)
I tried to address many of the comments about functionality. I also touched up use cases for cut/copy/paste etc. However I'm not a fan of making changes like this to the code. Unsure of how best to approach preventing copy/paste/duplicate of exposed nodes. (if i should at all)
https://github.com/Yahkub-R/godot/commit/fe42bdd9a03e9a1f70737ba4d2225e5a0f123564#r140697629
case TOOL_COPY: {
if (!edited_scene || !_validate_no_foreign()) {
break;
}
This is still a draft PR so until the author makes it ready for review it's unlikely to be reviewed, if the author feels it's ready it can get looked into in the next cycle
Here's a quick little demo project that has control and 3d instanced scenes with exposed nodes.
main.tscn
[gd_scene load_steps=8 format=3 uid="uid://dab1373s4j0vv"]
[ext_resource type="PackedScene" uid="uid://bv2pt3xsxwkk2" path="res://control_instance.tscn" id="1_wac7c"]
[ext_resource type="PackedScene" uid="uid://cdfbiyclq335g" path="res://3d_instance.tscn" id="2_6dtlv"]
[ext_resource type="PackedScene" uid="uid://i3op3urt6o3x" path="res://mesh/beanie/scene.gltf" id="3_kstod"]
[ext_resource type="PackedScene" uid="uid://dnmile0s1wtd1" path="res://mesh/tophat/scene.gltf" id="4_bptxn"]
[sub_resource type="ProceduralSkyMaterial" id="ProceduralSkyMaterial_cvfnl"]
sky_horizon_color = Color(0.64625, 0.65575, 0.67075, 1)
ground_horizon_color = Color(0.64625, 0.65575, 0.67075, 1)
[sub_resource type="Sky" id="Sky_50h75"]
sky_material = SubResource("ProceduralSkyMaterial_cvfnl")
[sub_resource type="Environment" id="Environment_e5nhw"]
background_mode = 2
sky = SubResource("Sky_50h75")
tonemap_mode = 2
glow_enabled = true
[node name="Node3D" type="Node3D"]
[node name="DirectionalLight3D" type="DirectionalLight3D" parent="."]
transform = Transform3D(0.968469, -0.215128, 0.125649, -0.0705338, 0.246945, 0.966459, -0.238941, -0.944848, 0.223985, 0, 0, 0)
shadow_enabled = true
[node name="WorldEnvironment" type="WorldEnvironment" parent="."]
environment = SubResource("Environment_e5nhw")
[node name="Camera3D" type="Camera3D" parent="."]
transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1, 4)
[node name="Login" parent="." instance=ExtResource("1_wac7c")]
offset_left = 36.0
offset_top = 22.0
offset_right = 246.0
offset_bottom = 170.0
[node name="Label" type="Label" parent="Login/%Header" index="0"]
layout_mode = 2
text = "Login Form"
[node name="HBoxContainer1" type="HBoxContainer" parent="Login/%Content" index="0"]
layout_mode = 2
[node name="user" type="Label" parent="Login/%Content/HBoxContainer1"]
layout_mode = 2
size_flags_horizontal = 3
text = "Username"
horizontal_alignment = 2
[node name="LineEdit" type="LineEdit" parent="Login/%Content/HBoxContainer1"]
layout_mode = 2
size_flags_horizontal = 3
[node name="HBoxContainer0" type="HBoxContainer" parent="Login/%Content" index="1"]
layout_mode = 2
size_flags_vertical = 3
[node name="pass" type="Label" parent="Login/%Content/HBoxContainer0"]
layout_mode = 2
size_flags_horizontal = 3
text = "Password"
horizontal_alignment = 2
[node name="LineEdit" type="LineEdit" parent="Login/%Content/HBoxContainer0"]
layout_mode = 2
size_flags_horizontal = 3
[node name="LoginButton" type="Button" parent="Login/%Content" index="2"]
custom_minimum_size = Vector2(100, 0)
layout_mode = 2
size_flags_horizontal = 4
size_flags_vertical = 3
text = "Login"
[node name="Dialog" parent="." instance=ExtResource("1_wac7c")]
offset_left = 252.0
offset_top = 21.0
offset_right = 466.0
offset_bottom = 170.0
[node name="Label" type="Label" parent="Dialog/%Header" index="0"]
layout_mode = 2
text = "Dialog"
[node name="Label" type="Label" parent="Dialog/%Content" index="0"]
layout_mode = 2
text = "A dialog has appeared..."
[node name="ReferenceRect" type="ReferenceRect" parent="Dialog/%Content" index="1"]
custom_minimum_size = Vector2(0, 40)
layout_mode = 2
[node name="HBoxContainer" type="HBoxContainer" parent="Dialog/%Content" index="2"]
layout_mode = 2
[node name="Button" type="Button" parent="Dialog/%Content/HBoxContainer"]
custom_minimum_size = Vector2(100, 0)
layout_mode = 2
size_flags_horizontal = 4
size_flags_vertical = 3
text = "Ok"
[node name="Button2" type="Button" parent="Dialog/%Content/HBoxContainer"]
custom_minimum_size = Vector2(100, 0)
layout_mode = 2
size_flags_horizontal = 4
size_flags_vertical = 3
text = "Cancel"
[node name="Gobot Beanie" parent="." instance=ExtResource("2_6dtlv")]
transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 2, 0, 0)
[node name="Beanie" parent="Gobot Beanie/%Hat_Mount" index="0" instance=ExtResource("3_kstod")]
transform = Transform3D(0.007, 0, 0, 0, 0.005, 0, 0, 0, 0.005, 0, 0, 0)
[node name="Gobot" parent="." instance=ExtResource("2_6dtlv")]
[node name="Gobot Top Hat" parent="." instance=ExtResource("2_6dtlv")]
transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -2, 0, 0)
[node name="TopHat" parent="Gobot Top Hat/%Hat_Mount" index="0" instance=ExtResource("4_bptxn")]
transform = Transform3D(0.435488, 7.54289e-23, -8.43668e-22, 2.52435e-29, 0.433758, 0.0387805, 8.47033e-22, -0.0387805, 0.433758, 3.87477e-16, 0.111278, 0.00989257)
3d_instance.tscn
[gd_scene load_steps=3 format=3 uid="uid://cdfbiyclq335g"]
[ext_resource type="ArrayMesh" uid="uid://cgv8fkqyfb4hk" path="res://mesh/godot_plush V2_Cube_0012.res" id="1_ptpo3"]
[ext_resource type="Script" path="res://spinner.gd" id="1_wotwy"]
[node name="3dInstance" type="Node3D"]
[node name="Plush" type="MeshInstance3D" parent="."]
transform = Transform3D(-6.98885, 0, -6.10985e-07, 0, 6.98885, 0, 6.10985e-07, 0, -6.98885, 0, -0.363793, 0)
mesh = ExtResource("1_ptpo3")
script = ExtResource("1_wotwy")
[node name="Hat_Mount" type="Node3D" parent="Plush"]
unique_name_in_owner = true
exposed_in_owner = true
transform = Transform3D(-0.143085, 0, 1.25089e-08, 0, 0.143085, 0, -1.25089e-08, 0, -0.143085, 0, 0.154249, 0)
control_instance.tscn
[gd_scene load_steps=3 format=3 uid="uid://bv2pt3xsxwkk2"]
[sub_resource type="StyleBoxFlat" id="StyleBoxFlat_fg638"]
bg_color = Color(0.145098, 0.145098, 0.145098, 1)
corner_radius_top_left = 5
corner_radius_top_right = 5
corner_radius_bottom_right = 5
corner_radius_bottom_left = 5
[sub_resource type="StyleBoxFlat" id="StyleBoxFlat_qcsvv"]
bg_color = Color(0.195, 0.20425, 0.75, 1)
corner_radius_top_left = 5
corner_radius_top_right = 5
[node name="Panel" type="PanelContainer"]
custom_minimum_size = Vector2(100, 100)
offset_right = 51.0
offset_bottom = 37.0
size_flags_horizontal = 3
size_flags_vertical = 3
theme_override_styles/panel = SubResource("StyleBoxFlat_fg638")
[node name="VBoxContainer" type="VBoxContainer" parent="."]
layout_mode = 2
[node name="PanelContainer" type="PanelContainer" parent="VBoxContainer"]
layout_mode = 2
theme_override_styles/panel = SubResource("StyleBoxFlat_qcsvv")
[node name="Header" type="MarginContainer" parent="VBoxContainer/PanelContainer"]
unique_name_in_owner = true
exposed_in_owner = true
layout_mode = 2
theme_override_constants/margin_left = 5
theme_override_constants/margin_top = 5
theme_override_constants/margin_right = 5
theme_override_constants/margin_bottom = 5
[node name="MarginContainer" type="MarginContainer" parent="VBoxContainer"]
layout_mode = 2
theme_override_constants/margin_left = 5
theme_override_constants/margin_top = 5
theme_override_constants/margin_right = 5
theme_override_constants/margin_bottom = 5
[node name="Content" type="VBoxContainer" parent="VBoxContainer/MarginContainer"]
unique_name_in_owner = true
exposed_in_owner = true
layout_mode = 2
Tested this again (tbh I forgot I already tried it before) and most of the issues I listed were addressed. However:
- I see the exposed node is still tied to unique names. Though I noticed that it's used when saving in scene; it's clever, maybe it's fine like this, idk
- When trying to expose node and a unique node with this name already exists, you get warning spam and nothing happens
Compare with setting unique name, which shows an error dialog instead.
- Why exposed nodes are gray? It's confusing (it's the same color as disabled nodes). They should keep default color in their original scene and use yellow color in instancing scene (like editable children)
Tested this again (tbh I forgot I already tried it before) and most of the issues I listed were addressed. However:
* I see the exposed node is still tied to unique names. Though I noticed that it's used when saving in scene; it's clever, maybe it's fine like this, idk
Yeah this is just my way of determining where to place the child nodes when saving, this could be solved by something like #86960 if we were able to reference nodes by a unique id.
* When trying to expose node and a unique node with this name already exists, you get warning spam and nothing happens Compare with setting unique name, which shows an error dialog instead.
Fixed :)
* Why exposed nodes are gray? It's confusing (it's the same color as disabled nodes). They should keep default color in their original scene and use yellow color in instancing scene (like editable children)
I was flip flopping between the two, so happy for the feedback, ill move them back to being warning instead of the disabled color as i do think that makes more sense to the user.
ill move them back to being warning instead of the disabled color as i do think that makes more sense to the user.
I also mentioned that the color should not appear in the original scene. The node already has an icon and yellow color is reserved for nodes from other scenes. It does not make sense for a node in its same scene.
I just found a major bug. Properties of exposed nodes are not saved. If you instance a scene with exposed node and modify the exposed node, the changes will be lost.
I just found a major bug. Properties of exposed nodes are not saved. If you instance a scene with exposed node and modify the exposed node, the changes will be lost.
This is also something I noticed, and discussed a possible solution here I'm not sure having this 'override' system as part of this pull request makes a lot of sense. Instead maybe finding a way to make the properties unable to be edited?
The 'problem' at least with my implementation is the exposed nodes dont get saved in the scene they are instanced in like the do with editable children.
Well properties can be made uneditable in the inspector, but editors are a bigger problem. Currently there is no concept of readonly nodes for 2D and 3D editors. The closest are locked nodes, but you are still able to change their visibility etc. So if you want the nodes uneditable, it has to be handled in many places.
Maybe it's possible to take advantage of unique names and make the node properties saved?
Well properties can be made uneditable in the inspector, but editors are a bigger problem. Currently there is no concept of readonly nodes for 2D and 3D editors. The closest are locked nodes, but you are still able to change their visibility etc. So if you want the nodes uneditable, it has to be handled in many places.
Maybe it's possible to take advantage of unique names and make the node properties saved?
Yeah thats exactly what I was going for in that discussion with overrides, having the ability to do something like this:
otherwise disallowing them to be edited in the inspector and/or clearly marking the properties as 'won't be saved' might be the way to go. Wasn't sure if this override system bloated the PR too much beyond the scope of what I wanted to achieve.
Wasn't sure if this override system bloated the PR too much beyond the scope of what I wanted to achieve.
Some way to prevent data loss is required for this feature. Users will expect that exposed nodes work the same as editable children. And tbh there is not much value in exposing a node if you can't edit it.
Wasn't sure if this override system bloated the PR too much beyond the scope of what I wanted to achieve.
Some way to prevent data loss is required for this feature. Users will expect that exposed nodes work the same as editable children. And tbh there is not much value in exposing a node if you can't edit it.
Fair enough, I'll work on ironing out the kinks with the override as well then.
Definitely disagree with that last bit, to me this was all in order to solve the issue of creating custom containers. Like in my first post of the discussion, having the ability to construct re-usable ui containers or expose parts of a node tree that I need to parent a node to is super helpful on its own. But I do agree being able to edit the exposed nodes as well is a great plus.
You merged your branch instead of doing a force push, please use git push --force instead in the future
Seems like overrides are working correctly, but they are stored twice for some reason:
Updated the original post to hopefully better explain what I'm trying to achieve and other issues I think this resolves. I will continue to cleanup this pr and slim down the number of changes it requires.
Edit: currently some bugs with saving, working on them
I believe I fixed some of the saving issues and updated it to display exposed nodes in correct hierarchical order:
base spinner scene:
Instantiated:
If an exposed node is a child of non-exposed node, it will not appear in the instancing scene.
If an exposed node is a child of non-exposed node, it will not appear in the instancing scene. 
- [x] If you enable editable children in exposed node, but its parent also has enabled editable children, the editable state of exposed node is not saved in scene.
EDIT:
- [x] Also editable children makes exposed node not saved. They don't save as overrides nor as regular editable children.