Godot-proposals: Add a method for instancing of nodes to be immediately added to the scene tree

[Xrayez]

Related feature proposals: #349, #435, godotengine/godot#6521.

---

Describe the project you are working on:
Turn-based artillery combat game. Also imagine bullet-hell-like games perhaps.

Describe the problem or limitation you are having in your project:
I have many cases where I need to create nodes to be added to scene tree while instancing them:

func explode():
    var explosion = load("res://explosion.tscn").instance()
    add_child(explosion)
    explosion.global_position = impact.global_position

```gdscript
func launch(p_weapon):
if p_weapon == "missile":
var missile = load("res://missile.tscn").instance()
add_child(missile)
missile.global_position = character.global_position
# ... and many others

Some usages lead to annoying programming errors which could be easily avoided, see https://github.com/godotengine/godot-proposals/issues/260#issuecomment-559559176:
```gdscript
var bomb = preload("res://bomb.tscn").instance()
bomb.fuse.wait_time = 5.0 # error: Invalid set index `time` on base `null`
add_child(bomb)

Describe how this feature / enhancement will help you overcome this problem or limitation:
It would better to have a shortcut for these particular use cases, something like:

var missile = preload("res://missile.tscn").instance_as_child(self)
missile.global_position = character.global_position

For those who like one-liners and use global classes with static methods in them:

Explosion.get_scene().instance_as_child(self).global_position = impact.global_position

Some common programming mistakes can be avoided:

var bomb = preload("res://bomb.tscn").instance_as_child(self)
# OK to reference onready vars now since they were initialized immediately.
bomb.fuse.wait_time = 5.0

Show a mock up screenshots/video or a flow diagram explaining how your proposal will work:

Describe implementation detail for your proposal (in code), if possible:

1. godotengine/godot#33974 - PackedScene built-in
2. godotengine/godot#33938 - Node built-in.

Alternative proposals which could replace/help with this one:

• https://github.com/godotengine/godot-proposals/issues/260#issuecomment-560004979 - godotengine/godot#23101

If this enhancement will not be used often, can it be worked around with a few lines of script?:
Yes, but I really don't want to reinvent the wheel across several projects. This could alleviate some of the prototyping hurdles.

Is there a reason why this should be core and not an add-on in the asset library?:
Child addition almost always follows scene instantiation in most use cases, which justifies the existence for such a method to be part of the core.

Quote from some documentation request godotengine/godot-docs#3338:

This is really something very basic and a incredibly common operation even a beginner would want to do.

The addition is quite small, the burden of having to go to the asset library in hopes that you find such utility function (by chance) outweighs the potential shortcomings of having to maintain this in the core.

Some community usages to justify this more objectively:

• spawn_instance. The implementation is quite reminiscent to godotengine/godot#33974.

[Xrayez]

Some possible names for the method:

1. add_child_scene.
2. add_child_from_scene.
3. instance_child.
4. instance_child_from_scene. instance implies scene.
5. new_child.
6. new_child_from_scene.

[hilfazer]

1. instantiate_child

Since we're moving to instantiatefor Godot 4.0: https://github.com/godotengine/godot/issues/16863

instantiateimplies it's a Node made from a scene and childimplies it's getting parented.

[Xrayez]

Yeah, in fact I think it would be better to name this as new_child though as to allow both scene instantiation and an empty node creation (to create node hierarchies more easily from code), see https://github.com/godotengine/godot/pull/33938#issuecomment-559182963.

[bojidar-bg]

_Suggestion:_

Maybe the convenience method could be part of PackedScene instead of Node?
It could look something like:

func _on_Timer_timeout():
    var enemy = preload("res://enemy.tscn").instantiate_as_child($EnemiesContainer)
    enemy.global_position = $SpawnPoint.global_position
