Add object_created_in_editor signal

[KoBeWi]

What problem(s) does this PR solve?

Closes (maybe) godotengine/godot-proposals#7593
Helps #119020
Helps https://chat.godotengine.org/channel/editor?msg=5NLvSXn9Tse6xdcEp

Additional information

This PR salvages #81217 and it's mostly the same, except the new notification is sent in instantiate_object_properties(), so nodes receive it in all cases when PROPERTY_USAGE_EDITOR_INSTANTIATE_OBJECT is handled. It also handles a few missed cases: when a script is dropped to make node, when scene root is created from favorites and when new scene is created using non-custom node.

See #81217 for rationale on this solution, especially my last comment.

EDIT:
Changed to a signal in EditorInterface instead.

[Mickeon]

Provide a few examples. Sorry for not doing so myself as of writing.

Suggested change

	Notification received right after the node has been created in the editor. This notification is only sent when a node is created using the Godot editor UI and will not occur when nodes are created by scripts or instantiating packed scenes.
	Notification received right after the node has been created using the editor's interface, such as via _ . This notification is not sent when the node is created by scripts or when instantiating packed scenes.

Or similar. I am not sure about the wording I suggested, either. I just know a few examples are a given.

[Calinou]

I'd also mention this case (which surprised me a bit at first, but it makes sense):

Suggested change

	Notification received right after the node has been created using the editor's interface, such as via Add Child Node or as a root of a new scene. This notification is not sent when the node is created by scripts or when instantiating packed scenes.
	Notification received right after the node has been created using the editor's interface, such as via Add Child Node or as a root of a new scene. This notification is not sent when the node is created by scripts, when instantiating packed scenes or when duplicating the node in the editor.

[KoBeWi]

I commented, but the comment is gone 🤦‍♂️

I don't think it's surprising when duplicating a node. It'd be more surprising if it was otherwise, because duplicating nodes could lead to losing some properties.

[Calinou]

Tested locally, it works as expected. Code looks good to me.

[Leonardo-Ma]

Could reasoning and possible approaches be expanded? The linked issue is very specific about scripts overriding engine defaults, but it's being suggested as a new approach instead of changing engine node default values to avoid breaking compatibility with previous versions. Something I don't see being explained in the issue, here, in the salvaged PR or in any of the PRs/issues that are being referenced to this (the linked chat message also doesn't open for me, so can't check that).

If it's also only applied when created by the editor, doesn't that mean two versions of every single instance to be maintained? That seems a nightmare?

[KoBeWi]

it's being suggested as a new approach instead of changing engine node default values to avoid breaking compatibility with previous versions.

Well, that's just one possible usage. #119020 does change properties, but they are not intended to be defaults; they are meant to make the node more usable in the editor out of the box. The customized "defaults" should be irrelevant outside the editor.

For changes that work around compatibility, the code should be explicitly marked with #ifndef DISABLE_DEPRECATED, so it's easy to identify as such. And we do have a similar mechanism already, for project settings when creating new projects.

godot/editor/editor_node.cpp

