From bf37a9bac6ebfb09c0a374260c35ede8373ce427 Mon Sep 17 00:00:00 2001 From: RedMser Date: Wed, 5 Jul 2023 14:45:10 +0200 Subject: [PATCH] Allow configuration warnings to refer to a property This is used by the inspector so it can show a warning icon on a specific property. --- doc/classes/EditorProperty.xml | 3 + doc/classes/Node.xml | 6 +- editor/editor_inspector.cpp | 96 ++++++++++++++++++ editor/editor_inspector.h | 10 ++ editor/gui/scene_tree_editor.cpp | 50 ++-------- .../4.2-stable.expected | 7 ++ scene/main/node.compat.inc | 41 ++++++++ scene/main/node.cpp | 97 +++++++++++++++++-- scene/main/node.h | 9 +- 9 files changed, 267 insertions(+), 52 deletions(-) create mode 100644 scene/main/node.compat.inc diff --git a/doc/classes/EditorProperty.xml b/doc/classes/EditorProperty.xml index 4fd288f16df..535515165be 100644 --- a/doc/classes/EditorProperty.xml +++ b/doc/classes/EditorProperty.xml @@ -72,6 +72,9 @@ Used by the inspector, set to [code]true[/code] when the property is checked. + + Used by the inspector, set to show a configuration warning on the property. + Used by the inspector, set to [code]true[/code] when the property can be deleted by the user. diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index 4f09a0c6162..f00bc594cbe 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -37,9 +37,13 @@ - + The elements in the array returned from this method are displayed as warnings in the Scene dock if the script that overrides it is a [code]tool[/code] script. + Each array element must either be a [String] or a [Dictionary]. + A dictionary element must contain a key [code]message[/code] of type [String] which is shown in the user interface. + The dictionary may optionally contain a key [code]property[/code] of type [NodePath], which also shows this warning in the inspector on the corresponding property. + If a string is found in the returned array, it is converted to an equivalent dictionary with the [code]message[/code] field set. Returning an empty array produces no warnings. Call [method update_configuration_warnings] when the warnings need to be updated for this node. [codeblock] diff --git a/editor/editor_inspector.cpp b/editor/editor_inspector.cpp index b97729db7b1..4501c52bc8e 100644 --- a/editor/editor_inspector.cpp +++ b/editor/editor_inspector.cpp @@ -214,6 +214,22 @@ void EditorProperty::_notification(int p_what) { text_size -= close->get_width() + 4 * EDSCALE; } } + + if (!configuration_warning.is_empty() && !read_only) { + Ref warning; + + warning = get_theme_icon(SNAME("NodeWarning"), SNAME("EditorIcons")); + + rect.size.x -= warning->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree")); + + if (is_layout_rtl()) { + rect.position.x += warning->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree")); + } + + if (no_children) { + text_size -= warning->get_width() + 4 * EDSCALE; + } + } } //set children @@ -399,6 +415,38 @@ void EditorProperty::_notification(int p_what) { } else { delete_rect = Rect2(); } + + if (!configuration_warning.is_empty() && !read_only) { + Ref warning; + + StringName warning_icon; + Node *node = Object::cast_to(object); + if (node) { + const int warning_num = node->get_configuration_warnings_of_property(property_path).size(); + warning_icon = Node::get_configuration_warning_icon(warning_num); + } else { + // This shouldn't happen, but let's not crash over an icon. + warning_icon = "NodeWarning"; + } + warning = get_theme_icon(warning_icon, SNAME("EditorIcons")); + + ofs -= warning->get_width() + get_theme_constant(SNAME("hseparator"), SNAME("Tree")); + + Color color2(1, 1, 1); + if (configuration_warning_hover) { + color2.r *= 1.2; + color2.g *= 1.2; + color2.b *= 1.2; + } + configuration_warning_rect = Rect2(ofs, ((size.height - warning->get_height()) / 2), warning->get_width(), warning->get_height()); + if (rtl) { + draw_texture(warning, Vector2(size.width - configuration_warning_rect.position.x - warning->get_width(), configuration_warning_rect.position.y), color2); + } else { + draw_texture(warning, configuration_warning_rect.position, color2); + } + } else { + configuration_warning_rect = Rect2(); + } } break; } } @@ -674,6 +722,12 @@ void EditorProperty::gui_input(const Ref &p_event) { check_hover = new_check_hover; queue_redraw(); } + + bool new_configuration_warning_hover = configuration_warning_rect.has_point(mpos) && !button_left; + if (new_configuration_warning_hover != configuration_warning_hover) { + configuration_warning_hover = new_configuration_warning_hover; + queue_redraw(); + } } Ref mb = p_event; @@ -730,6 +784,16 @@ void EditorProperty::gui_input(const Ref &p_event) { queue_redraw(); emit_signal(SNAME("property_checked"), property, checked); } + + if (configuration_warning_rect.has_point(mpos)) { + if (warning_dialog == nullptr) { + warning_dialog = memnew(AcceptDialog); + add_child(warning_dialog); + warning_dialog->set_title(TTR("Node Configuration Warning!")); + } + warning_dialog->set_text(configuration_warning); + warning_dialog->popup_centered(); + } } else if (mb.is_valid() && mb->is_pressed() && mb->get_button_index() == MouseButton::RIGHT) { accept_event(); _update_popup(); @@ -855,6 +919,16 @@ float EditorProperty::get_name_split_ratio() const { return split_ratio; } +void EditorProperty::set_configuration_warning(const String &p_configuration_warning) { + configuration_warning = p_configuration_warning; + queue_redraw(); + queue_sort(); +} + +String EditorProperty::get_configuration_warning() const { + return configuration_warning; +} + void EditorProperty::set_object_and_property(Object *p_object, const StringName &p_property) { object = p_object; property = p_property; @@ -911,6 +985,15 @@ void EditorProperty::_update_pin_flags() { } } +void EditorProperty::_update_configuration_warnings() { + Node *node = Object::cast_to(object); + if (node) { + const PackedStringArray warnings = node->get_configuration_warnings_as_strings(true, property_path); + const String warning_lines = String("\n").join(warnings); + set_configuration_warning(warning_lines); + } +} + Control *EditorProperty::make_custom_tooltip(const String &p_text) const { EditorHelpBit *tooltip = nullptr; @@ -980,6 +1063,9 @@ void EditorProperty::_bind_methods() { ClassDB::bind_method(D_METHOD("set_deletable", "deletable"), &EditorProperty::set_deletable); ClassDB::bind_method(D_METHOD("is_deletable"), &EditorProperty::is_deletable); + ClassDB::bind_method(D_METHOD("set_configuration_warning", "configuration_warning"), &EditorProperty::set_configuration_warning); + ClassDB::bind_method(D_METHOD("get_configuration_warning"), &EditorProperty::get_configuration_warning); + ClassDB::bind_method(D_METHOD("get_edited_property"), &EditorProperty::get_edited_property); ClassDB::bind_method(D_METHOD("get_edited_object"), &EditorProperty::get_edited_object); @@ -997,6 +1083,7 @@ void EditorProperty::_bind_methods() { ADD_PROPERTY(PropertyInfo(Variant::BOOL, "draw_warning"), "set_draw_warning", "is_draw_warning"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "keying"), "set_keying", "is_keying"); ADD_PROPERTY(PropertyInfo(Variant::BOOL, "deletable"), "set_deletable", "is_deletable"); + ADD_PROPERTY(PropertyInfo(Variant::STRING, "configuration_warning"), "set_configuration_warning", "get_configuration_warning"); ADD_SIGNAL(MethodInfo("property_changed", PropertyInfo(Variant::STRING_NAME, "property"), PropertyInfo(Variant::NIL, "value", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_NIL_IS_VARIANT), PropertyInfo(Variant::STRING_NAME, "field"), PropertyInfo(Variant::BOOL, "changing"))); ADD_SIGNAL(MethodInfo("multiple_properties_changed", PropertyInfo(Variant::PACKED_STRING_ARRAY, "properties"), PropertyInfo(Variant::ARRAY, "value"))); @@ -3314,6 +3401,7 @@ void EditorInspector::update_tree() { ep->set_keying(keying); ep->set_read_only(property_read_only || all_read_only); ep->set_deletable(deletable_properties || p.name.begins_with("metadata/")); + ep->_update_configuration_warnings(); } current_vbox->add_child(editors[i].property_editor); @@ -3960,6 +4048,12 @@ void EditorInspector::_node_removed(Node *p_node) { } } +void EditorInspector::_warning_changed(Node *p_node) { + if (p_node == object) { + update_tree_pending = true; + } +} + void EditorInspector::_notification(int p_what) { switch (p_what) { case NOTIFICATION_READY: { @@ -3971,6 +4065,7 @@ void EditorInspector::_notification(int p_what) { case NOTIFICATION_ENTER_TREE: { if (!sub_inspector) { get_tree()->connect("node_removed", callable_mp(this, &EditorInspector::_node_removed)); + get_tree()->connect("node_configuration_warning_changed", callable_mp(this, &EditorInspector::_warning_changed)); } } break; @@ -3981,6 +4076,7 @@ void EditorInspector::_notification(int p_what) { case NOTIFICATION_EXIT_TREE: { if (!sub_inspector) { get_tree()->disconnect("node_removed", callable_mp(this, &EditorInspector::_node_removed)); + get_tree()->disconnect("node_configuration_warning_changed", callable_mp(this, &EditorInspector::_warning_changed)); } edit(nullptr); } break; diff --git a/editor/editor_inspector.h b/editor/editor_inspector.h index 2dcad88840f..d8b3e32d7d1 100644 --- a/editor/editor_inspector.h +++ b/editor/editor_inspector.h @@ -76,6 +76,7 @@ private: String doc_path; bool internal = false; bool has_doc_tooltip = false; + AcceptDialog *warning_dialog = nullptr; int property_usage; @@ -98,6 +99,8 @@ private: bool check_hover = false; Rect2 delete_rect; bool delete_hover = false; + Rect2 configuration_warning_rect; + bool configuration_warning_hover = false; bool can_revert = false; bool can_pin = false; @@ -121,12 +124,15 @@ private: Control *bottom_editor = nullptr; PopupMenu *menu = nullptr; + String configuration_warning; + HashMap cache; GDVIRTUAL0(_update_property) GDVIRTUAL1(_set_read_only, bool) void _update_pin_flags(); + void _update_configuration_warnings(); protected: void _notification(int p_what); @@ -203,6 +209,9 @@ public: void set_name_split_ratio(float p_ratio); float get_name_split_ratio() const; + void set_configuration_warning(const String &p_configuration_warning); + String get_configuration_warning() const; + void set_object_and_property(Object *p_object, const StringName &p_property); virtual Control *make_custom_tooltip(const String &p_text) const override; @@ -532,6 +541,7 @@ class EditorInspector : public ScrollContainer { void _object_id_selected(const String &p_path, ObjectID p_id); void _node_removed(Node *p_node); + void _warning_changed(Node *p_node); HashMap per_array_page; void _page_change_request(int p_new_page, const StringName &p_array_prefix); diff --git a/editor/gui/scene_tree_editor.cpp b/editor/gui/scene_tree_editor.cpp index 1f35da322a2..6c8e8b75efc 100644 --- a/editor/gui/scene_tree_editor.cpp +++ b/editor/gui/scene_tree_editor.cpp @@ -132,32 +132,13 @@ void SceneTreeEditor::_cell_button_pressed(Object *p_item, int p_column, int p_i } undo_redo->commit_action(); } else if (p_id == BUTTON_WARNING) { - const PackedStringArray warnings = n->get_configuration_warnings(); - + const PackedStringArray warnings = n->get_configuration_warnings_as_strings(true); if (warnings.is_empty()) { return; } - // Improve looks on tooltip, extra spacing on non-bullet point newlines. - const String bullet_point = U"• "; - String all_warnings; - for (const String &w : warnings) { - all_warnings += "\n" + bullet_point + w; - } - - // Limit the line width while keeping some padding. - // It is not efficient, but it does not have to be. - const PackedInt32Array boundaries = TS->string_get_word_breaks(all_warnings, "", 80); - PackedStringArray lines; - for (int i = 0; i < boundaries.size(); i += 2) { - const int start = boundaries[i]; - const int end = boundaries[i + 1]; - const String line = all_warnings.substr(start, end - start); - lines.append(line); - } - all_warnings = String("\n").join(lines).indent(" ").replace(U" •", U"\n•").substr(2); // We don't want the first two newlines. - - warning->set_text(all_warnings); + const String warning_lines = String("\n").join(warnings); + warning->set_text(warning_lines); warning->popup_centered(); } else if (p_id == BUTTON_SIGNALS) { @@ -294,29 +275,12 @@ void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) { if (can_rename) { //should be can edit.. - const PackedStringArray warnings = p_node->get_configuration_warnings(); + const PackedStringArray warnings = p_node->get_configuration_warnings_as_strings(false); const int num_warnings = warnings.size(); if (num_warnings > 0) { - String warning_icon; - if (num_warnings == 1) { - warning_icon = SNAME("NodeWarning"); - } else if (num_warnings <= 3) { - warning_icon = vformat("NodeWarnings%d", num_warnings); - } else { - warning_icon = SNAME("NodeWarnings4Plus"); - } - - // Improve looks on tooltip, extra spacing on non-bullet point newlines. - const String bullet_point = U"• "; - String all_warnings; - for (const String &w : warnings) { - all_warnings += "\n\n" + bullet_point + w.replace("\n", "\n "); - } - if (num_warnings == 1) { - all_warnings.remove_at(0); // With only one warning, two newlines do not look great. - } - - item->add_button(0, get_editor_theme_icon(warning_icon), BUTTON_WARNING, false, TTR("Node configuration warning:") + all_warnings); + const StringName warning_icon = Node::get_configuration_warning_icon(num_warnings); + const String warning_lines = String("\n\n").join(warnings); + item->add_button(0, get_editor_theme_icon(warning_icon), BUTTON_WARNING, false, TTR("Node configuration warning:") + "\n\n" + warning_lines); } if (p_node->is_unique_name_in_owner()) { diff --git a/misc/extension_api_validation/4.2-stable.expected b/misc/extension_api_validation/4.2-stable.expected index 04e046ec932..9389fe53f16 100644 --- a/misc/extension_api_validation/4.2-stable.expected +++ b/misc/extension_api_validation/4.2-stable.expected @@ -82,3 +82,10 @@ Validate extension JSON: Error: Field 'classes/GPUParticles3D/properties/process Validate extension JSON: Error: Field 'classes/Sky/properties/sky_material': type changed value in new API, from "ShaderMaterial,PanoramaSkyMaterial,ProceduralSkyMaterial,PhysicalSkyMaterial" to "PanoramaSkyMaterial,ProceduralSkyMaterial,PhysicalSkyMaterial,ShaderMaterial". Property hints reordered to improve editor usability. The types allowed are still the same as before. No adjustments should be necessary. + + +GH-68420 +-------- +Validate extension JSON: Error: Field 'classes/Node/methods/_get_configuration_warnings/return_value': type changed value in new API, from "PackedStringArray" to "Array". + +Allow configuration warnings to refer to a property. Compatibility method registered. diff --git a/scene/main/node.compat.inc b/scene/main/node.compat.inc new file mode 100644 index 00000000000..7e957e5a14a --- /dev/null +++ b/scene/main/node.compat.inc @@ -0,0 +1,41 @@ +/**************************************************************************/ +/* node.compat.inc */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#ifndef DISABLE_DEPRECATED + +PackedStringArray Node::get_configuration_warnings_bind_compat_68420() const { + return PackedStringArray(get_configuration_warnings()); +} + +void Node::_bind_compatibility_methods() { + ClassDB::bind_compatibility_method(D_METHOD("get_configuration_warnings"), &Node::get_configuration_warnings_bind_compat_68420); +} + +#endif // DISABLE_DEPRECATED diff --git a/scene/main/node.cpp b/scene/main/node.cpp index b78dfb2f2c6..0a6afb3d596 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -3130,18 +3130,93 @@ void Node::clear_internal_tree_resource_paths() { } } -PackedStringArray Node::get_configuration_warnings() const { - ERR_THREAD_GUARD_V(PackedStringArray()); - PackedStringArray ret; +Array Node::get_configuration_warnings() const { + ERR_THREAD_GUARD_V(Array()); + Array warnings; + GDVIRTUAL_CALL(_get_configuration_warnings, warnings); + return warnings; +} - Vector warnings; - if (GDVIRTUAL_CALL(_get_configuration_warnings, warnings)) { - ret.append_array(warnings); +Dictionary Node::configuration_warning_to_dict(const Variant &p_warning) const { + switch (p_warning.get_type()) { + case Variant::Type::DICTIONARY: + return p_warning; + case Variant::Type::STRING: { + // Convert string to dictionary. + Dictionary warning; + warning["message"] = p_warning; + return warning; + } + default: { + ERR_FAIL_V_MSG(Dictionary(), "Node::get_configuration_warnings returned a value which is neither a string nor a dictionary, but a " + Variant::get_type_name(p_warning.get_type())); + } } +} +Vector Node::get_configuration_warnings_as_dicts() const { + Vector ret; + Array mixed = get_configuration_warnings(); + for (int i = 0; i < mixed.size(); i++) { + ret.append(configuration_warning_to_dict(mixed[i])); + } return ret; } +Vector Node::get_configuration_warnings_of_property(const String &p_property) const { + Vector ret; + Vector warnings = get_configuration_warnings_as_dicts(); + if (p_property.is_empty()) { + ret.append_array(warnings); + } else { + // Filter by property path. + for (int i = 0; i < warnings.size(); i++) { + Dictionary warning = warnings[i]; + String warning_property = warning.get("property", String()); + if (p_property == warning_property) { + ret.append(warning); + } + } + } + return ret; +} + +PackedStringArray Node::get_configuration_warnings_as_strings(bool p_wrap_lines, const String &p_property) const { + Vector warnings = get_configuration_warnings_of_property(p_property); + + const String bullet_point = U"• "; + PackedStringArray all_warnings; + for (const Dictionary &warning : warnings) { + if (!warning.has("message")) { + continue; + } + + // Prefix with property name if we are showing all warnings. + String text; + if (warning.has("property") && p_property.is_empty()) { + text = bullet_point + vformat("[%s] %s", warning["property"], warning["message"]); + } else { + text = bullet_point + static_cast(warning["message"]); + } + + if (p_wrap_lines) { + // Limit the line width while keeping some padding. + // It is not efficient, but it does not have to be. + const PackedInt32Array boundaries = TS->string_get_word_breaks(text, "", 80); + PackedStringArray lines; + for (int i = 0; i < boundaries.size(); i += 2) { + const int start = boundaries[i]; + const int end = boundaries[i + 1]; + String line = text.substr(start, end - start); + lines.append(line); + } + text = String("\n").join(lines); + } + text = text.replace("\n", "\n "); + all_warnings.append(text); + } + return all_warnings; +} + void Node::update_configuration_warnings() { ERR_THREAD_GUARD #ifdef TOOLS_ENABLED @@ -3611,6 +3686,16 @@ String Node::_get_name_num_separator() { return " "; } +StringName Node::get_configuration_warning_icon(int p_count) { + if (p_count == 1) { + return SNAME("NodeWarning"); + } else if (p_count <= 3) { + return vformat("NodeWarnings%d", p_count); + } else { + return SNAME("NodeWarnings4Plus"); + } +} + Node::Node() { orphan_node_count++; } diff --git a/scene/main/node.h b/scene/main/node.h index 48eae3bbfa6..5d246cc18e1 100644 --- a/scene/main/node.h +++ b/scene/main/node.h @@ -294,6 +294,7 @@ protected: static void _bind_methods(); static String _get_name_num_separator(); + static StringName get_configuration_warning_icon(int p_count); friend class SceneState; @@ -320,7 +321,7 @@ protected: GDVIRTUAL0(_enter_tree) GDVIRTUAL0(_exit_tree) GDVIRTUAL0(_ready) - GDVIRTUAL0RC(Vector, _get_configuration_warnings) + GDVIRTUAL0RC(Array, _get_configuration_warnings) GDVIRTUAL1(_input, Ref) GDVIRTUAL1(_shortcut_input, Ref) @@ -636,7 +637,11 @@ public: _FORCE_INLINE_ Viewport *get_viewport() const { return data.viewport; } - virtual PackedStringArray get_configuration_warnings() const; + virtual Array get_configuration_warnings() const; + Dictionary configuration_warning_to_dict(const Variant &p_warning) const; + Vector get_configuration_warnings_as_dicts() const; + Vector get_configuration_warnings_of_property(const String &p_property = String()) const; + PackedStringArray get_configuration_warnings_as_strings(bool p_wrap_lines, const String &p_property = String()) const; void update_configuration_warnings();