Godot: Have add_child return a value.

[Sslaxx]

Issue description (what happened, and what was expected):
As per http://docs.godotengine.org/en/latest/classes/class_node.html?highlight=add_child currently add_child currently returns void when called. I think it might be helpful for add_child to instead return a value.

I'm thinking it should:

• Return the absolute path of the added node if valid/correct. So if the new parent was "/root/nodRootMain" and the function was add_child (SceneMe) where the root node of SceneMe was "a_scene", it'd return "/root/nodRootMain/a_scene".
• Alternatively, return the resultant node object.
• Return something else (null, an ERR_ condition etc) if invalid.

[vitvakatu]

I'm against it.
1) Returning the absolute path of the added node seems cool, but if you think about it for a time, you'll probably see, that you do not need it. Give me an example of code, where I can use that.
2) You already have a child node object, haven't you? Otherwise, how you can attach child without creating it before?
3) What can go wrong way when adding child to node? I can't imagine this situation.

[Zylann]

I'm against it too, especially the path, because it involves computations I don't need.
Note there is get_path() for this.

The second proposal is useless because you already have the node, except for introducing a more fluent interface, but it's mostly syntactic sugar.

The third proposition seems a bit interesting at first look, because you don't want the name of your node to collide with any other child and that could indeed be considered an error (well, if you care about it).
But even that is currently not needed, because Godot renames your node in this case:

@Node@2
@Node@3
@Node@4
@Node@5

Side note: this also means Godot compares every other child's names before adding your node, so if you frequently add nodes to a parent that has say, 1000 children, it looks like you'll have a lot of string comparisons going on. Fortunately, because they are StringNames (and not Strings), these comparisons should be as fast as comparing two integers https://github.com/godotengine/godot/blob/master/core/string_db.h#L106

[eon-s]

I like to get error codes but not sure if will be useful here.

With the upcoming of C#, where people is afraid of variables and love chaining, some may like to do
parent.add_child(packed_scene.instance()).do_something()

In this case, should add_child return the child or the parent?

[volzhs]

parent.add_child(packed_scene.instance()).do_something() seems unclear code to me.

[eon-s]

@volzhs I agree on the lack of clarity.

And what about Visual Script? Could it get some benefit from returning something in this method?

[Zylann]

@eon-s this is called "fluent interface", and you can do this with any language. If you were referring to LINQ, not all game developers like it (I am one of them, for technical reasons rather than the syntax).

[bojidar-bg]

@eon-s VisualScript would benefit even less from such changes, since you already can hook the newly-added child into whatever other function you have as well.

[Oranjoose]

I am definitely for having add_child return the child being added.

For example:
add_child(load("res://MyScene.tscn").instance()).position = Vector2(100, 150) #no variables

That might look super ugly to you, but there are plenty in the open-source JS game engine community (Phaser, etc.) looking into Godot, who find chaining very natural. Those who hate it: no skin off their backs. Keep using it as if it's void.

@eon-s I agree that VisualScript benefits from it. Taking Unreal's Blueprints for example, they add output pins whenever possible, even if it would seem superfluous. For example, the Blueprint set nodes return what they are setting:

Again, this might seem superfluous, because why can't they just use the same wire for the value that they used for the input pin on the set node? Because sometimes it keeps the graph more organized when you can start a new wire at the output pin on the set node, and again, for those who don't want to use it: no skin off their backs.

And for what it's worth, here are other APIs with add_child or add_child-like functions that return the child being added:

• Flash: http://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/display/DisplayObjectContainer.html#addChild%28%29
• Phaser: http://phaser.io/examples/v2/sprites/child-sprites
• CreateJS: http://createjs.com/docs/easeljs/classes/Stage.html#method_addChild
• Unreal: https://docs.unrealengine.com/latest/INT/API/Editor/Kismet/FSCSEditorTreeNode/AddChild/2/index.html
• JQuery: http://api.jquery.com/add/
• PIXI: http://www.goodboydigital.com/pixijs/docs/classes/DisplayObjectContainer.html#method_addChild
• Processing: https://processing.org/reference/XML_addChild_.html

