Refactor Object metadata

[reduz]

• API kept the same (Although functions could be renamed to set_metadata/get_metadata in a later PR), so not much should change.
• Metadata now exposed as individual properties.
• Properties are editable in inspector (unless metadata name begins with _) under the metadata/ namespace.
• Added the ability to Add/Remove metadata properties to the inspector.
• Because metadata is now exposed as properties, the values must now be valid identifiers (had to modify tests to pass).

This is a functionality that was requested very often, that makes metadata work a bit more similar to custom properties in Blender.

How it looks:

So, as a note, as metadata is now user visible, make sure to make the ones you don't intend to be seen begin with underscore ("_"). Such as

obj.set_metadata("_customdata",someval)

This way, the editor will omit it, but it will still be saved.

Supersedes #30765

[Calinou]

The feature itself looks good to me.

However, the "+" icon to the left of "Add Metadata" should have more spacing. Right now, the icon looks like it's too close to the text. It might be worth adding some vertical padding above the button too.

PS: Does this PR supersede #30765? If so, maybe #30765 could be salvaged for 3.x since we can't make compatibility-breaking changes there.

[reduz]

@Calinou

However, the "+" icon to the left of "Add Metadata" should have more spacing. Right now, the icon looks like it's too close to the text. It might be worth adding some vertical padding above the button too.

Can you make a small mock up or give me feedback what you would change in code?

PS: Does this PR supersede #30765? If so, maybe #30765 could be salvaged for 3.x since we can't make compatibility-breaking changes there.

It does, will specify in the description, thanks!

[h0lley]

is there any harm to people potentially assuming meta data is functionality of Node? because the UI suggests that.
perhaps there should be an Object category as separator?

alternatively, a separate Tab could be introduced like this plugin does:

[reduz]

@h0lley No, metadata is a property of everything, not just nodes. I prefer not add a separate tab and keep everything as part of the inspector. This allows taking better advantage of the existing editors as well as the possibility of animating these values.

[Mickeon]

While I do welcome the feature, metadata is something more advanced users may want or need access to. I'm afraid this bit of extra info may be a tad overwhelming for starting users when displayed right there, as the difference between normal properties and metadata may not readily be apparent.

[reduz]

@Mickeon The way its proposed to work is that, if you don't know what it is or not need it, you will not use it for the most part.

[fire-forge]

So, as a note, as metadata is now user visible, make sure to make the ones you don't intend to be seen begin with underscore ("_").

Could an editor or project setting be added to show all metadata, including ones that start with an underscore? Being able to see all metadata could be useful for debugging.

However, the "+" icon to the left of "Add Metadata" should have more spacing. Right now, the icon looks like it's too close to the text. It might be worth adding some vertical padding above the button too.

It could be made to look like the button in the Array and Dictionary editors:

[reduz]

Alright, updated to look like this:

[Mickeon]

Could an editor or project setting be added to show all metadata, including ones that start with an underscore?

Perhaps this Editor setting could be set to "None" (completely hide the feature, depending on the workflow one may not need it), "Omit Underscore" (the default and current behaviour), "All" (display all metadata).

[Calinou]

Here are my changes for the button (made before @fire-forge posted their suggestion):

diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp
index 11df8c88f3..558e5b2b1e 100644
--- a/editor/editor_inspector.cpp
+++ b/editor/editor_inspector.cpp
@@ -2959,12 +2959,15 @@ void EditorInspector::update_tree() {
 
 	if (!hide_metadata) {
 		CenterContainer *cc = memnew(CenterContainer);
+		// Add some spacing above and below the button.
+		cc->set_custom_minimum_size(Size2(0, 50 * EDSCALE));
 		main_vbox->add_child(cc);
 		Button *add_md = memnew(Button);
-		add_md->set_flat(true);
+		add_md->set_flat(false);
 		add_md->set_text(TTR("Add Metadata"));
 		add_md->set_focus_mode(Control::FOCUS_NONE);
 		add_md->set_icon(get_theme_icon("Add", "EditorIcons"));
+		add_md->add_theme_constant_override(SNAME("hseparation"), 8 * EDSCALE);
 		add_md->connect("pressed", callable_mp(this, &EditorInspector::_add_meta_callback));
 		cc->add_child(add_md);
 	}

This is outdated now, except for the part about the CenterContainer minimum size (which adds some spacing above and below the button):

[theraot]

@reduz, I know what you say here is correct:

@h0lley No, metadata is a property of everything, not just nodes.

However, notice that in the mockup it appears inside "Node". There are "CanvasItem" and "Node" sections in the inspector:

So if you

prefer not add a separate tab and keep everything as part of the inspector.

Then consider the other part that @h0lley was saying

perhaps there should be an Object category as separator?

Something like this:

I haven't tested the build. So perhaps you already did that and I'm unaware.

Otherwise as @h0lley said the UI suggest it is part of Node.

[KoBeWi]

I got this crash when testing, after pressing the Add Metadata button:

CrashHandlerException: Program crashed
Engine version: Godot Engine v4.0.alpha.custom_build (bab2ad4d3283a9b4e6050a2345a4ea408001484e)
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] Node::is_inside_tree (C:\godot_source\scene\main\node.h:316)
[1] Node::is_inside_tree (C:\godot_source\scene\main\node.h:316)
[2] Window::popup_centered (C:\godot_source\scene\main\window.cpp:1054)
[3] EditorInspector::_add_meta_callback (C:\godot_source\editor\editor_inspector.cpp:3725)
[4] call_with_variant_args_helper<EditorInspector> (C:\godot_source\core\variant\binder_common.h:241)
[5] call_with_variant_args<EditorInspector> (C:\godot_source\core\variant\binder_common.h:351)
[6] CallableCustomMethodPointer<EditorInspector>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[7] Callable::call (C:\godot_source\core\variant\callable.cpp:51)
[8] Object::emit_signalp (C:\godot_source\core\object\object.cpp:1115)
[9] Object::emit_signal<> (C:\godot_source\core\object\object.h:790)
[10] BaseButton::_pressed (C:\godot_source\scene\gui\base_button.cpp:135)
[11] BaseButton::on_action_event (C:\godot_source\scene\gui\base_button.cpp:176)
[12] BaseButton::gui_input (C:\godot_source\scene\gui\base_button.cpp:67)
[13] Control::_call_gui_input (C:\godot_source\scene\gui\control.cpp:942)
[14] Viewport::_gui_call_input (C:\godot_source\scene\main\viewport.cpp:1303)
[15] Viewport::_gui_input_event (C:\godot_source\scene\main\viewport.cpp:1603)
[16] Viewport::push_input (C:\godot_source\scene\main\viewport.cpp:2709)
[17] Window::_window_input (C:\godot_source\scene\main\window.cpp:974)
[18] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (C:\godot_source\core\variant\binder_common.h:236)
[19] call_with_variant_args<Window,Ref<InputEvent> const &> (C:\godot_source\core\variant\binder_common.h:351)
[20] CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[21] Callable::call (C:\godot_source\core\variant\callable.cpp:51)
[22] DisplayServerWindows::_dispatch_input_event (C:\godot_source\platform\windows\display_server_windows.cpp:2033)
[23] DisplayServerWindows::_dispatch_input_events (C:\godot_source\platform\windows\display_server_windows.cpp:1998)
[24] Input::_parse_input_event_impl (C:\godot_source\core\input\input.cpp:658)
[25] Input::flush_buffered_events (C:\godot_source\core\input\input.cpp:878)
[26] DisplayServerWindows::process_events (C:\godot_source\platform\windows\display_server_windows.cpp:1747)
[27] OS_Windows::run (C:\godot_source\platform\windows\os_windows.cpp:677)
[28] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:169)
[29] _main (C:\godot_source\platform\windows\godot_windows.cpp:191)
[30] main (C:\godot_source\platform\windows\godot_windows.cpp:205)
[31] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:219)
[32] __scrt_common_main_seh (D:\a01\_work\26\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[33] BaseThreadInitThunk
-- END OF BACKTRACE --

Seems to be happening right after opening project.

Otherwise as h0lley said the UI suggest it is part of Node.

script is also part of the Object, not Node.

That aside, the script property looks awkward between 2 sections:

Another awkward thing is double popup:

It also causes Transient parent has another exclusive child. error.
Maybe it would be better to have a simple validation, like in plugin creation dialog?

And disable Add button if name already exists.

EDIT4:
Another odd thing is that the button is part of the section:

At least it unfolds after adding though.

Also also, this is probably not worth addressing, but undoing meta delete will change their order:

[KoBeWi]

remove_meta() isn't properly handled by the inspector. When you use it (with a tool script at least), the value will remain in the inspector and can't be removed. Also interacting with it might result in a crash. This is most likely because remove_meta() and set_meta(null) are handled differently (with the latter being correct).

EDIT:
Seems like some metadata types don't appear in the inspector, even though they are created successfully (or at least without any error). Object metadata seems to have Resource hint, but it's not created (it doesn't even appear in scene). Meantime, creating Signal/Callable/RID metadata will successfully create them in the object. They will not appear in the inspector, but the scene will be corrupted when saved.

[TokisanGames]

@theraot
Then consider the other part that @h0lley was saying

perhaps there should be an Object category as separator?

Object is not instanceable in the editor, so it doesn't matter. As mentioned, script is also part of Object. So it's consistent that everything exposed in Object and Node show up together.

[PoisonousGame]

因此，请注意，由于元数据现在对用户可见，请确保您将不希望看到以下划线的元数据（“_”）。

是否可以添加编辑器或设置以显示所有元数据，包括以下规划线的元数据？查看所有元数据对于调试有用的项目。

应该是，“元元”的“+”图标看上去有更多的数据。

它可以看起来像目录和字典编辑器中的按钮：

This is not good, the + and the text are too separated, I personally think Calinou's design is better

[reduz]

@KoBeWi

It's possible to bypass the meta validation by pressing Enter in the LineEdit

This sounds like a bug in ConfirmationDialog, will give it a check.

Also the bug I mentioned in edit of #59452 (comment) still exists. (i.e. Signal/Callable/RID types corrupt the scene and Object has no effect at all)

I will change it to remove types that can be conflicting, although for Object it seems broken somehow.

[KoBeWi]

Suggested change

	continue; //not editable by inspector.
	continue; // Not editable by inspector.

[KoBeWi]

The resource icon is broken:

But other than that it's good now.

[reduz]

@KoBeWi It seems the icon for "Resource" is just missing because, for some reason, we never really needed to use it until now.

I suppose it can be added in a subsequent PR.

[akien-mga]

Thanks!

[Calinou]

Note: I added the breaks compat label because of this change:

• Because metadata is now exposed as properties, the values must now be valid identifiers (had to modify tests to pass).

[MelPeslier]

How can we know the list of editor metadata ?

[KoBeWi]

From scene file.

[MelPeslier]

Well, when I open the file in an external editor, it does show me the metadata applied yes. But not all of the possible ones, and that's what I want to know, how many are they ? what are they, and even better, what's their names to call them ??

[KoBeWi]

There is no list of "possible" metadata, the values are assigned dynamically. The best you can do is search the C++ source code for set_meta().