From 60012d984ba970ffe7a57029693f3fe9f36e6024 Mon Sep 17 00:00:00 2001 From: Serhii Snitsaruk Date: Wed, 22 Nov 2023 13:05:55 +0100 Subject: [PATCH] Fix drag & drop position in the same branch Also, refactor get_child_index() into get_index(). --- bt/tasks/bt_task.cpp | 43 +++++++++++++++++-------------- bt/tasks/bt_task.h | 12 +++++---- doc_classes/BTTask.xml | 5 ++-- editor/limbo_ai_editor_plugin.cpp | 38 +++++++++++++++------------ editor/task_tree.cpp | 4 +-- tests/test_task.h | 34 +++++++++++++++++++----- 6 files changed, 83 insertions(+), 53 deletions(-) diff --git a/bt/tasks/bt_task.cpp b/bt/tasks/bt_task.cpp index dcc8ba0..4490b5e 100644 --- a/bt/tasks/bt_task.cpp +++ b/bt/tasks/bt_task.cpp @@ -67,6 +67,7 @@ void BTTask::_set_children(Array p_children) { Variant task_var = p_children[i]; Ref task_ref = task_var; task_ref->data.parent = this; + task_ref->data.index = i; data.children.set(i, task_var); } } @@ -117,6 +118,7 @@ Ref BTTask::clone() const { Ref c = get_child(i)->clone(); if (c.is_valid()) { c->data.parent = inst.ptr(); + c->data.index = i; inst->data.children.set(i - num_null, c); } else { num_null += 1; @@ -219,6 +221,7 @@ int BTTask::get_child_count_excluding_comments() const { void BTTask::add_child(Ref p_child) { ERR_FAIL_COND_MSG(p_child->get_parent().is_valid(), "p_child already has a parent!"); p_child->data.parent = this; + p_child->data.index = data.children.size(); data.children.push_back(p_child); emit_changed(); } @@ -230,23 +233,34 @@ void BTTask::add_child_at_index(Ref p_child, int p_idx) { } data.children.insert(p_idx, p_child); p_child->data.parent = this; + p_child->data.index = p_idx; + for (int i = p_idx + 1; i < data.children.size(); i++) { + get_child(i)->data.index = i; + } emit_changed(); } void BTTask::remove_child(Ref p_child) { int idx = data.children.find(p_child); - if (idx == -1) { - ERR_FAIL_MSG("p_child not found!"); - } else { - data.children.remove_at(idx); - p_child->data.parent = nullptr; - emit_changed(); + ERR_FAIL_COND_MSG(idx == -1, "p_child not found!"); + data.children.remove_at(idx); + p_child->data.parent = nullptr; + p_child->data.index = -1; + for (int i = idx; i < data.children.size(); i++) { + get_child(i)->data.index = i; } + emit_changed(); } void BTTask::remove_child_at_index(int p_idx) { ERR_FAIL_INDEX(p_idx, get_child_count()); + data.children[p_idx]->data.parent = nullptr; + data.children[p_idx]->data.index = -1; data.children.remove_at(p_idx); + for (int i = p_idx; i < data.children.size(); i++) { + get_child(i)->data.index = i; + } + emit_changed(); } bool BTTask::has_child(const Ref &p_child) const { @@ -264,15 +278,10 @@ bool BTTask::is_descendant_of(const Ref &p_task) const { return false; } -int BTTask::get_child_index(const Ref &p_child) const { - return data.children.find(p_child); -} - Ref BTTask::next_sibling() const { if (data.parent != nullptr) { - int idx = data.parent->get_child_index(Ref(this)); - if (idx != -1 && data.parent->get_child_count() > (idx + 1)) { - return data.parent->get_child(idx + 1); + if (get_index() != -1 && data.parent->get_child_count() > (get_index() + 1)) { + return data.parent->get_child(get_index() + 1); } } return Ref(); @@ -316,7 +325,7 @@ void BTTask::_bind_methods() { ClassDB::bind_method(D_METHOD("remove_child_at_index", "p_idx"), &BTTask::remove_child_at_index); ClassDB::bind_method(D_METHOD("has_child", "p_child"), &BTTask::has_child); ClassDB::bind_method(D_METHOD("is_descendant_of", "p_task"), &BTTask::is_descendant_of); - ClassDB::bind_method(D_METHOD("get_child_index", "p_child"), &BTTask::get_child_index); + ClassDB::bind_method(D_METHOD("get_index"), &BTTask::get_index); ClassDB::bind_method(D_METHOD("next_sibling"), &BTTask::next_sibling); ClassDB::bind_method(D_METHOD("print_tree", "p_initial_tabs"), &BTTask::print_tree, Variant(0)); ClassDB::bind_method(D_METHOD("get_task_name"), &BTTask::get_task_name); @@ -351,12 +360,6 @@ void BTTask::_bind_methods() { } BTTask::BTTask() { - data.custom_name = String(); - data.agent = nullptr; - data.parent = nullptr; - data.children = Vector>(); - data.status = FRESH; - data.elapsed = 0.0; } BTTask::~BTTask() { diff --git a/bt/tasks/bt_task.h b/bt/tasks/bt_task.h index da94d8b..806bf71 100644 --- a/bt/tasks/bt_task.h +++ b/bt/tasks/bt_task.h @@ -20,6 +20,7 @@ #include "core/object/ref_counted.h" #include "core/string/ustring.h" #include "core/templates/vector.h" +#include "core/typedefs.h" #include "core/variant/array.h" #include "core/variant/binder_common.h" #include "core/variant/dictionary.h" @@ -55,13 +56,14 @@ private: // Avoid namespace pollution in derived classes. struct Data { + int index = -1; String custom_name; - Node *agent; + Node *agent = nullptr; Ref blackboard; - BTTask *parent; + BTTask *parent = nullptr; Vector> children; - Status status; - double elapsed; + Status status = FRESH; + double elapsed = 0.0; } data; Array _get_children() const; @@ -116,7 +118,7 @@ public: void remove_child_at_index(int p_idx); bool has_child(const Ref &p_child) const; bool is_descendant_of(const Ref &p_task) const; - int get_child_index(const Ref &p_child) const; + _FORCE_INLINE_ int get_index() const { return data.index; } Ref next_sibling() const; void print_tree(int p_initial_tabs = 0) const; diff --git a/doc_classes/BTTask.xml b/doc_classes/BTTask.xml index dcce34e..8bef4f8 100644 --- a/doc_classes/BTTask.xml +++ b/doc_classes/BTTask.xml @@ -108,11 +108,10 @@ Returns the number of child tasks not counting [BTComment] tasks. - + - - Returns the child task's index. If [code]p_child[/code] is not a child of the task, [code]-1[/code] is returned instead. + Returns the task's position in the behavior tree branch. Returns [code]-1[/code] if the task doesn't belong to a task tree, i.e. doesn't have a parent. diff --git a/editor/limbo_ai_editor_plugin.cpp b/editor/limbo_ai_editor_plugin.cpp index bf66f12..846fcd8 100644 --- a/editor/limbo_ai_editor_plugin.cpp +++ b/editor/limbo_ai_editor_plugin.cpp @@ -105,7 +105,7 @@ void LimboAIEditor::_remove_task(const Ref &p_task) { undo_redo->add_undo_method(task_tree->get_bt().ptr(), SNAME("set_root_task"), task_tree->get_bt()->get_root_task()); } else { undo_redo->add_do_method(p_task->get_parent().ptr(), SNAME("remove_child"), p_task); - undo_redo->add_undo_method(p_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, p_task->get_parent()->get_child_index(p_task)); + undo_redo->add_undo_method(p_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, p_task->get_index()); } undo_redo->add_do_method(task_tree, SNAME("update_tree")); undo_redo->add_undo_method(task_tree, SNAME("update_tree")); @@ -347,7 +347,7 @@ void LimboAIEditor::_action_selected(int p_id) { Ref sel = task_tree->get_selected(); if (sel.is_valid() && sel->get_parent().is_valid()) { Ref parent = sel->get_parent(); - int idx = parent->get_child_index(sel); + int idx = sel->get_index(); if (idx > 0 && idx < parent->get_child_count()) { undo_redo->create_action(TTR("Move BT Task")); undo_redo->add_do_method(parent.ptr(), SNAME("remove_child"), sel); @@ -365,7 +365,7 @@ void LimboAIEditor::_action_selected(int p_id) { Ref sel = task_tree->get_selected(); if (sel.is_valid() && sel->get_parent().is_valid()) { Ref parent = sel->get_parent(); - int idx = parent->get_child_index(sel); + int idx = sel->get_index(); if (idx >= 0 && idx < (parent->get_child_count() - 1)) { undo_redo->create_action(TTR("Move BT Task")); undo_redo->add_do_method(parent.ptr(), SNAME("remove_child"), sel); @@ -388,7 +388,7 @@ void LimboAIEditor::_action_selected(int p_id) { parent = sel; } const Ref &sel_dup = sel->clone(); - undo_redo->add_do_method(parent.ptr(), SNAME("add_child_at_index"), sel_dup, parent->get_child_index(sel) + 1); + undo_redo->add_do_method(parent.ptr(), SNAME("add_child_at_index"), sel_dup, sel->get_index() + 1); undo_redo->add_undo_method(parent.ptr(), SNAME("remove_child"), sel_dup); undo_redo->add_do_method(task_tree, SNAME("update_tree")); undo_redo->add_undo_method(task_tree, SNAME("update_tree")); @@ -408,7 +408,7 @@ void LimboAIEditor::_action_selected(int p_id) { undo_redo->add_do_method(sel.ptr(), SNAME("add_child"), old_root); undo_redo->add_undo_method(sel.ptr(), SNAME("remove_child"), old_root); undo_redo->add_undo_method(task_tree->get_bt().ptr(), SNAME("set_root_task"), old_root); - undo_redo->add_undo_method(parent.ptr(), SNAME("add_child_at_index"), sel, parent->get_child_index(sel)); + undo_redo->add_undo_method(parent.ptr(), SNAME("add_child_at_index"), sel, sel->get_index()); undo_redo->add_do_method(task_tree, SNAME("update_tree")); undo_redo->add_undo_method(task_tree, SNAME("update_tree")); undo_redo->commit_action(); @@ -424,7 +424,7 @@ void LimboAIEditor::_action_selected(int p_id) { undo_redo->add_undo_method(task_tree->get_bt().ptr(), SNAME("set_root_task"), task_tree->get_bt()->get_root_task()); } else { undo_redo->add_do_method(sel->get_parent().ptr(), SNAME("remove_child"), sel); - undo_redo->add_undo_method(sel->get_parent().ptr(), SNAME("add_child_at_index"), sel, sel->get_parent()->get_child_index(sel)); + undo_redo->add_undo_method(sel->get_parent().ptr(), SNAME("add_child_at_index"), sel, sel->get_index()); } undo_redo->add_do_method(task_tree, SNAME("update_tree")); undo_redo->add_undo_method(task_tree, SNAME("update_tree")); @@ -442,9 +442,9 @@ void LimboAIEditor::_on_probability_edited(double p_value) { Ref probability_selector = selected->get_parent(); ERR_FAIL_COND(probability_selector.is_null()); if (percent_mode->is_pressed()) { - probability_selector->set_probability(probability_selector->get_child_index(selected), p_value * 0.01); + probability_selector->set_probability(selected->get_index(), p_value * 0.01); } else { - probability_selector->set_weight(probability_selector->get_child_index(selected), p_value); + probability_selector->set_weight(selected->get_index(), p_value); } } @@ -453,7 +453,7 @@ void LimboAIEditor::_update_probability_edit() { ERR_FAIL_COND(selected.is_null()); Ref prob = selected->get_parent(); ERR_FAIL_COND(prob.is_null()); - double others_weight = prob->get_total_weight() - prob->get_weight(prob->get_child_index(selected)); + double others_weight = prob->get_total_weight() - prob->get_weight(selected->get_index()); bool cannot_edit_percent = others_weight == 0.0; percent_mode->set_disabled(cannot_edit_percent); if (cannot_edit_percent && percent_mode->is_pressed()) { @@ -611,15 +611,21 @@ void LimboAIEditor::_on_task_dragged(Ref p_task, Ref p_to_task, if (p_type == 0) { undo_redo->add_do_method(p_to_task.ptr(), SNAME("add_child"), p_task); undo_redo->add_undo_method(p_to_task.ptr(), SNAME("remove_child"), p_task); - } else if (p_type == -1) { - undo_redo->add_do_method(p_to_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, p_to_task->get_parent()->get_child_index(p_to_task)); - undo_redo->add_undo_method(p_to_task->get_parent().ptr(), SNAME("remove_child"), p_task); - } else if (p_type == 1) { - undo_redo->add_do_method(p_to_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, p_to_task->get_parent()->get_child_index(p_to_task) + 1); - undo_redo->add_undo_method(p_to_task->get_parent().ptr(), SNAME("remove_child"), p_task); + } else { + int drop_idx = p_to_task->get_index(); + if (p_to_task->get_parent() == p_task->get_parent() && drop_idx > p_task->get_index()) { + drop_idx -= 1; + } + if (p_type == -1) { + undo_redo->add_do_method(p_to_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, drop_idx); + undo_redo->add_undo_method(p_to_task->get_parent().ptr(), SNAME("remove_child"), p_task); + } else if (p_type == 1) { + undo_redo->add_do_method(p_to_task->get_parent().ptr(), SNAME("add_child_at_index"), p_task, drop_idx + 1); + undo_redo->add_undo_method(p_to_task->get_parent().ptr(), SNAME("remove_child"), p_task); + } } - undo_redo->add_undo_method(p_task->get_parent().ptr(), "add_child_at_index", p_task, p_task->get_parent()->get_child_index(p_task)); + undo_redo->add_undo_method(p_task->get_parent().ptr(), "add_child_at_index", p_task, p_task->get_index()); undo_redo->add_do_method(task_tree, SNAME("update_tree")); undo_redo->add_undo_method(task_tree, SNAME("update_tree")); diff --git a/editor/task_tree.cpp b/editor/task_tree.cpp index 4cbdcb7..f8da03d 100644 --- a/editor/task_tree.cpp +++ b/editor/task_tree.cpp @@ -223,7 +223,7 @@ double TaskTree::get_selected_probability_weight() const { ERR_FAIL_COND_V(selected.is_null(), 0.0); Ref probability_selector = selected->get_parent(); ERR_FAIL_COND_V(probability_selector.is_null(), 0.0); - return probability_selector->get_weight(probability_selector->get_child_index(selected)); + return probability_selector->get_weight(selected->get_index()); } double TaskTree::get_selected_probability_percent() const { @@ -231,7 +231,7 @@ double TaskTree::get_selected_probability_percent() const { ERR_FAIL_COND_V(selected.is_null(), 0.0); Ref probability_selector = selected->get_parent(); ERR_FAIL_COND_V(probability_selector.is_null(), 0.0); - return probability_selector->get_probability(probability_selector->get_child_index(selected)) * 100.0; + return probability_selector->get_probability(selected->get_index()) * 100.0; } bool TaskTree::selected_has_probability() const { diff --git a/tests/test_task.h b/tests/test_task.h index 2247cd0..084e982 100644 --- a/tests/test_task.h +++ b/tests/test_task.h @@ -33,6 +33,8 @@ TEST_CASE("[Modules][LimboAI] BTTask") { REQUIRE(task->get_child_count() == 2); REQUIRE(task->get_child(0) == child1); REQUIRE(task->get_child(1) == child3); + CHECK(child1->get_index() == 0); + CHECK(child3->get_index() == 1); // * add_child_at_index Ref child2 = memnew(BTTask); @@ -57,14 +59,14 @@ TEST_CASE("[Modules][LimboAI] BTTask") { Ref other = memnew(BTTask); CHECK_FALSE(task->has_child(other)); } - SUBCASE("Test get_child_index()") { - CHECK(task->get_child_index(child1) == 0); - CHECK(task->get_child_index(child2) == 1); - CHECK(task->get_child_index(child3) == 2); + SUBCASE("Test get_index()") { + CHECK(child1->get_index() == 0); + CHECK(child2->get_index() == 1); + CHECK(child3->get_index() == 2); } - SUBCASE("Test get_child_index() with an out-of-hierarchy task") { + SUBCASE("Test get_index() with an out-of-hierarchy task") { Ref other = memnew(BTTask); - CHECK(task->get_child_index(other) == -1); + CHECK(other->get_index() == -1); } SUBCASE("Test is_descendant_of()") { Ref grandchild = memnew(BTTask); @@ -83,13 +85,22 @@ TEST_CASE("[Modules][LimboAI] BTTask") { REQUIRE(task->get_child_count() == 2); CHECK(task->get_child(0) == child1); CHECK(task->get_child(1) == child3); + CHECK(child1->get_index() == 0); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == 1); task->remove_child(child3); REQUIRE(task->get_child_count() == 1); CHECK(task->get_child(0) == child1); + CHECK(child1->get_index() == 0); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == -1); task->remove_child(child1); REQUIRE(task->get_child_count() == 0); + CHECK(child1->get_index() == -1); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == -1); } SUBCASE("Test remove_child() with an out-of-hierarchy task") { Ref other = memnew(BTTask); @@ -98,18 +109,27 @@ TEST_CASE("[Modules][LimboAI] BTTask") { task->remove_child(other); ERR_PRINT_ON; } - SUBCASE("Test remove_at_index()") { + SUBCASE("Test remove_child_at_index()") { task->remove_child_at_index(1); REQUIRE(task->get_child_count() == 2); CHECK(task->get_child(0) == child1); CHECK(task->get_child(1) == child3); + CHECK(child1->get_index() == 0); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == 1); task->remove_child_at_index(1); REQUIRE(task->get_child_count() == 1); CHECK(task->get_child(0) == child1); + CHECK(child1->get_index() == 0); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == -1); task->remove_child_at_index(0); REQUIRE(task->get_child_count() == 0); + CHECK(child1->get_index() == -1); + CHECK(child2->get_index() == -1); + CHECK(child3->get_index() == -1); } SUBCASE("Test remove_child_at_index() with an out-of-bounds index") { // * Must not crash.