Lines 8418 to 8420 in ac7ef92

	// This is used to set better defaults for new projects without affecting existing projects.
	// Keep the list alphabetically sorted.
	HashMap<String, Variant> EditorNode::get_initial_settings() {

If it's also only applied when created by the editor, doesn't that mean two versions of every single instance to be maintained? That seems a nightmare?

There is a risk if it's overused I guess. I think the editor defaults should be mentioned in the docs.

However this PR only introduces the notification, the details on its usage in the engine are yet to be decided in the subsequent PRs. There are just numerous cases where it can be useful.

[kitbdev]

This leads to adding blocks of editor only code in each Node that uses it.

Users can add nodes to a scene in the editor from code, and if they want to use editor defaults they may try to send the notification manually. The proposal mentions wanting the notification from code, and this would be a workaround for that. However, instantiate_object_properties doesn't get called so it may be inaccurate.

I wanted to mention an alternative approach to consider, since I haven't really seen it brought up.
We could have a single method in editor code that takes the new Node as an argument and sets the default values, eg:

void set_editor_defaults(Node *p_node) {
	Label *label = Object::cast_to<Label>(p_node);
	if (label) {
		label->set_text("Label");
	}
	Decal *decal = Object::cast_to<Decal>(p_node);
	//etc
}

There can be a signal emitted on EditorNode from instantiate_object_properties that connects to this method (in EditorNode or maybe a new file b/c of includes). Other places / modules can connect to it too to configure their nodes separately.
We can emit a signal on EditorInterface for users to connect to. And probably expose a method that calls instantiate_object_properties in case the user makes nodes in the editor and wants editor defaults.

This way editor-only code isn't added to each node. Users wouldn't have to make their Nodes tool scripts to have editor defaults, it can be in a separate tool script.
This would also allow users to override the defaults of any Node that gets created in the editor, which seems useful for plugins to configure defaults for a project.
There are some drawbacks though, like more merge conflicts in this method, and you need to find this to find the defaults for a node.
Private variables also won't be accessible, but since the purpose is to set editor default values, it shouldn't need private variables.

---

Should we have this for Resources too?
If a Resource is part of a Node the Node can set the default of the Resource, but that is different than changing the editor defaults of Resources themselves.
It may need to be a separate notification(/signal) anyway so we can wait until its needed.

Also cc @godotengine/xr since the proposal is mainly about XR.

[Mickeon]

Admittedly we have a very chaotic setup for editor-only code which would be nice to re-evaluate.

• It reminded me of Allow to define editor methods in a separate file #89160 again.

[KoBeWi]

We could have a single method in editor code that takes the new Node as an argument and sets the default values

That sounds like sensible approach.

A signal could be even connected in plugins, so e.g. particle plugin could be responsible for setting particle defaults, so we don't have a single super method for handling everything.

One downside is that many callbacks might get connected to such signal, each checking whether the provided node is of expected type, which might eventually make instantiating nodes slower. Although it does not cause problems with settings_changed(), so it's probably fine.

Another thing is that the order of callbacks for signal can't be reasonably guaranteed, unlike for a notification. Multiple callbacks customizing one node can easily mess up each other's settings.

[KoBeWi]

Changed notification to a signal and fixed some cases where object wasn't initialized.

[KoBeWi]

This is notable change. Objects instantiated here are technically created by the editor, so this allows to initialize them recursively.

[Calinou]

With this new approach, is it still possible to implement #122066 in a way that doesn't break encapsulation?

#ifdef TOOLS_ENABLED
void Decal::_notification(int p_what) {
	switch (p_what) {
		case NOTIFICATION_READY: {
			EditorInterface::get_singleton()->connect("object_created_in_editor", Callable(this, "_object_created_in_editor"));
		} break;
	}
}

void Decal::_object_created_in_editor(Object *p_object) {
	print_line("test");

	// Set improved defaults for newly created Decal nodes in the editor.
	// This does not affect Decals created by code, or Decals created in older versions of Godot.
	Decal* d = Object::cast_to<Decal>(p_object);
	if (d) {
		d->set_size(Vector3(2.0, 1.0, 2.0));
		d->set_upper_fade(0.0);
		d->set_lower_fade(0.0);
	}
}
#endif

For this to compile, I need to #include "editor/editor_interface.h", but Decal is part of scene/. (Either way, it doesn't seem to get called anyway.)

[KoBeWi]

It's not possible in the class itself. You'd have to add DecalEditorPlugin, which handles the signal. It's more involved, but avoids adding editor-specific code to scene classes.

Either way, it doesn't seem to get called anyway.

You should use callable_mp(), Callable() is only for registered methods.

[Calinou]

It's not possible in the class itself. You'd have to add DecalEditorPlugin, which handles the signal. It's more involved, but avoids adding editor-specific code to scene classes.

Thanks for the advice, I updated #122066 accordingly 🙂

[Mickeon]

Y'know, now that I think about it, both the signal and the notification don't have to be mutually exclusive features.

Because, yes, with a signal we do not break encapsulation and allow addons to inject their own code, but one could argue some users would rather not be bothered to make their own editor plugin for a few tweaks on object creation.
The logic could be present in the original script to keep code less spread out, and this may need to run in the editor with @tool, anyway, for other miscellaneous custom behavior.

Either way we would have to choose one or the other within Godot's codebase.

[KoBeWi]

You don't really need a plugin to use this signal in your scripts, you only have to avoid using EditorInterface directly.

@tool
class_name TestClass extends Node

func _init() -> void:
	if Engine.has_singleton(&"EditorInterface"):
		Engine.get_singleton(&"EditorInterface").object_created_in_editor.connect(initialize_editor_stuff, CONNECT_ONE_SHOT)

func initialize_editor_stuff(object: Object):
	if object == self:
		print("Test")

This works and does not break scripts at runtime. It's a bit more involved than notification though.

[kitbdev]

Tested the signal and method, looks good.
Creating Scripts and Shaders don't emit the signal, but that seems fine.

• For the main use case of godotengine/godot-proposals#7593, having godotengine/godot-proposals#338 might be a cleaner solution.
  But for the purposes of changing defaults without breaking compatibility or letting users set defaults without inheriting the class, this is a good approach. And it allows a workaround for not being able to override inherited defaults.

[romgerman]

Creating Scripts and Shaders don't emit the signal, but that seems fine

Why is that if they are still Resources? That special treatment for Scripts is exactly why some things is impossible to do in the editor.

[KoBeWi]

Fixed Scripts and Shaders. It was an oversight.