From 3e4e0f08c4fcad6453259a3d8b2ab1d8c775aa32 Mon Sep 17 00:00:00 2001 From: Micky Date: Tue, 23 Jan 2024 18:47:26 +0100 Subject: [PATCH] Improve appearance of Node configuration warnings --- editor/gui/scene_tree_editor.cpp | 45 ++++++++++++++++++++------------ scene/2d/collision_shape_2d.cpp | 2 +- scene/main/node.cpp | 14 ---------- scene/main/node.h | 1 - 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/editor/gui/scene_tree_editor.cpp b/editor/gui/scene_tree_editor.cpp index 7a9df26fa7f8..6c9c8123976b 100644 --- a/editor/gui/scene_tree_editor.cpp +++ b/editor/gui/scene_tree_editor.cpp @@ -132,20 +132,32 @@ void SceneTreeEditor::_cell_button_pressed(Object *p_item, int p_column, int p_i } undo_redo->commit_action(); } else if (p_id == BUTTON_WARNING) { - String config_err = n->get_configuration_warnings_as_string(); - if (config_err.is_empty()) { + const PackedStringArray warnings = n->get_configuration_warnings(); + + if (warnings.is_empty()) { return; } - const PackedInt32Array boundaries = TS->string_get_word_breaks(config_err, "", 80); + // 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]; - lines.append(config_err.substr(start, end - start + 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(String("\n").join(lines)); + warning->set_text(all_warnings); warning->popup_centered(); } else if (p_id == BUTTON_SIGNALS) { @@ -282,9 +294,9 @@ void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) { if (can_rename) { //should be can edit.. - String conf_warning = p_node->get_configuration_warnings_as_string(); - if (!conf_warning.is_empty()) { - const int num_warnings = p_node->get_configuration_warnings().size(); + const PackedStringArray warnings = p_node->get_configuration_warnings(); + const int num_warnings = warnings.size(); + if (num_warnings > 0) { String warning_icon; if (num_warnings == 1) { warning_icon = SNAME("NodeWarning"); @@ -296,17 +308,15 @@ void SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent) { // Improve looks on tooltip, extra spacing on non-bullet point newlines. const String bullet_point = U"• "; - int next_newline = 0; - while (next_newline != -1) { - next_newline = conf_warning.find("\n", next_newline + 2); - if (conf_warning.substr(next_newline + 1, bullet_point.length()) != bullet_point) { - conf_warning = conf_warning.insert(next_newline + 1, " "); - } + 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. } - String newline = (num_warnings == 1 ? "\n" : "\n\n"); - - item->add_button(0, get_editor_theme_icon(warning_icon), BUTTON_WARNING, false, TTR("Node configuration warning:") + newline + conf_warning); + item->add_button(0, get_editor_theme_icon(warning_icon), BUTTON_WARNING, false, TTR("Node configuration warning:") + all_warnings); } if (p_node->is_unique_name_in_owner()) { @@ -1545,6 +1555,7 @@ SceneTreeEditor::SceneTreeEditor(bool p_label, bool p_can_rename, bool p_can_ope warning = memnew(AcceptDialog); add_child(warning); warning->set_title(TTR("Node Configuration Warning!")); + warning->set_flag(Window::FLAG_POPUP, true); last_hash = 0; blocked = 0; diff --git a/scene/2d/collision_shape_2d.cpp b/scene/2d/collision_shape_2d.cpp index 70ec57aa7a2a..ee413c7bc2db 100644 --- a/scene/2d/collision_shape_2d.cpp +++ b/scene/2d/collision_shape_2d.cpp @@ -178,7 +178,7 @@ PackedStringArray CollisionShape2D::get_configuration_warnings() const { CollisionObject2D *col_object = Object::cast_to(get_parent()); if (col_object == nullptr) { - warnings.push_back(RTR("CollisionShape2D only serves to provide a collision shape to a CollisionObject2D derived node. Please only use it as a child of Area2D, StaticBody2D, RigidBody2D, CharacterBody2D, etc. to give them a shape.")); + warnings.push_back(RTR("CollisionShape2D only serves to provide a collision shape to a CollisionObject2D derived node.\nPlease only use it as a child of Area2D, StaticBody2D, RigidBody2D, CharacterBody2D, etc. to give them a shape.")); } if (!shape.is_valid()) { warnings.push_back(RTR("A shape must be provided for CollisionShape2D to function. Please create a shape resource for it!")); diff --git a/scene/main/node.cpp b/scene/main/node.cpp index f7d695bf318b..defc8de9e6ef 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -3131,20 +3131,6 @@ PackedStringArray Node::get_configuration_warnings() const { return ret; } -String Node::get_configuration_warnings_as_string() const { - PackedStringArray warnings = get_configuration_warnings(); - String all_warnings; - for (int i = 0; i < warnings.size(); i++) { - if (i > 0) { - all_warnings += "\n\n"; - } - // Format as a bullet point list to make multiple warnings easier to distinguish - // from each other. - all_warnings += String::utf8("• ") + warnings[i]; - } - return all_warnings; -} - void Node::update_configuration_warnings() { ERR_THREAD_GUARD #ifdef TOOLS_ENABLED diff --git a/scene/main/node.h b/scene/main/node.h index 8130c61a3419..5966d48ed1ac 100644 --- a/scene/main/node.h +++ b/scene/main/node.h @@ -636,7 +636,6 @@ public: _FORCE_INLINE_ Viewport *get_viewport() const { return data.viewport; } virtual PackedStringArray get_configuration_warnings() const; - String get_configuration_warnings_as_string() const; void update_configuration_warnings();