Rework the Configuration Warning system

[RedMser]

Closes godotengine/godot-proposals#9416
Closes godotengine/godot-proposals#550 again
Closes godotengine/godot-proposals#9098
For my first attempt at implementing this (which has since been reverted), see #68420 and #88182

What this PR does

• User facing changes
  • Configuration warnings are now a more general system called configuration info.
  • Configuration info is available not only on Node, but also on Resource.
  • Configuration info supports different severity levels, which show up as different icons: info, warning (default), error.
  • Configuration info supports being attached to a specific property, which makes the message show in the Inspector.
  • At the top of the inspector, you can now see a list of configuration info for the selected Node or Resource (see screenshots below).
• API changes
  • A new API _get_configuration_info is available which accepts a mixed Array of Dictionaries and Strings ("my warning" is equivalent to { "message": "my warning" }).
  • C++ code that did warnings.push_back(RTR("...")) will now instead use CONFIG_WARNING(RTR("...")) macro (can be suffixed with _P to refer to a specific property).
  • No compatibility breaking changes. New APIs for everything, old stuff is left untouched but marked deprecated. Any code still using _get_configuration_warnings will behave the same as before.
  • Pulled as much logic out of Node as possible, instead moving it to its own EditorConfigurationInfo class, so logic can be shared with Resources, and between different Editor UIs. Without tools enabled, get_configuration_info is not available, reducing build size by around 100KB.

Screenshots

For reviewers

First commit modifies the core and editor only, while the second commit adjusts all Nodes to use the new API.

Notes and Known Issues

These do not have to be resolved to merge this PR, unless deemed necessary by reviewers.

• Is the name "Configuration Info" clear enough, from a user perspective? I can not think of a better name, now that it's not only used for warnings but also errors and information text... Suggestions welcome!
• Improve the UI when clicking on the icon next to a property or in the scene tree. Should use the same RichTextLabel logic as the inspector plugin does, to show severity icons and different text color based on severity.
• Would be nice to have a tooltip when hovering the icon, but since other icons in the Inspector don't have this yet, it can be done in a follow-up PR.
• Localized property name style probably does not work properly, as it needs additional logic from EditorInspector. Would need to update the ConfigInfo struct with classname.
• None of the built in resources currently return any configuration info. This can be added as a follow-up.

[Calinou]

• Test whether exported games don't break, since some of the config info system is now part of the editor and not in Node anymore.

Note that there was some discussion about whether node configuration warnings should be able to show when running a project in an editor build, but outside the editor (i.e. with a print in the Output panel). I can't find the proposal in question, but I remember discussing it myself.

Edit: This was probably in godotengine/godot-proposals#3004 (comment).

It also makes me wonder whether all _get_configuration_warnings() methods (or at least the method bodies) should be placed behind #ifdef TOOLS_ENABLED to reduce binary size.

[RedMser]

It also makes me wonder whether all _get_configuration_warnings() methods (or at least the method bodies) should be placed behind #ifdef TOOLS_ENABLED to reduce binary size.

I tried this change (ifdef the entirety of get_configuration_info) and it seems to reduce the binary size by 100KB. Doing so for the body only would be a lot more manual labor, not sure if it's worth the hassle.

[RedMser]

This PR should be ready now. I've rewritten the description, to remove finished TODOs and clarify what this PR exactly does.

A larger UI change has been made by introducing a configuration info summary in the inspector (see screenshots in PR description).

I'd like feedback whether this list should show for Resource only, or also for Node like in the current implementation.

[Mickeon]

Quoting my own comment on the matter last time just for the sake for others to remember it, haven't looked at the implementation yet.

[RedMser]

Thanks for the multiple reviews! I applied the latest changes and squashed everything into two commits. My recompile locally was instant, so my rebase maneuver did not accidentally swallow any changes either ^^

[Calinou]

@RedMser Could you look into rebasing this PR so I can review it?

[RedMser]

@Calinou Updated to latest master! ^^

[Calinou]

I get a build failure here after rebasing on top of master on my end:

Compiling scene/2d/physics/scu/scu_scene_2d_physics.gen.cpp ...
In file included from scene/2d/scu/scu_scene_2d_1.gen.cpp:4:
./scene/2d/light_occluder_2d.cpp: In member function 'void OccluderPolygon2D::set_polygon(const Vector<Vector2>&)':
./scene/2d/light_occluder_2d.cpp:93:9: error: 'update_configuration_warning' was not declared in this scope; did you mean 'update_configuration_info'?
   93 |         update_configuration_warning();
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         update_configuration_info
scons: *** [scene/2d/scu/scu_scene_2d_1.gen.linuxbsd.editor.x86_64.o] Error 1
scons: building terminated because of errors.
[Time elapsed: 00:00:37.33]

