From 56115cbb8381fb3496e1bd86f5eb8d60f16dd8d1 Mon Sep 17 00:00:00 2001
From: Davide Faconti <davide.faconti@gmail.com>
Date: Tue, 3 Feb 2026 09:09:56 +0100
Subject: [PATCH] Re-evaluate SUCCESS_IF and FAILURE_IF during RUNNING state
 (#917)

Previously, _successIf and _failureIf preconditions were only checked
when the node was IDLE or SKIPPED. This meant that in ReactiveSequence,
if a condition changed while an action was RUNNING, the change would
not take effect until the action completed.

Now these preconditions are also checked during RUNNING state, matching
the existing behavior of _while. When a condition triggers, the node
is halted and returns the appropriate status.

Includes tests for both _successIf and _failureIf with async actions
in ReactiveSequence that verify condition changes during RUNNING.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 src/tree_node.cpp             | 24 ++++++++--
 tests/gtest_preconditions.cpp | 85 +++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/src/tree_node.cpp b/src/tree_node.cpp
index 16171a135..b14ab5020 100644
--- a/src/tree_node.cpp
+++ b/src/tree_node.cpp
@@ -222,13 +222,29 @@ Expected<NodeStatus> TreeNode::checkPreConditions()
         return NodeStatus::SKIPPED;
       }
     }
-    else if(_p->status == NodeStatus::RUNNING && preID == PreCond::WHILE_TRUE)
+    else if(_p->status == NodeStatus::RUNNING)
     {
-      // what to do if the condition is false
-      if(!parse_executor(env).cast<bool>())
+      // Check WHILE_TRUE when running - halt if condition becomes false
+      if(preID == PreCond::WHILE_TRUE)
+      {
+        // what to do if the condition is false
+        if(!parse_executor(env).cast<bool>())
+        {
+          haltNode();
+          return NodeStatus::SKIPPED;
+        }
+      }
+      // Issue #917: Also check SUCCESS_IF and FAILURE_IF when running
+      // This allows reactive sequences to respond to condition changes
+      else if(preID == PreCond::SUCCESS_IF && parse_executor(env).cast<bool>())
       {
         haltNode();
-        return NodeStatus::SKIPPED;
+        return NodeStatus::SUCCESS;
+      }
+      else if(preID == PreCond::FAILURE_IF && parse_executor(env).cast<bool>())
+      {
+        haltNode();
+        return NodeStatus::FAILURE;
       }
     }
   }
diff --git a/tests/gtest_preconditions.cpp b/tests/gtest_preconditions.cpp
index f0dee8a32..3c78b6a19 100644
--- a/tests/gtest_preconditions.cpp
+++ b/tests/gtest_preconditions.cpp
@@ -508,3 +508,88 @@ TEST(Preconditions, SkippedSequence)
   status = tree.tickWhileRunning();
   ASSERT_EQ(status, BT::NodeStatus::SUCCESS);
 }
+
+// Test for Issue #917: _successIf and _failureIf not re-evaluated when node is RUNNING
+// in a ReactiveSequence
+TEST(Preconditions, Issue917_SuccessIfWhenRunning)
+{
+  // The issue is that _successIf is only checked when a node is IDLE or SKIPPED.
+  // In a ReactiveSequence, when a variable changes to make _successIf true,
+  // the running node should return SUCCESS (after being halted).
+
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<KeepRunning>("KeepRunning");
+
+  static constexpr auto xml_text = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Main">
+      <ReactiveSequence>
+        <Script code="loop := loop + 1; my_var := (loop >= 3) ? 42 : 0"/>
+        <KeepRunning _successIf="my_var != 0"/>
+      </ReactiveSequence>
+    </BehaviorTree>
+  </root>
+  )";
+
+  auto tree = factory.createTreeFromText(xml_text);
+  tree.rootBlackboard()->set("loop", 0);
+  tree.rootBlackboard()->set("my_var", 0);
+
+  // First tick: loop=1, my_var=0, KeepRunning returns RUNNING
+  auto status = tree.tickOnce();
+  ASSERT_EQ(status, NodeStatus::RUNNING);
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("loop"), 1);
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("my_var"), 0);
+
+  // Second tick: loop=2, my_var=0, KeepRunning still RUNNING
+  status = tree.tickOnce();
+  ASSERT_EQ(status, NodeStatus::RUNNING);
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("loop"), 2);
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("my_var"), 0);
+
+  // Third tick: loop=3, my_var=42, _successIf should trigger SUCCESS
+  status = tree.tickOnce();
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("loop"), 3);
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("my_var"), 42);
+  // This is the critical assertion - the node should return SUCCESS
+  // because _successIf="my_var != 0" is now true
+  ASSERT_EQ(status, NodeStatus::SUCCESS);
+}
+
+TEST(Preconditions, Issue917_FailureIfWhenRunning)
+{
+  // Similar test for _failureIf: verify it's also evaluated when RUNNING
+
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<KeepRunning>("KeepRunning");
+
+  static constexpr auto xml_text = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Main">
+      <ReactiveSequence>
+        <Script code="loop := loop + 1; my_var := (loop >= 3) ? 42 : 0"/>
+        <KeepRunning _failureIf="my_var != 0"/>
+      </ReactiveSequence>
+    </BehaviorTree>
+  </root>
+  )";
+
+  auto tree = factory.createTreeFromText(xml_text);
+  tree.rootBlackboard()->set("loop", 0);
+  tree.rootBlackboard()->set("my_var", 0);
+
+  // First tick: loop=1, my_var=0, KeepRunning returns RUNNING
+  auto status = tree.tickOnce();
+  ASSERT_EQ(status, NodeStatus::RUNNING);
+
+  // Second tick: loop=2, my_var=0, KeepRunning still RUNNING
+  status = tree.tickOnce();
+  ASSERT_EQ(status, NodeStatus::RUNNING);
+
+  // Third tick: loop=3, my_var=42, _failureIf should trigger FAILURE
+  status = tree.tickOnce();
+  ASSERT_EQ(tree.rootBlackboard()->get<int>("my_var"), 42);
+  // This is the critical assertion - the node should return FAILURE
+  // because _failureIf="my_var != 0" is now true
+  ASSERT_EQ(status, NodeStatus::FAILURE);
+}