From a83eebbdc73c9de1be3977e8379cbfcb1264229d Mon Sep 17 00:00:00 2001 From: Alexander Montag Date: Wed, 2 Oct 2024 18:22:56 +0200 Subject: [PATCH] Prune redundant and unify style of comments at TreeSearch(Panel) --- editor/tree_search.cpp | 73 +++++++++++++++++++++--------------------- editor/tree_search.h | 18 +++++++---- 2 files changed, 48 insertions(+), 43 deletions(-) diff --git a/editor/tree_search.cpp b/editor/tree_search.cpp index b93609f..d49c779 100644 --- a/editor/tree_search.cpp +++ b/editor/tree_search.cpp @@ -43,8 +43,9 @@ void TreeSearch::_clean_callable_cache() { ERR_FAIL_COND(!tree_reference); + HashMap new_callable_cache; - new_callable_cache.reserve(callable_cache.size()); // Efficiency + new_callable_cache.reserve(callable_cache.size()); for (int i = 0; i < ordered_tree_items.size(); i++) { TreeItem *cur_item = ordered_tree_items[i]; @@ -90,7 +91,7 @@ void TreeSearch::_clear_filter() { void TreeSearch::_highlight_tree() { ERR_FAIL_COND(!tree_reference); - // This might not be the prettiest, but it is the most efficient solution probably. + for (HashMap::Iterator it = number_matches.begin(); it != number_matches.end(); ++it) { TreeItem *tree_item = it->key; _highlight_tree_item(tree_item); @@ -105,48 +106,48 @@ void TreeSearch::_highlight_tree_item(TreeItem *p_tree_item) { return; } - // make sure to also call any draw method already defined. + // Make sure to also call any draw method already defined. Callable parent_draw_method; if (p_tree_item->get_cell_mode(0) == TreeItem::CELL_MODE_CUSTOM) { parent_draw_method = p_tree_item->get_custom_draw_callback(0); } - // if the cached draw method is already applied, do nothing. + // If the cached draw method is already applied, do nothing. if (callable_cache.has(p_tree_item) && parent_draw_method == callable_cache.get(p_tree_item)) { return; } Callable draw_callback = callable_mp(this, &TreeSearch::_draw_highlight_item).bind(parent_draw_method); - // -- this is necessary because of the modularity of this implementation - // cache render properties of entry + // This is necessary because of the modularity of this implementation. + // Cache render properties of entry. String cached_text = p_tree_item->get_text(0); Ref cached_icon = p_tree_item->get_icon(0); int cached_max_width = p_tree_item->get_icon_max_width(0); callable_cache[p_tree_item] = draw_callback; - // this removes render properties in entry + // This removes render properties in entry. p_tree_item->set_custom_draw_callback(0, draw_callback); p_tree_item->set_cell_mode(0, TreeItem::CELL_MODE_CUSTOM); - // restore render properties + // Restore render properties. p_tree_item->set_text(0, cached_text); p_tree_item->set_icon(0, cached_icon); p_tree_item->set_icon_max_width(0, cached_max_width); } -// custom draw callback for highlighting (bind the parent_drw_method to this) +// Custom draw callback for highlighting (bind the parent_drw_method to this) void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, const Callable p_parent_draw_method) { if (!p_tree_item) { return; } - // call any parent draw methods such as for probability FIRST. + // Call any parent draw methods such as for probability FIRST. p_parent_draw_method.call(p_tree_item, p_rect); - // first part: outline + // First part: outline if (matching_entries.has(p_tree_item)) { - // font info + // Font info Ref font = p_tree_item->get_custom_font(0); if (font.is_null()) { font = p_tree_item->get_tree()->get_theme_font(LW_NAME(font)); @@ -157,7 +158,7 @@ void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, font_size = p_tree_item->get_tree()->get_theme_font_size(LW_NAME(font)); } - // substring size + // Substring size String string_full = p_tree_item->get_text(0); StringSearchIndices substring_idx = _substring_bounds(string_full, _get_search_mask()); @@ -167,14 +168,14 @@ void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, String substring_before = string_full.substr(0, substring_idx.lower); Vector2 substring_before_size = font->get_string_size(substring_before, HORIZONTAL_ALIGNMENT_LEFT, -1.f, font_size); - // stylebox + // Stylebox Ref stylebox = p_tree_item->get_tree()->get_theme_stylebox(LW_NAME(Focus)); ERR_FAIL_NULL(stylebox); - // extract separation + // Extract separation float h_sep = p_tree_item->get_tree()->get_theme_constant(LW_NAME(h_separation)); - // compose draw rect + // Compose draw rect const Vector2 PADDING = Vector2(4., 2.); Rect2 draw_rect = p_rect; @@ -186,11 +187,11 @@ void TreeSearch::_draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, draw_rect.position += rect_offset - PADDING / 2; draw_rect.size = substring_match_size + PADDING; - // draw + // Draw stylebox->draw(p_tree_item->get_tree()->get_canvas_item(), draw_rect); } - // second part: draw number + // Second part: draw number int num_mat = number_matches.has(p_tree_item) ? number_matches.get(p_tree_item) : 0; if (num_mat > 0) { float h_sep = p_tree_item->get_tree()->get_theme_constant(LW_NAME(h_separation)); @@ -214,7 +215,7 @@ void TreeSearch::_update_matching_entries(const String &p_search_mask) { matching_entries = accum; } -/* this linearizes the tree into [ordered_tree_items] like so: +/* Linaerizes the tree into [ordered_tree_items] like so: - i1 - i2 - i3 @@ -226,10 +227,10 @@ void TreeSearch::_update_ordered_tree_items(TreeItem *p_tree_item) { if (p_tree_item == p_tree_item->get_tree()->get_root()) { ordered_tree_items.clear(); } - // Add the current item to the list + // Add the current item to the list. ordered_tree_items.push_back(p_tree_item); - // Recursively collect items from the first child + // Recursively collect items from the first child. TreeItem *child = p_tree_item->get_first_child(); while (child) { _update_ordered_tree_items(child); @@ -267,7 +268,7 @@ void TreeSearch::_find_matching_entries(TreeItem *p_tree_item, const String &p_s _find_matching_entries(child, p_search_mask, p_accum); } - // sort the result if we are at the root + // Sort the result if we are at the root. if (p_tree_item == p_tree_item->get_tree()->get_root()) { p_accum.sort(); } @@ -322,13 +323,13 @@ void TreeSearch::_select_item(TreeItem *p_item) { return; ERR_FAIL_COND(!tree_reference || p_item->get_tree() != tree_reference); - // first unfold ancestors + // First unfold ancestors TreeItem *ancestor = p_item->get_parent(); while (ancestor) { ancestor->set_collapsed(false); ancestor = ancestor->get_parent(); } - //then scroll to [item] + // Then scroll to [item] tree_reference->scroll_to_item(p_item); // ...and select it @@ -374,7 +375,7 @@ void TreeSearch::_select_previous_match() { _select_last_match(); return; } - // find [selected_idx] among ordered_tree_items + // Find [selected_idx] among ordered_tree_items. int selected_idx = 0; for (int i = ordered_tree_items.size() - 1; i >= 0; i--) { if (ordered_tree_items[i] == selected) { @@ -382,7 +383,7 @@ void TreeSearch::_select_previous_match() { break; } } - // find first entry before [selected_idx]. + // Find first entry before [selected_idx]. for (int i = MIN(ordered_tree_items.size() - 1, selected_idx) - 1; i >= 0; i--) { TreeItem *item = ordered_tree_items[i]; if (_vector_has_bsearch(matching_entries, item)) { @@ -390,7 +391,7 @@ void TreeSearch::_select_previous_match() { return; } } - // wrap around. + // Wrap around. _select_last_match(); } @@ -405,7 +406,7 @@ void TreeSearch::_select_next_match() { return; } - // find [selected_idx] among ordered_tree_items + // Find [selected_idx] among ordered_tree_items int selected_idx = 0; for (int i = 0; i < ordered_tree_items.size(); i++) { if (ordered_tree_items[i] == selected) { @@ -414,7 +415,7 @@ void TreeSearch::_select_next_match() { } } - // find first entry after [selected_idx]. + // Find first entry after [selected_idx]. for (int i = MAX(0, selected_idx) + 1; i < ordered_tree_items.size(); i++) { TreeItem *item = ordered_tree_items[i]; if (_vector_has_bsearch(matching_entries, item)) { @@ -422,7 +423,7 @@ void TreeSearch::_select_next_match() { return; } } - // wrap around. + // Wrap around. _select_first_match(); } @@ -448,14 +449,14 @@ void TreeSearch::notify_item_edited(TreeItem *item) { _highlight_tree_item(item); } -// Call this as a post-processing step for the already constructed tree. +// Called as a post-processing step for the already constructed tree. void TreeSearch::update_search(Tree *p_tree) { ERR_FAIL_COND(!search_panel || !p_tree); tree_reference = p_tree; if (!search_panel->is_visible() || search_panel->get_text().length() == 0) { - // clear and redraw if search was active recently. + // Clear and redraw if search was active recently. if (was_searched_recently) { _clear_filter(); number_matches.clear(); @@ -514,13 +515,13 @@ void TreeSearchPanel::_initialize_controls() { find_next_button->set_tooltip_text("Next Match"); find_prev_button->set_tooltip_text("Previous Match"); - // positioning and sizing + // Positioning and sizing set_anchors_and_offsets_preset(LayoutPreset::PRESET_BOTTOM_WIDE); set_v_size_flags(SIZE_SHRINK_CENTER); // Do not expand vertically line_edit_search->set_h_size_flags(SIZE_EXPAND_FILL); - _add_spacer(0.1); // otherwise the lineedits expand margin touches the left border. + _add_spacer(0.1); // -> Otherwise the lineedits expand margin touches the left border. add_child(line_edit_search); add_child(find_prev_button); add_child(find_next_button); @@ -542,11 +543,11 @@ void TreeSearchPanel::_add_spacer(float p_width_multiplier) { void TreeSearchPanel::_notification(int p_what) { switch (p_what) { case NOTIFICATION_READY: { - // close callbacks + // Close callbacks close_button->connect(LW_NAME(pressed), Callable(this, LW_NAME(set_visible)).bind(false)); close_button->connect(LW_NAME(pressed), Callable(this, LW_NAME(emit_signal)).bind(LW_NAME(Close))); close_button->set_shortcut(LW_GET_SHORTCUT("limbo_ai/hide_tree_search")); // TODO: use internal shortcut. - // search callbacks + // Search callbacks Callable c_update_requested = Callable(this, LW_NAME(emit_signal)).bind("update_requested"); Callable c_text_submitted = Callable(this, LW_NAME(emit_signal)).bind(LW_NAME(text_submitted)); Callable c_select_previous_match = Callable(this, LW_NAME(emit_signal)).bind("select_previous_match"); diff --git a/editor/tree_search.h b/editor/tree_search.h index 9f85f3e..4b1b039 100644 --- a/editor/tree_search.h +++ b/editor/tree_search.h @@ -53,14 +53,14 @@ private: // For TaskTree: These are updated when the tree is updated through TaskTree::_create_tree. Tree *tree_reference; - // linearized ordering of tree items. + // Linearized ordering of tree items. Vector ordered_tree_items; - // entires that match the search mask. + // Entires that match the search mask. // TODO: Decide if this can be removed. It can be implicitly inferred from number_matches. Vector matching_entries; - // number of descendant matches for each tree item. + // Number of descendant matches for each tree item. HashMap number_matches; - // custom draw-callbacks for each tree item. + // Custom draw-callbacks for each tree item. HashMap callable_cache; bool was_searched_recently = false; // Performance @@ -68,7 +68,7 @@ private: void _clean_callable_cache(); - // Update_search() calls these + // update_search() calls these void _filter_tree(); void _filter_tree(TreeItem *item, bool p_parent_matching); void _clear_filter(); @@ -76,7 +76,7 @@ private: void _highlight_tree(); void _highlight_tree_item(TreeItem *p_tree_item); - // Custom draw-Callback (bind inherited Callable). + // custom draw-Callback (bind inherited Callable). void _draw_highlight_item(TreeItem *p_tree_item, const Rect2 p_rect, const Callable p_parent_draw_method); void _update_matching_entries(const String &p_search_mask); @@ -96,7 +96,8 @@ private: void _on_search_panel_closed(); - // TODO: make p_vec ref `const` once Vector::bsearch is const. See: https://github.com/godotengine/godot/pull/90341 + // TODO: make p_vec ref `const` once Vector::bsearch is const. + // See: https://github.com/godotengine/godot/pull/90341 template bool _vector_has_bsearch(Vector &p_vec, T *element) const; @@ -115,7 +116,10 @@ public: bool visible; }; + // Called as a post-processing step for the already constructed tree. void update_search(Tree *p_tree); + + // This restores the highlight-drawing if a single item got edited. void notify_item_edited(TreeItem *p_item); TreeSearch() { ERR_FAIL_MSG("TreeSearch needs a TreeSearchPanel to work properly."); }