Prune redundant and unify style of comments at TreeSearch(Panel)

+ Add tooltips for buttons

editor/tree_search.cpp

@@ -43,8 +43,9 @@
 
 void TreeSearch::_clean_callable_cache() {
 	ERR_FAIL_COND(!tree_reference);
+
 	HashMap<TreeItem *, Callable> 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];
@@ -57,15 +58,17 @@ void TreeSearch::_clean_callable_cache() {
 
 void TreeSearch::_filter_tree() {
 	ERR_FAIL_COND(!tree_reference);
+	if (matching_entries.is_empty()) {
+		return;
+	}
 	_filter_tree(tree_reference->get_root(), false);
 }
 
 void TreeSearch::_filter_tree(TreeItem *p_item, bool p_parent_matching) {
 	bool visible = number_matches.has(p_item) | p_parent_matching;
 
-	if (!visible) {
+	p_item->set_visible(visible);
-		p_item->set_visible(visible);
+
-	}
 	bool is_matching = _vector_has_bsearch(matching_entries, p_item);
 	for (int i = 0; i < p_item->get_child_count(); i++) {
 		_filter_tree(p_item->get_child(i), is_matching | p_parent_matching);
@@ -88,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<TreeItem *, int>::Iterator it = number_matches.begin(); it != number_matches.end(); ++it) {
 		TreeItem *tree_item = it->key;
 		_highlight_tree_item(tree_item);
@@ -103,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
+	// This is necessary because of the modularity of this implementation.
-	// cache render properties of entry
+	// Cache render properties of entry.
 	String cached_text = p_tree_item->get_text(0);
 	Ref<Texture2D> 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> font = p_tree_item->get_custom_font(0);
 		if (font.is_null()) {
 			font = p_tree_item->get_tree()->get_theme_font(LW_NAME(font));
@@ -155,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());
 
@@ -165,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> 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;
 
@@ -184,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));
@@ -212,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
@@ -224,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);
@@ -265,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();
 	}
@@ -320,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
@@ -348,18 +351,63 @@ void TreeSearch::_select_first_match() {
 	}
 }
 
+void TreeSearch::_select_last_match() {
+	if (matching_entries.size() == 0) {
+		return;
+	}
+	for (int i = ordered_tree_items.size() - 1; i >= 0; i--) {
+		TreeItem *item = ordered_tree_items[i];
+		if (!_vector_has_bsearch(matching_entries, item)) {
+			continue;
+		}
+		_select_item(item);
+		return;
+	}
+}
+
+void TreeSearch::_select_previous_match() {
+	if (matching_entries.size() == 0) {
+		return;
+	}
+
+	TreeItem *selected = tree_reference->get_selected();
+	if (!selected) {
+		_select_last_match();
+		return;
+	}
+	// 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) {
+			selected_idx = i;
+			break;
+		}
+	}
+	// 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)) {
+			_select_item(item);
+			return;
+		}
+	}
+	// Wrap around.
+	_select_last_match();
+}
+
 void TreeSearch::_select_next_match() {
 	if (matching_entries.size() == 0) {
 		return;
 	}
+
 	TreeItem *selected = tree_reference->get_selected();
 	if (!selected) {
 		_select_first_match();
 		return;
 	}
 
-	// find [selected_idx] among ordered_tree_items
+	// Find [selected_idx] among ordered_tree_items
-	int selected_idx = -1;
+	int selected_idx = 0;
 	for (int i = 0; i < ordered_tree_items.size(); i++) {
 		if (ordered_tree_items[i] == selected) {
 			selected_idx = i;
@@ -367,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)) {
@@ -375,7 +423,7 @@ void TreeSearch::_select_next_match() {
 			return;
 		}
 	}
-	// wrap around.
+	// Wrap around.
 	_select_first_match();
 }
 
