Improve error message for Node.set_owner
#79000
Conversation
AThousandShips
force-pushed
the
owner_error
branch
from
6e3a844 to
dd43bc4
Compare
AThousandShips
force-pushed
the
owner_error
branch
from
dd43bc4 to
0183340
Compare
akien-mga
added
the
cherrypick:4.1
| The node owner. A node can have any other node as owner (as long as it is a valid parent, grandparent, etc. ascending in the tree). When saving a node (using [PackedScene]), all the nodes it owns will be saved with it. This allows for the creation of complex [SceneTree]s, with instancing and subinstancing. | ||
| [b]Note:[/b] If you want a child to be persisted to a [PackedScene], you must set [member owner] in addition to calling [method add_child]. This is typically relevant for [url=$DOCS_URL/tutorials/plugins/running_code_in_the_editor.html]tool scripts[/url] and [url=$DOCS_URL/tutorials/plugins/editor/index.html]editor plugins[/url]. If [method add_child] is called without setting [member owner], the newly added [Node] will not be visible in the scene tree, though it will be visible in the 2D/3D view. | ||
| The node owner. A node can have any ancestor node as owner (i.e. a parent, grandparent, etc. node ascending in the tree). This implies that [method add_child] should be called before setting the owner, so that this relationship of parenting exists. When saving a node (using [PackedScene]), all the nodes it owns will be saved with it. This allows for the creation of complex scene trees, with instancing and subinstancing. | ||
| [b]Note:[/b] If you want a child to be persisted to a [PackedScene], you must set [member owner] in addition to calling [method add_child]. This is typically relevant for [url=$DOCS_URL/tutorials/plugins/running_code_in_the_editor.html]tool scripts[/url] and [url=$DOCS_URL/tutorials/plugins/editor/index.html]editor plugins[/url]. If a new node is added to the tree without setting its owner as an ancestor in that tree, it will be visible in the 2D/3D view, but not in the scene tree (and not persisted when packing or saving). |
There was a problem hiding this comment.
The last sentence is indeed true and a little more detailed than the previous one but it still describes only a subset of all cases when a node will end up not visible in the scene tree dock. A node with an owner set properly can still end up not visible in the scene tree dock.
E.g. for such hierarchy:
┖╴root (owner=null)
┠╴a (owner=root)
┃ ┖╴b (owner=a)
┖╴c (owner=null)
┖╴d (owner=root)
nodes b, c, d won't be visible in the scene tree dock (b and d even despite having an owner set properly).
(b would be visible if a would be an instanced scene with editable children enabled)
There was a problem hiding this comment.
Do you have a suggestion for improved wording that would cover this?
There was a problem hiding this comment.
Sorry for the late reply here: same as you, can't think about a simple one, it would indeed be probably more confusing. And again, the current explanation is correct, it just doesn't cover all possible cases.
Probably the simplest way to explain how it works would be by example, kinda like above. 🤔
|
Thanks! I went ahead and merge the current version which is already a major improvement. We can fine tune the explanation of edge cases as pointed out by @kleonc in a follow-up, so to be fair I'm not sure how to describe it without being more confusing that necessary. |
|
Thank you! Agreed was going to suggest merging this and leaving the rest to further work |
YuriSizov
removed
the
cherrypick:4.1
|
Cherry-picked for 4.1.1. |
Provides a less cryptic message than
Condition "!owner_valid" is true.While this is documented clearly I feel a clearer error message helps greatly here, as the check is for an unknown variable (from the perspective of a user) unlike something like
ERR_FAIL_COND(p_owner == this)where the condition speak for itself (and users don't always look at the documentation when facing an error either, though arguably they should that's no guarantee, and it's easy to explain here)