Improving InstancePlaceholders

[Lielay9]

Introduction

InstancePlaceholders are a powerful tool to create dynamic levels that don't need to load everything at once. The major benefit to them over creating factories or various other loaders is that they retain the editability of a regular child scene; exported properties remain visible and are automatically set to the instanced child scene when created. Despite their general usefulness, InstancePlaceholders see very little attention. That can be traced back to the lack of tutorials and less-than-ideal implementation. This is a discussion about improving the latter, but before delving into the issues it's best to recap how InstancePlaceholders can be used:

• Create a scene with child scenes.
• Mark desired child scenes to be loaded as placeholders by right-clicking their root node and enabling "Load As Placeholder".
• At runtime access the created InstancePlaceholder and call create_instance()

InstancePlaceholders are deceptively simple to use. Deceptively, because they have many undocumented quirks to them; some bordering the line between a bug and implementation detail. The following is a short list of either unclear details or general considerations about their current implementation and how they could be improved. This is a very open-ended proposal. Any suggestions, thoughts, alternatives etc. are most welcome. Without further ado:

Issues

1. Undo/redo

Related: godotengine/godot#62727.
Toggling "Load As Placeholder" should be saved to undo history despite it currently being a rather undestructive operation (still alters the scene file).

2. Child nodes

(Nodes added under child scenes)

Current:

When the scene is loaded as a placeholder, the nodes will be children of the created InstancePlaceholder. When the placeholder is "replaced" its children will be freed alongside it. This is only expected if the user intended the nodes to be children of the placeholder node, but this is unclear within the editor.

# Nodes stay as children of InstancePlaceholder
create_instance(false) 

# Nodes are moved under the replacement
var scene_root := placeholder.create_instance(false)
scene_root.get_parent().remove_child(scene_root)
placeholder.replace_by(scene_root)
scene_root.name = placeholder.name 

# Nodes are freed
create_instance(true)

Alternative 1. (Simple)

Document current behaviour (preferably with an example).

Alternative 2. (Simple)

Change behaviour

create_instance(false) # Nodes stay as children of InstancePlaceholder
create_instance(true) # Nodes are moved under the replacement
create_instance(false); queue_free() # Nodes are freed

Alternative 3. (Simple)

Clarify behaviour by bringing back deprecated replace_by_instance() and remove parameter replace from create_instance()

create_instance() # Nodes stay as children of InstancePlaceholder
replace_by_instance() # Nodes are moved under the replacement
create_instance(); queue_free() # Nodes are freed

3. Function naming*

*Written before merging the updated documentation; Could be considered solved.
Related: godotengine/godot#64127

Current

The function create_instance() is a little misleading as it doesn't imply that the instance will also be added to the scene. It is also unclear where it is added in relation to the InstancePlaceholder.

Alternative 1. (Simple)

Retain current behaviour, but document how the function operates.

Alternative 2. (Simple)

Bring back deprecated replace_by_instance() and change create_instance() to only create and return the instance making it possible to manually choose where the instance will be added:

# Same as current "create_instance(false)"
var node := create_instance()
add_sibling(node)

# Same as current "create_instance(true)"
replace_by_instance()

4. Groups and signals

Related: godotengine/godot#18697.
Related: godotengine/godot#64238.

Current

Currently, groups and signals set in the editor are assigned to the InstancePlaceholder instead of the child scene root. This is undocumented and there are no indications of this happening inside the editor.

Alternative 1 (Simple)

Retain current editor behaviour, but document it and raise a configuration warning when a signal is tried to connect to or from the child scene. Also don't connect the signals to the InstancePlaceholder.

Alternative 2 (Complex)

Have InstancePlaceholder save connections from and to it and try to assign them during replacement (this might be complicated to do on the engine side and may lead to other unexpected behaviour) and also save the group inside InstancePlaceholder and assign it to the replacement (likely optionally similar to replace_by()).

Alternative 3 (Simple)

Combination of 1 and 2. Raise warning for signals but (optionally) save and assign groups to the replacement.

5. Exported properties

Current

Similar to signals and groups, InstancePlaceholder "steals" all properties saved to the scene it can. This leads to the funky behaviour where overridden properties of node and object are attached to the InstancePlaceholder (such as process_priority and script) instead of the child scene. Properties not found in those classes work "as expected", and are stored inside stored_values in InstancePlaceholder. The current behaviour is borderline a bug and should probably be changed.