@@ -401,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();
@@ -427,7 +475,6 @@ void TreeSearch::update_search(Tree *p_tree) {
 	_update_matching_entries(search_mask);
 	_update_number_matches();
 
-	_clear_filter();
 	_highlight_tree();
 	if (search_mode == TreeSearchMode::FILTER) {
 		_filter_tree();
@@ -443,6 +490,7 @@ TreeSearch::TreeSearch(TreeSearchPanel *p_search_panel) {
 	search_panel = p_search_panel;
 	search_panel->connect(LW_NAME(text_submitted), callable_mp(this, &TreeSearch::_select_next_match));
 	search_panel->connect(LW_NAME(Close), callable_mp(this, &TreeSearch::_on_search_panel_closed));
+	search_panel->connect("select_previous_match", callable_mp(this, &TreeSearch::_select_previous_match));
 }
 
 /* !TreeSearch */
@@ -453,20 +501,30 @@ void TreeSearchPanel::_initialize_controls() {
 	line_edit_search = memnew(LineEdit);
 	check_button_filter_highlight = memnew(CheckBox);
 	close_button = memnew(Button);
+	find_next_button = memnew(Button);
+	find_prev_button = memnew(Button);
 	label_filter = memnew(Label);
 
 	line_edit_search->set_placeholder(TTR("Search tree"));
 
 	close_button->set_theme_type_variation(LW_NAME(FlatButton));
+	find_next_button->set_theme_type_variation(LW_NAME(FlatButton));
+	find_prev_button->set_theme_type_variation(LW_NAME(FlatButton));
 
-	// positioning and sizing
+	close_button->set_tooltip_text("Hide");
+	find_next_button->set_tooltip_text("Next Match");
+	find_prev_button->set_tooltip_text("Previous Match");
+
+	// 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.25); // 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);
 	_add_spacer(0.25);
 
 	add_child(check_button_filter_highlight);
@@ -474,7 +532,6 @@ void TreeSearchPanel::_initialize_controls() {
 
 	_add_spacer(0.25);
 	add_child(close_button);
-	_add_spacer(0.25);
 }
 
 void TreeSearchPanel::_add_spacer(float p_width_multiplier) {
@@ -486,13 +543,16 @@ 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"));
+			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((Object *)this, LW_NAME(emit_signal)).bind(LW_NAME(text_submitted));
+			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");
+			find_next_button->connect(LW_NAME(pressed), c_text_submitted);
+			find_prev_button->connect(LW_NAME(pressed), c_select_previous_match);
 
 			line_edit_search->connect(LW_NAME(text_changed), c_update_requested.unbind(1));
 			check_button_filter_highlight->connect(LW_NAME(pressed), c_update_requested);
@@ -501,6 +561,8 @@ void TreeSearchPanel::_notification(int p_what) {
 		}
 		case NOTIFICATION_THEME_CHANGED: {
 			BUTTON_SET_ICON(close_button, get_theme_icon(LW_NAME(Close), LW_NAME(EditorIcons)));
+			BUTTON_SET_ICON(find_prev_button, get_theme_icon("MoveUp", LW_NAME(EditorIcons)));
+			BUTTON_SET_ICON(find_next_button, get_theme_icon("MoveDown", LW_NAME(EditorIcons)));
 			label_filter->set_text(TTR("Filter"));
 			break;
 		}
@@ -510,6 +572,7 @@ void TreeSearchPanel::_notification(int p_what) {
 void TreeSearchPanel::_bind_methods() {
 	ADD_SIGNAL(MethodInfo("update_requested"));
 	ADD_SIGNAL(MethodInfo(LW_NAME(text_submitted)));
+	ADD_SIGNAL(MethodInfo("select_previous_match"));
 	ADD_SIGNAL(MethodInfo(LW_NAME(Close)));
 }
 

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<TreeItem *> 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<TreeItem *> matching_entries;
-	// number of descendant matches for each tree item.
+	// Number of descendant matches for each tree item.
 	HashMap<TreeItem *, int> number_matches;
-	// custom draw-callbacks for each tree item.
+	// Custom draw-callbacks for each tree item.
 	HashMap<TreeItem *, Callable> 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);
@@ -89,11 +89,15 @@ private:
 
 	void _select_item(TreeItem *p_item);
 	void _select_first_match();
+	void _select_last_match();
+
+	void _select_previous_match();
 	void _select_next_match();
 
 	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 <typename T>
 	bool _vector_has_bsearch(Vector<T *> &p_vec, T *element) const;
 
@@ -112,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."); }
@@ -127,6 +134,8 @@ class TreeSearchPanel : public HFlowContainer {
 private:
 	Button *toggle_button_filter_highlight;
 	Button *close_button;
+	Button *find_next_button;
+	Button *find_prev_button;
 	Label *label_filter;
 	LineEdit *line_edit_search;
 	CheckBox *check_button_filter_highlight;