From 9c0afbb15ca301ae85264e79e052f106cbead344 Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Wed, 18 Sep 2024 14:01:10 -0300 Subject: [PATCH] Fixing TreeItem get_prev_xxx methods when p_wrap is true Fixes #85032 The code that fix the issue is courtesy of @Jesusemora, I just added unit tests for it and did a rebase with the latest changes on master. Co-authored-by: Jesusemora <32273722+Jesusemora@users.noreply.github.com> --- scene/gui/tree.cpp | 20 +++---- tests/scene/test_tree.h | 126 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 11 deletions(-) diff --git a/scene/gui/tree.cpp b/scene/gui/tree.cpp index e4f52ee8ee0..5e6ba953632 100644 --- a/scene/gui/tree.cpp +++ b/scene/gui/tree.cpp @@ -923,19 +923,17 @@ TreeItem *TreeItem::_get_prev_in_tree(bool p_wrap, bool p_include_invisible) { if (!prev_item) { current = current->parent; - if (current == tree->root && tree->hide_root) { - return nullptr; - } else if (!current) { - if (p_wrap) { - current = this; - TreeItem *temp = get_next_visible(); - while (temp) { - current = temp; - temp = temp->get_next_visible(); - } - } else { + if (!current || (current == tree->root && tree->hide_root)) { + if (!p_wrap) { return nullptr; } + // Wrap around to the last visible item. + current = this; + TreeItem *temp = get_next_visible(); + while (temp) { + current = temp; + temp = temp->get_next_visible(); + } } } else { current = prev_item; diff --git a/tests/scene/test_tree.h b/tests/scene/test_tree.h index e19f8311e21..a74158d328c 100644 --- a/tests/scene/test_tree.h +++ b/tests/scene/test_tree.h @@ -139,18 +139,30 @@ TEST_CASE("[SceneTree][Tree]") { TreeItem *child1 = tree->create_item(); TreeItem *child2 = tree->create_item(); TreeItem *child3 = tree->create_item(); + CHECK_EQ(root->get_next(), nullptr); + CHECK_EQ(root->get_next_visible(), child1); + CHECK_EQ(root->get_next_in_tree(), child1); CHECK_EQ(child1->get_next(), child2); + CHECK_EQ(child1->get_next_visible(), child2); CHECK_EQ(child1->get_next_in_tree(), child2); CHECK_EQ(child2->get_next(), child3); + CHECK_EQ(child2->get_next_visible(), child3); CHECK_EQ(child2->get_next_in_tree(), child3); CHECK_EQ(child3->get_next(), nullptr); + CHECK_EQ(child3->get_next_visible(), nullptr); CHECK_EQ(child3->get_next_in_tree(), nullptr); + CHECK_EQ(root->get_prev(), nullptr); + CHECK_EQ(root->get_prev_visible(), nullptr); + CHECK_EQ(root->get_prev_in_tree(), nullptr); CHECK_EQ(child1->get_prev(), nullptr); + CHECK_EQ(child1->get_prev_visible(), root); CHECK_EQ(child1->get_prev_in_tree(), root); CHECK_EQ(child2->get_prev(), child1); + CHECK_EQ(child2->get_prev_visible(), child1); CHECK_EQ(child2->get_prev_in_tree(), child1); CHECK_EQ(child3->get_prev(), child2); + CHECK_EQ(child3->get_prev_visible(), child2); CHECK_EQ(child3->get_prev_in_tree(), child2); TreeItem *nested1 = tree->create_item(child2); @@ -158,13 +170,127 @@ TEST_CASE("[SceneTree][Tree]") { TreeItem *nested3 = tree->create_item(child2); CHECK_EQ(child1->get_next(), child2); + CHECK_EQ(child1->get_next_visible(), child2); CHECK_EQ(child1->get_next_in_tree(), child2); CHECK_EQ(child2->get_next(), child3); + CHECK_EQ(child2->get_next_visible(), nested1); CHECK_EQ(child2->get_next_in_tree(), nested1); CHECK_EQ(child3->get_prev(), child2); + CHECK_EQ(child3->get_prev_visible(), nested3); CHECK_EQ(child3->get_prev_in_tree(), nested3); CHECK_EQ(nested1->get_prev_in_tree(), child2); CHECK_EQ(nested1->get_next_in_tree(), nested2); + CHECK_EQ(nested3->get_next_in_tree(), child3); + + memdelete(tree); + } + + SUBCASE("[Tree] Previous and Next items with hide root.") { + Tree *tree = memnew(Tree); + tree->set_hide_root(true); + TreeItem *root = tree->create_item(); + + TreeItem *child1 = tree->create_item(); + TreeItem *child2 = tree->create_item(); + TreeItem *child3 = tree->create_item(); + CHECK_EQ(root->get_next(), nullptr); + CHECK_EQ(root->get_next_visible(), child1); + CHECK_EQ(root->get_next_in_tree(), child1); + CHECK_EQ(child1->get_next(), child2); + CHECK_EQ(child1->get_next_visible(), child2); + CHECK_EQ(child1->get_next_in_tree(), child2); + CHECK_EQ(child2->get_next(), child3); + CHECK_EQ(child2->get_next_visible(), child3); + CHECK_EQ(child2->get_next_in_tree(), child3); + CHECK_EQ(child3->get_next(), nullptr); + CHECK_EQ(child3->get_next_visible(), nullptr); + CHECK_EQ(child3->get_next_in_tree(), nullptr); + + CHECK_EQ(root->get_prev(), nullptr); + CHECK_EQ(root->get_prev_visible(), nullptr); + CHECK_EQ(root->get_prev_in_tree(), nullptr); + CHECK_EQ(child1->get_prev(), nullptr); + CHECK_EQ(child1->get_prev_visible(), nullptr); + CHECK_EQ(child1->get_prev_in_tree(), nullptr); + CHECK_EQ(child2->get_prev(), child1); + CHECK_EQ(child2->get_prev_visible(), child1); + CHECK_EQ(child2->get_prev_in_tree(), child1); + CHECK_EQ(child3->get_prev(), child2); + CHECK_EQ(child3->get_prev_visible(), child2); + CHECK_EQ(child3->get_prev_in_tree(), child2); + + memdelete(tree); + } + + SUBCASE("[Tree] Previous and Next items wrapping.") { + Tree *tree = memnew(Tree); + TreeItem *root = tree->create_item(); + + TreeItem *child1 = tree->create_item(); + TreeItem *child2 = tree->create_item(); + TreeItem *child3 = tree->create_item(); + CHECK_EQ(root->get_next_visible(true), child1); + CHECK_EQ(root->get_next_in_tree(true), child1); + CHECK_EQ(child1->get_next_visible(true), child2); + CHECK_EQ(child1->get_next_in_tree(true), child2); + CHECK_EQ(child2->get_next_visible(true), child3); + CHECK_EQ(child2->get_next_in_tree(true), child3); + CHECK_EQ(child3->get_next_visible(true), root); + CHECK_EQ(child3->get_next_in_tree(true), root); + + CHECK_EQ(root->get_prev_visible(true), child3); + CHECK_EQ(root->get_prev_in_tree(true), child3); + CHECK_EQ(child1->get_prev_visible(true), root); + CHECK_EQ(child1->get_prev_in_tree(true), root); + CHECK_EQ(child2->get_prev_visible(true), child1); + CHECK_EQ(child2->get_prev_in_tree(true), child1); + CHECK_EQ(child3->get_prev_visible(true), child2); + CHECK_EQ(child3->get_prev_in_tree(true), child2); + + TreeItem *nested1 = tree->create_item(child2); + TreeItem *nested2 = tree->create_item(child2); + TreeItem *nested3 = tree->create_item(child2); + + CHECK_EQ(child1->get_next_visible(true), child2); + CHECK_EQ(child1->get_next_in_tree(true), child2); + CHECK_EQ(child2->get_next_visible(true), nested1); + CHECK_EQ(child2->get_next_in_tree(true), nested1); + CHECK_EQ(nested3->get_next_visible(true), child3); + CHECK_EQ(nested3->get_next_in_tree(true), child3); + CHECK_EQ(child3->get_prev_visible(true), nested3); + CHECK_EQ(child3->get_prev_in_tree(true), nested3); + CHECK_EQ(nested1->get_prev_in_tree(true), child2); + CHECK_EQ(nested1->get_next_in_tree(true), nested2); + CHECK_EQ(nested3->get_next_in_tree(true), child3); + + memdelete(tree); + } + + SUBCASE("[Tree] Previous and Next items wrapping with hide root.") { + Tree *tree = memnew(Tree); + tree->set_hide_root(true); + TreeItem *root = tree->create_item(); + + TreeItem *child1 = tree->create_item(); + TreeItem *child2 = tree->create_item(); + TreeItem *child3 = tree->create_item(); + CHECK_EQ(root->get_next_visible(true), child1); + CHECK_EQ(root->get_next_in_tree(true), child1); + CHECK_EQ(child1->get_next_visible(true), child2); + CHECK_EQ(child1->get_next_in_tree(true), child2); + CHECK_EQ(child2->get_next_visible(true), child3); + CHECK_EQ(child2->get_next_in_tree(true), child3); + CHECK_EQ(child3->get_next_visible(true), root); + CHECK_EQ(child3->get_next_in_tree(true), root); + + CHECK_EQ(root->get_prev_visible(true), child3); + CHECK_EQ(root->get_prev_in_tree(true), child3); + CHECK_EQ(child1->get_prev_visible(true), child3); + CHECK_EQ(child1->get_prev_in_tree(true), child3); + CHECK_EQ(child2->get_prev_visible(true), child1); + CHECK_EQ(child2->get_prev_in_tree(true), child1); + CHECK_EQ(child3->get_prev_visible(true), child2); + CHECK_EQ(child3->get_prev_in_tree(true), child2); memdelete(tree); }