Merge master into fix/930-mock-substitution-hang

Resolve conflict in tests/gtest_substitution.cpp: keep both the #934
SubTreeNodeSubstitution test (from master) and the #930 tests.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: facontidavide

include/behaviortree_cpp/bt_factory.h

@@ -224,8 +224,8 @@ class BehaviorTreeFactory
   BehaviorTreeFactory(const BehaviorTreeFactory& other) = delete;
   BehaviorTreeFactory& operator=(const BehaviorTreeFactory& other) = delete;
 
-  BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept = default;
-  BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept = default;
+  BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept;
+  BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept;
 
   /// Remove a registered ID.
   bool unregisterBuilder(const std::string& ID);

src/bt_factory.cpp

@@ -106,6 +106,10 @@ BehaviorTreeFactory::BehaviorTreeFactory() : _p(new PImpl)
 
 BehaviorTreeFactory::~BehaviorTreeFactory() = default;
 
+BehaviorTreeFactory::BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept = default;
+BehaviorTreeFactory&
+BehaviorTreeFactory::operator=(BehaviorTreeFactory&& other) noexcept = default;
+
 bool BehaviorTreeFactory::unregisterBuilder(const std::string& ID)
 {
   if(builtinNodes().count(ID) != 0)

src/xml_parsing.cpp

@@ -867,8 +867,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
     config.input_ports = port_remap;
     new_node =
         factory->instantiateTreeNode(instance_name, toStr(NodeType::SUBTREE), config);
+    // If a substitution rule replaced the SubTree with a different node
+    // (e.g. a TestNode), the dynamic_cast will return nullptr.
     auto subtree_node = dynamic_cast<SubTreeNode*>(new_node.get());
-    subtree_node->setSubtreeID(type_ID);
+    if(subtree_node != nullptr)
+    {
+      subtree_node->setSubtreeID(type_ID);
+    }
   }
   else
   {

tests/gtest_factory.cpp

@@ -450,6 +450,28 @@ TEST(BehaviorTreeFactory, ManifestMethod)
   EXPECT_NE(xml.find(expectedXML), std::string::npos);
 }
 
+TEST(BehaviorTreeFactory, ReturnByValue)
+{
+  // Issue #937: BehaviorTreeFactory cannot be returned by value from
+  // a function because the move constructor/assignment were defaulted
+  // in the header where PImpl is an incomplete type.
+  auto make_factory = []() -> BT::BehaviorTreeFactory {
+    BT::BehaviorTreeFactory factory;
+    factory.registerNodeType<DummyNodes::SaySomething>("SaySomething");
+    return factory;
+  };
+  auto factory = make_factory();
+
+  // Verify the moved-into factory is fully functional
+  const auto& manifests = factory.manifests();
+  ASSERT_TRUE(manifests.count("SaySomething"));
+
+  // Also test move assignment
+  BT::BehaviorTreeFactory factory2;
+  factory2 = std::move(factory);
+  ASSERT_TRUE(factory2.manifests().count("SaySomething"));
+}
+
 TEST(BehaviorTreeFactory, addMetadataToManifest)
 {
   BehaviorTreeFactory factory;

tests/gtest_substitution.cpp

@@ -53,6 +53,42 @@ TEST(Substitution, Parser)
   ASSERT_EQ(*std::get_if<std::string>(&rules.at("actionC")), "NotAConfig");
 }
 
+// Regression test for issue #934: segfault when substituting a SubTree node
+TEST(Substitution, SubTreeNodeSubstitution)
+{
+  static const char* parent_xml = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Parent">
+      <SubTree ID="Child" name="child" />
+    </BehaviorTree>
+  </root>
+  )";
+
+  static const char* child_xml = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Child">
+      <AlwaysSuccess />
+    </BehaviorTree>
+  </root>
+  )";
+
+  BehaviorTreeFactory factory;
+  factory.registerBehaviorTreeFromText(parent_xml);
+  factory.registerBehaviorTreeFromText(child_xml);
+
+  BT::TestNodeConfig config;
+  config.return_status = BT::NodeStatus::SUCCESS;
+  factory.addSubstitutionRule("child", config);
+
+  // This should not crash (was a segfault before the fix)
+  Tree tree;
+  ASSERT_NO_THROW(tree = factory.createTree("Parent"));
+
+  // The substituted tree should tick successfully
+  auto status = tree.tickWhileRunning();
+  ASSERT_EQ(status, BT::NodeStatus::SUCCESS);
+}
+
 // Test for issue #930: Mock substitution with registerSimpleAction
 // should not hang when using string-based substitution from JSON.
 TEST(Substitution, StringSubstitutionWithSimpleAction_Issue930)