[RedMser]

Seems like this got added a few hours after my rebase, haha. Did another, should work now.

[Calinou]

Tested locally (rebased on top of master c6c464c), it works as expected. This is a great start 🙂

The API that's exposed seems good to me. Some feedback regarding the UI:

• The "N Configuration Infos" text should be changed to mention the type of messages it contains, so you can get better information without needing to expand it. For example:

❌ 1 configuration error, 1 warning
⚠️ 2 configuration warnings, 1 info
❌ 3 configuration errors, 1 warning, 2 infos
ℹ️ 2 configuration infos

• There should be visual feedback when hovering the dropdown that can be expanded (the "1 Configuration Info" part):

• On the above dropdown, the folding arrow should be moved to the left of the text (and it should point to the right when folded) to match usual Tree behavior. Here's a mockup:

• Line spacing should be increased within the expanded dropdown, as it's quite low currently (compared to the rest of the editor):

• The property name written in brackets should be capitalized using EditorPropertyNameProcessor::get_singleton()->process_name() (so that it follows editor settings). Right now, it always displays the original property name which is typically in snake_case, regardless of editor settings.

• The warning icon next to a property should be moved to the left slightly, so that it's centered:

Here's a mockup of how it should look like:

• When hovering the warning icon, a tooltip with the warning description should appear, like when you hover it in the Scene tree dock. Right now, it shows the property's documentation instead.

[RedMser]

Thanks for your review and feedback! Screenshot after the suggested changes:

The "N Configuration Infos" text should be changed

Done, see _get_summary_text. I intentionally split the translations like this, since I assume some languages can't simply prefix/suffix the word "configuration" without changing conjugation.

Your mockup included emojis for the icons, but I decided not to add an icon for the dropdown title text. The list below already has many icons, so by not having an icon on the title, it gives it a bit of hierarchy. Let me know if you have a strong opinion on changing it.

visual feedback when hovering the dropdown
the folding arrow should be moved to the left of the text (and it should point to the right when folded)

Done. I've copied the UI from the "control positioning warning" inspector plugin, so it would make sense to update that one in a new PR as well.

Line spacing should be increased

Done.

The property name written in brackets should be capitalized

Done, although it will probably fail for "localized" property style, since there's a lot of additional logic in the inspector which would need to be pulled out (see code snippet). We would also need to update the struct to include class name.

godot/editor/editor_inspector.cpp

Lines 3177 to 3193 in 78aaa55

	// Get the property label's string.
	String name_override = (path.contains("/")) ? path.substr(path.rfind("/") + 1) : path;
	String feature_tag;
	{
	const int dot = name_override.find(".");
	if (dot != -1) {
	feature_tag = name_override.substr(dot);
	name_override = name_override.substr(0, dot);
	}
	}
	// Don't localize script variables.
	EditorPropertyNameProcessor::Style name_style = property_name_style;
	if ((p.usage & PROPERTY_USAGE_SCRIPT_VARIABLE) && name_style == EditorPropertyNameProcessor::STYLE_LOCALIZED) {
	name_style = EditorPropertyNameProcessor::STYLE_CAPITALIZED;
	}
	const String property_label_string = EditorPropertyNameProcessor::get_singleton()->process_name(name_override, name_style, p.name, doc_name) + feature_tag;

I'd consider this a known issue for now which can be optimized later.

The warning icon next to a property should [...] centered

Done by adding 50% extra width. I also noticed that I forgot to update the minimum size, so I added that and refactored duplicate code into helper methods.

When hovering the warning icon, a tooltip [...]

It is a known limitation of this PR, since none of the icon buttons in EditorProperty have tooltips right now (insert keyframe, deletable, etc.). A follow-up PR could introduce a tooltip system for all of these buttons.

[Calinou]

The UI looks great now!

[adamscott]

I would keep Node::_get_configuration_warnings() and similar methods for backward compatibility sake. Removing the functionality would break so many plugins.

I would make it so that if configuration info is given, then it takes priority on any configuration warning given.

[RedMser]

@adamscott Current behavior is that both methods exist side-by-side and work, with _get_configuration_warnings marked as deprecated (to be removed in 5.0, which will likely break plugins anyway).
The results of both methods are merged together into one array, so nothing takes priority over the other.

I have not explicitly tested backwards compatibility for GDExtension or modules, but I would hope it works as intended when not compiling with DISABLE_DEPRECATED.
The idea is that compiling a GDExtension for 4.3 with config warnings API, and loading that into this PR, should work as expected.