# As opposed to:
func _on_Timer_timeout():
    var enemy = $EnemiesContainer.new_child(preload("res://enemy.tscn")
    var enemy = $EnemiesContainer.instantiate_child(preload("res://enemy.tscn")
    enemy.global_position = $SpawnPoint.global_position
# Current:
func _on_Timer_timeout():
    var enemy = preload("res://enemy.tscn").instance()
    $EnemiesContainer.add_child(enemy)
    enemy.global_position = $SpawnPoint.global_position

_Rationale:_

Currently, there is one thing in Node which knows about PackedScene: DUPLICATE_USE_INSTANCING.
It uses PackedScene only in the implementation, however, and not in the interface.
If an instantiate_child/new_child method is added to Node, it would make a bi-directional reference between the Node and PackedScene interfaces.

[rainlizard]

A problem with calling it new_child is it's a direct synonym of add_child, see how similar the two sound? add_child, new_child, that in itself would be confusing to newbies.

I like instance_child (or instantiate_child) because it encapsulates that it's performing two actions with its two words. instance() and add_child(), the two words directly correspond to those two lines, which is beautiful.

That's my 2 cents, in the end I honestly wouldn't care what it's called. Less typing is a very good point after all.

Sadly, after my conversation with groud I don't believe he'd allow it in because it seems to be very important to him that each line of code in GDScript only performs one action. https://github.com/godotengine/godot/pull/33926#issuecomment-559041452
Which is too bad, GameMaker's GML is still beating GDScript on the front of ease of use. I'm very confident that having a function that does both of these things would be used by everyone all the time, it would absolutely be the default usage.

As it stands, when spawning a scene newbies are required to understand and distinguish two different concepts of "instancing" and "child adding" (I was certainly confused at first). However, give them instance_child and they would not be required to learn the two concepts right away as the actions are performed automatically. Thus making Godot easier.

[girng]

This can obviously be done by creating a wrapper func. However, if added to the core and the performance is greater than a wrapper func (which I think it will be?).. my vote would be for instance_child.

I have a skill system where you can link skills (support, active, etc). Explicitly setting the position on an extra line can be annoying. It's not a big deal to me because I've adapted to it over the years (and also need to set other properties like angle), but I do share the OP's concern.

[groud]

Sadly, after my conversation with groud I don't believe he'd allow it

I am not the only one to decide. ^^ It's my opinion, but if other contributors disagree it can be accepted anyway.

But regarding this feature, the same applies here. It proposes a new function only to avoid three lines of code, that are not used that often. I believe explicit APIs, with single responsibility functions are a lot better than APIs that provides you with functions to write code with less lines. The fact that we have hard time naming the function is basically a proof of the fact this kind of function is a bad idea, as it is hard to describe in simple terms what this function is doing.

[Xrayez]

@bojidar-bg I understand the logic behind decoupling Node and PackedScene, but both are part of the scene infrastructure. Idiomatically, PackedScene should also be part of the core. The whole engine is a coupled beast.

It uses PackedScene only in the implementation, however, and not in the interface.
If an instantiate_child/new_child method is added to Node, it would make a bi-directional reference between the Node and PackedScene interfaces.

What if new_child could accept Variant which could be casted to PackedScene in the implementation? (if that's even an initial concern).

[Xrayez]

It proposes a new function only to avoid three lines of code, that are not used that often.

Look at add_child_below_node implementation (even with quite similar branching as in proposed new_child). (godotengine/godot#4652).

I don't think I'll ever use the above method, maybe once in a year under very specific use cases, yet it was merged and exposed to GDScript to fix a local bug as it seems.

[groud]

I don't think I'll ever use the above method, maybe once in a year under very specific use cases, yet it was merged and exposed to GDScript to fix a local bug as it seems.

Yeah, I would personally not have accepted it.

_Edit: but it was years ago, I was not even here to discuss it ^^_

[Xrayez]

A problem with calling it new_child is it's a direct synonym of add_child, see how similar the two sound? add_child, new_child, that in itself would be confusing to newbies.

Well yeah, if you look at ItemList's add_item, it doesn't add an existing item, it creates a new one, so go figure now.

I'm fine reverting to instantiate_child, makes it +1 points to "single responsibility functions". 🙂

[Xrayez]

Some more thoughts/points.

I recall situations where node/scene functionality depends on it being added to the scene tree (like onready var node references), I'd oftentimes forget to actually add the instanced scene:

# bomb.gd
class_name Bomb extends RigidBody2D
onready var fuse: Timer = $fuse

var bomb = preload("res://bomb.tscn").instance()
bomb.fuse.wait_time = 5.0 # error: Invalid set index `time` on base `null`
add_child(bomb)

add_child should be called immediately here for the fuse timer to be initialized _on_ready(). I know there are a number of ways to deal with node references but these kind of "out-of-order" mistakes can be frustrating at times.

Yet again, it feels like the node/scene/class relationship are not currently treated on the same level of usability, that's why the proposal may look like a workaround currently. The very first introductory tutorial does talk about nodes and scenes on the same page.

This symptom may be explained better by #22.

I think the node/scene relationship is overlooked because hardly any part of the engine utilizes scene instancing mechanism, most of the "instancing" in the engine is done manually in either constructor and/or NOTIFICATION_ENTER_TREE/READY, while the "real" instancing is mainly editor functionality.

[Xrayez]

I've implemented godotengine/godot#33974 as PackedScene method as well, I guess either works.

Compare:

var bomb = instance_child(Bomb.get_scene())
var bomb = Bomb.get_scene().instance_as_child(self)

Note: I'm aware of instantiate renaming but it it's better to stick to what we currently have for familiarity and discussion purposes.

[Xrayez]

For those who wonder about specific static methods that I use per class:

class_name Bomb extends RigidBody2D

static func get_scene():
    return load("bomb.tscn")

static func instance():
    return get_scene().instance()

This allows me to link the class with the scene without having to define any global references/singletons etc. I could as well replicate the suggestion https://github.com/godotengine/godot-proposals/issues/260#issuecomment-559285521 about scene.instance_as_child() this way without ever dealing with the core in the first place:

static func instance(parent: Node):
    var new_node = get_scene().instance()
    parent.add_child(new_node)
    return new_node

# ...

func _ready():
    var bomb = Bomb.instance(self) # yay, instance + add_child works flawlessly

But this leads to another usability problem: I'd have to redefine the same method literally in every class. So I'd be also happy if godotengine/godot#23101 could be implemented to help with this perhaps.

[willnationsdev]

Not to say this shouldn't be added to Node/PackedScene for whatever reason, but this stuck out to me:

But this leads to another usability problem: I'd have to redefine the same method literally in every class.

extends Reference
class_name Utils
static func instance_child(p_target: Node, p_class: Script) -> Node:
    assert(p_target)
    assert(p_class)
    var new_node = p_class.new()
    p_target.add_child(new_node)
    return new_node

Voila, a reusable instantiation method that doesn't need to be redefined for every class that uses it.

extends Node
class MyClass
func _ready():
    Utils.instance_child(self, Bomb)

[Xrayez]

True, sorry but I have to clarify, I know how to make it reusable, but it still creates usability problem (whatever the definition of usability may be for a person). I'd rather redefine the same method over several classes to achieve this level of usability, and there's already a get_scene static method with the scene resource path which needs to be hard-coded per class anyway. One could define a singleton which could effectively map the global class to its associated scene though, and use Utils.instance_child() that way internally.

To make it more game-specific, I have a Game global class which can retrieve an instance of the running game node, so the interface would look like this:

var bomb = Game.get_instance().add(Bomb)

The add method could as well define a way how and where particular scenes are added to the scene tree depending on the context, so the whole complexity is justified within the specific game architecture.

As we see there are multiple ways to achieve the same thing even with scripting, the question is whether the engine could help the scripting part to be easier. Again this proposal may not be suitable for real-life architectures where an implementation is dictated by design, but for prototyping this could really speed up development.

[wyattbiker]

Have you tried something like this where you use get_node("fuse") for child nodes:

# bomb.gd
class_name Bomb extends RigidBody2D
onready var fuse: Timer = $fuse
onready var some_int: int =10

var bomb = preload("res://bomb.tscn").instance()
bomb.get_node("fuse").wait_time = 5.0 # This works
bomb.some_int=20
add_child(bomb)

Off course would be nice if one didn't have to use get_node(). Is it possible .instance() just needs to be fixed to instantiate the complete object, instead of adding yet one more method.

[Xrayez]

Yeah, having to use get_node(path) defeats the purpose of onready vars in the first place for me. I've also realized I can prevent this from happening using NOTIFICATION_INSTANCE over NOTIFICATION_READY (aka onready) to initialize node paths that way, see workaround at https://github.com/godotengine/godot/issues/33620#issuecomment-559999681.

[ttencate]

What if add_child simply returned the added node, instead of void? One could then write:

func explode():
    var explosion = add_child(load("res://explosion.tscn").instance())
    explosion.global_position = impact.global_position

[Xrayez]

@ttencate I'm not sure about any of the performance overheads for add_child to return anything for such a common operation (godotengine/godot#6521), especially when it's used for core stuff within the engine itself, so I think it's better to have a dedicated method for that instead. So far core devs seem to prefer godotengine/godot#33974 implementation for this kind of thing, and I would personally be satisfied with it at this point. 🙂

[bojidar-bg]

The problem with add_child returning the new child is that it is not obvious from the function's name. Thus the produced code can be confusing, as mentioned in the linked issue.

# It might look like it allows for fluent-API-like chaining: (but it doesn't)
main_menu.add_child(settings).add_child(level_select)
# It opens way for some ugly one-liners
options.add_child(Button.new()).text = "Click me!"
# It raises question about other one-liners
options.add_child(Button.new()).set_text("Click me!").connect("pressed", self, "press")

There should be none or little performance overheads from this, as it is just passing a pointer around.

[asheraryam]

I'd suggest to name it instance_as_child() instead of instance_child(), since the latter implies that the child is a PackedScene (which is ridiculous but still).

For 4.0 this would be instantiate_as_child() which is very intuitive imo.

[Xrayez]

Yes I think having as won't hurt anything.

But regarding instantiate, I'm not sure where you're from, but as a non-native speaker, I don't find instance to be confusing at all (a lot of words have the same naming for both noun and verb in English, so I take this for granted), and I'd rather avoid typing out potentially difficult-to-spell word. But I can only imagine how confusing this may be for a native speaker (or anyone used to Latin-based languages). 🙂

I think English is also quite an implicit language, so I understand the reasoning behind making it more explicit. 😛

So for godotengine/godot#33974, I'd personally wait until we actually rename the existing PackedScene.instance method before proceeding with the renaming.

[Xrayez]

I've been pondering upon this recently and recalled godotengine/godot#33610.

If you search for "spawn" throughout this proposal as well, then you'll see that it's used in examples and use cases.

So, what I'm trying to say is that the engine core could certainly facilitate implementing those "spawning" mechanics via plugins, which is a quite frequently used mechanic in games in my opinion. And instantiate_as_child() would do exactly what it says on the tin, I cannot possibly imagine what else a PackedScene could do by itself when this proposal gets implemented, this is as far as it can go, which would complement the PackedScene functionality, not just the raw API which could look "pretty", but to reflect those use cases to full extent.

I mean, the way I use it, it's 95% chance that the instantiated scene is going to be added to the scene tree right after. So, in the worst possible case, you'd just make people happier. 🙂