Re-evaluate SUCCESS_IF and FAILURE_IF during RUNNING state

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;
       }
     }
   }

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);
+}