[ghost]

One benefit I see is that you can create instance and adding it as child in the same line. This code seems clear to me:

var label = add_child(Label.new())

I personally wouldn't set a property or call a method on that line, though.

[ghost]

Still reproducible/relevant in 3.1 master 9ad1800 : add_child doesn't return a value.

[girng]

@ghost, LOL, took me a while to understand what that will return. now that i think of it.. i still don't know

[dkotfis]

Still an issue? I'm looking to add this feature.

[hilfazer]

What i find useful in this proposal isn't chaining but this:
var child = add_child(load("res://MyScene.tscn").instance())

Now, when i need to do something with a child i'm writing two lines - one with child declaration, the other with instance(). With a return value in add_child it would be just one line.

[Xrayez]

Nodoby has yet mentioned about countless warnings that will appear due to Return value discarded in existing projects (though I find those mostly useless so I just disable them for now).

Some examples:

node = add_child(node) # reuse the same node to avoid the warning

var _child = add_child(node) # use underscore discard syntax explicitly

# warning-ignore:return_value_discarded
add_child(node)

Also see https://github.com/godotengine/godot/pull/30628#issuecomment-512642195.

[Xrayez]

Many who support adding this functionality here basically write this as https://github.com/godotengine/godot/issues/6521#issuecomment-559000937:

var child = add_child(load("res://MyScene.tscn").instance())

... and this smells like an XY problem. 🙂

So perhaps it wouldn't hurt adding something akin to instance_child(scene) for
people coming from GameMaker https://github.com/godotengine/godot/pull/33926#issuecomment-559020825 (or as a method which will be used quite often by many Godoter in fact):

var node = instance_child(load("res://MyScene.tscn"))

The intention is fairly explicit, and the warning about discarded value would be legit now.

[Sslaxx]

@Xrayez - my original intent (and still primary) is to see error-checking added to add_child, but that'd certainly resolve a lot of the issues people are bringing up here. If an instance_child was added, then add_child could simply return a true or false if the node was added correctly.

As for the warnings caused, maybe this should be marked as a breaking compatibility issue.

[akien-mga]

To bug triagers: please don't add the "junior job" label to feature requests which are not consensual. This has not been implemented not because we lack manpower, but because we lack consensus on it being a good change - so we shouldn't encourage new contributors to work on them.

[nathanfranke]

Consistency in code

Consistency in code

While this doesn't break compatibility, it also allows code to be inconsistent. Right now, there is pretty much one way to do every simple tree action. Breaking that would make tutorials less consistent, and lead to confusion for new developers. "Why does this return a value? I already have this variable". It may lead to wtf code like child = parent.add_child(child).

Consistency in the engine

Consistency in the engine

If we were to do this, we would also need to implement other returns for methods like remove_child() for the sake of consistency. For example, you might do parent.remove_child(child).queue_free(). (However, in this instance, you could just use queue_free()).

I should note that many APIs (Although not Godot's) often return booleans after operations. This may make sense for remove_child() (if the child isn't there to begin with), but add_child() almost never has problems (And therefore should have exceptions instead of returning a value _every time_.

What does this even do?

What does this even do?

I agree with volzhs. It is difficult to know what this code does; Many APIs return values for the sake of compact code, but there is no defined convention. jQuery also has a system of hierarchy (HTML) and you can _definitely_ make compact code... But almost all operations return the parent. $("#parent").append(child).attr("id") // parent

---

Lastly, I reference this image from Will your contribution be merged? Here's how to tell

[Sslaxx]

See #33938 and #33974. At this point add_child might benefit from just returning a bool if it added the child successfuly or not, but that might be part of a larger argument about error-checking.

[akien-mga]

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!