From b8c9db0d44710e39654fa2bfc556b0d03c1d613e Mon Sep 17 00:00:00 2001 From: Serhii Snitsaruk Date: Sat, 14 Sep 2024 16:37:15 +0200 Subject: [PATCH] Fix BTForEach crash if elements are removed from the array during iteration --- bt/tasks/decorators/bt_for_each.cpp | 11 +++++++---- tests/test_for_each.h | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/bt/tasks/decorators/bt_for_each.cpp b/bt/tasks/decorators/bt_for_each.cpp index 0390586..12df366 100644 --- a/bt/tasks/decorators/bt_for_each.cpp +++ b/bt/tasks/decorators/bt_for_each.cpp @@ -43,12 +43,15 @@ void BTForEach::_enter() { } BT::Status BTForEach::_tick(double p_delta) { - ERR_FAIL_COND_V_MSG(get_child_count() == 0, FAILURE, "ForEach decorator has no child."); - ERR_FAIL_COND_V_MSG(save_var == StringName(), FAILURE, "ForEach save variable is not set."); - ERR_FAIL_COND_V_MSG(array_var == StringName(), FAILURE, "ForEach array variable is not set."); + ERR_FAIL_COND_V_MSG(get_child_count() == 0, FAILURE, "BTForEach: Decorator has no child."); + ERR_FAIL_COND_V_MSG(save_var == StringName(), FAILURE, "BTForEach: Save variable is not set."); + ERR_FAIL_COND_V_MSG(array_var == StringName(), FAILURE, "BTForEach: Array variable is not set."); Array arr = get_blackboard()->get_var(array_var, Variant()); - if (arr.size() == 0) { + if (current_idx >= arr.size()) { + if (current_idx != 0) { + WARN_PRINT("BTForEach: Array size changed during iteration."); + } return SUCCESS; } Variant elem = arr[current_idx]; diff --git a/tests/test_for_each.h b/tests/test_for_each.h index 00cad30..135e565 100644 --- a/tests/test_for_each.h +++ b/tests/test_for_each.h @@ -96,6 +96,22 @@ TEST_CASE("[Modules][LimboAI] BTForEach") { CHECK_ENTRIES_TICKS_EXITS(task, 3, 6, 3); CHECK(blackboard->get_var("element", "wetgoop") == "mushroom"); } + + SUBCASE("Shouldn't crash if elements are removed during iteration") { + CHECK(fe->execute(0.01666) == BTTask::RUNNING); + CHECK(task->get_status() == BTTask::SUCCESS); + CHECK_ENTRIES_TICKS_EXITS(task, 1, 1, 1); + CHECK(blackboard->get_var("element", "wetgoop") == "apple"); + + arr.clear(); + + ERR_PRINT_OFF; + CHECK(fe->execute(0.01666) == BTTask::SUCCESS); // Returns SUCCESS and prints a warning without executing child task. + ERR_PRINT_ON; + CHECK(task->get_status() == BTTask::SUCCESS); // Same status. + CHECK_ENTRIES_TICKS_EXITS(task, 1, 1, 1); // Task is not re-executed as there is not enough elements to continue iteration. + CHECK(blackboard->get_var("element", "wetgoop") == "apple"); // Not changed. + } } } //namespace TestForEach