Alternative 1 (Simple)

Pass the changed property values to the replacement, instead of stealing them.

Alternative 2 (Complex but flexible)

Make the properties of InstancePlaceholder editable inside the Inspector.
Examples: Image1 Or: Image2

6. Extending

Current

Currently, InstancePlaceholder isn't exposed fully. It is not possible to extend InstancePlaceholder even though it is possible to add a script to it at runtime. This means users can only attach scripts that extend Node to it. Similarly, it is not possible to add InstancePlaceholder-node to a scene inside the editor. However, it is possible to save a scene that has InstancePlaceholders result.

Alternative 1 (Simple, flexible)

Expose InstancePlaceholder to be used as a node and extended and when packing a scene save InstancePlaceholder nodes optionally as placeholders instead of nodes of type InstancePlaceholder.

Alternative 2 (Simple, restrictive)

Don't expose and when packing a scene, save InstancePlaceholders nodes as placeholders instead of nodes of type InstancePlaceholder.

Missing features

This is a list of features that would generally improve InstancePlaceholders but are not as crucial to solve as the issues mentioned before.

1. Function to convert nodes/scenes to InstancePlaceholders or create them at runtime. (They're only created when PackedScene is instantiated) I stand corrected. It is actually trivial to create them exactly by packing a scene:

# Script on child scene root
set_scene_instance_load_placeholder(true)
var packed_scene := PackedScene.new()
packed_scene.pack(self)
replace_by(packed_scene.instantiate())
queue_free()

3. Virtual function or property in InstancePlaceholder, that when returns or is set to true, will replace the placeholder before the signal connections or groups are assigned. (Related: Add a way to cancel adding a Node to tree #5102). Would effectively make it possible save multiple versions of the same scene with some nodes "removed" without saving an entirely new scene on disk.
4. Function to change what Scene InstancePlaceholder will instance. If implemented with the previous, it'd be possible to create even more complex versions of scenes. (It is possible to use a custom_scene even now but only when calling create_instance())

End

Thank you for coming to my TED talk. Once again I'm asking for your wiser opinions, suggestions and feedback. Does anyone even care? Should InstancePlaceholders be deprecated altogether? Let yourself be known.

[Mickeon]

Does anyone even care? Should InstancePlaceholders be deprecated altogether?

Absolutely not! These are all great ideas. As Godot can be used for creating increasingly bigger games, there will be an increasing necessity to load things dynamically while still being able to manage and edit them as one. InstancePlaceholders can be used for that kind of thing, too, and if large games are bound to be made, improving them during 4.x should probably be high up there.

Speaking of which, it does bother me there's no simple way to load the Node InstancePlaceholders are taking the place of in between frames, without locking the main thread. It is possible, but it requires another Script and a bit of grease and elbow. If InstancePlaceholders could have their own properties, a setting to allow this would certainly be welcome.

[Lielay9]

Speaking of which, it does bother me there's no simple way to load the Node InstancePlaceholders are taking the place of in between frames, without locking the main thread. It is possible, but it requires another Script and a bit of grease and elbow. If InstancePlaceholders could have their own properties, a setting to allow this would certainly be welcome.

Exposing the class could be a good start since it also would fix a couple of other issues. Though that still doesn't help with the fact that there's no way to set the properties of the to-be-placeholder (bar the bug; issue 5). Exposing the properties is a little more complicated since it would need a lot more changes to the core (Exposing the properties and the PackedScene-instantiation would need a rework). That said, I think it'd be a worthwhile endeavour in the long run.

[azur-wolve]

Hey, this looks really useful.
Specially for things like level design and in a higher level, world design.
And anything that requires not instancing until necessity, for performance.

Got an idea!

What about have another type called EntityPlaceholder (or EntityInstancePlaceholder)

It would have a property that could be either a Node2D (so Control nodes), a Node3D, or null (for non-visual, Node).

Used just like any other node that can be in a node hierarchy, its node children would be Nodes/Scenes that aren't instanced yet, but just lay there on "stand-by".

It would work just like a substitute until the instances are created (holding the values (game state) of the desired properties of the specified instances).

Its role would be kind of a placeholder / factory / spawner / arranger / proxy.

Some use cases for EntityPlaceholder:

• can access "the properties of the non-instanced nodes"; using EntityPlaceholder to store certain values that could still be used by other entities. When the instances are created, values get mapped (passed to respective properties) and erase them fromEntityPlaceholder. (some kind of temporal substitute for game state), or optionally allow to keep them if desired. (this could be achieved by having two types: PlaceholderValue and KeptPlaceholderValue.

• allow to instance only certain instances under need, and also delete, clone them, etc

• allow them to use "factories" to create families of types. [NOTE: easy by just attaching a custom type factory to them; let them have factories as properties could not be the most convenient, maybe better just attach custom scene factories and let them do their job, so allow the possibility to have EntityPlaceholder children already instanced from the start.]

Also after instancing, can choose between:

• keep the EntityPlaceholder as parent of the instances
• move the instances to a specified node parent, and then delete EntityPlaceholder
• move the instances to a specified node parent, and keep the EntityPlaceholder (useful to be used as entity spawners/factiories, etc)
  (keep the EntityPlaceholder is logic if you only instance some children, not all.

[Zireael07]

I love that EntityPlaceholder idea. I just spent two days rewriting my mapgen to space it over several frames so that I can visualize what's actually happening, but the placeholders would actually let me spawn things in "buckets"/"lots" (whatever they end up called) when needed (i.e. when the player gets close to the map area) and not all at once on game start (and yes, my mapgen is full of things depending on other things's variables or positions...

[azur-wolve]

thx for feedback

also it would implement logic to de-instance children and use PlaceholderValues again (idea to play side-by-side with signals)

gonna open a discussion thread about it
I encourage anyone who wants this to become a thing, participating on its conception.

[Lielay9]

While this is similar to what I've done for my game, creating more complexity and trying to meet every game's needs runs the risk of either not being general enough or becoming a bloaty mess. I don't want to discourage you from creating a new system, just keep this in mind when doing so. I do think it'd be great to have ways to better stream Godot scenes, for example, a way to automatically load and unload parts of big 3d scenes based on a 3d-nodes (players) position (not quite something InstancePlaceholders can do on their own) but it nighs impossible to do in a way that works for all games. In that sense, I'm also against most of such tries... so far.

On that note, I'm not quite sold on having a dedicated factory node, as that takes away the biggest advantage of InstancePlaceholders: they don't break the node-hierarchy. With due respect, I find it unlikely that EntityPlaceholders could be more advantageous than implementing your own factory node to meet the needs of your game.

Also, since I familiarized myself with the InstancePlaceholder and PackedScene initialisation while creating this pull request -> godotengine/godot#64238, I think some of your suggestions for EntityPlaceholder are actually implemented there but not really documented or even intended to be used. Namely:

can access "the properties of the non-instanced nodes"; using EntityPlaceholder to store certain values that could still be used by other entities. When the instances are created, values get mapped (passed to respective properties) and erase them fromEntityPlaceholder. (some kind of temporal substitute for game state), or optionally allow to keep them if desired. (this could be achieved by having two types: PlaceholderValue and KeptPlaceholderValue.

You can access the values InstancePlaceholder stores with get(property) and store new ones with set(property, value) This is, of course, undocumented, and setting the value doesn't actually "set" the value it simply stores a new one. The same also applies to groups and signals... or should if the current issues with them are resolved. I've also thought to add flags property to disable such behaviour when needed.

As a final note, I must admit I'm privileged to understand enough c++ to access the core and exactly change and choose how I want my scenes to be loaded, so my position on these matters probably doesn't represent the vanilla Godot-user. In fact, I've already addressed most of my own qualms with InstancePlaceholders and level streaming, including the issues listed in this discussion. I just wanted to garner some information if it is a worthy endeavour to adapt/port some of the more general parts of those experiments to the core.

[azur-wolve]

It is fine.
Just was a brain storm. So ye there's much idea that can overcomplicate all.
About the Factory it just was a way of speak. It is better to implement your custom factories. Just that set those factories as children of such EntityPlaceholders could be convenient in some scenarios.
The biggest difference between InstancePlaceholder and EntityPlaceholder is that the first can't be instantiated directly as any other node, where the second would be.

What you talk about to load world chunks under need can be implemented.
Already have discussed it a bit myself proposing various world node types on another thread: #4052 (comment)
(have not opened a discussion/proposal yet for this one, but had redacted some sketch)

Such EntityPlaceholder would play a key role on loading world under-need.
But I don't know well InstancePlaceholder, so as you said that "some of the stuff I mentioned already can be done by it", Idk what to say tbh.