Fix #937: enable returning BehaviorTreeFactory by value (#1082)

* Fix #937: allow BehaviorTreeFactory to be returned by value

Move constructor and move assignment were defaulted in the header where
PImpl is an incomplete type, preventing compilation when returning
BehaviorTreeFactory by value from inline functions. Move the definitions
to bt_factory.cpp where PImpl is complete, matching the existing pattern
used for the destructor.

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

* Fix clang-tidy readability-implicit-bool-conversion warning

Use explicit != nullptr comparison for pointer-to-bool conversion
flagged by clang-tidy.

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

---------

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/tree_node.cpp

@@ -481,7 +481,7 @@ AnyPtrLocked BT::TreeNode::getLockedPortContent(const std::string& key)
   {
     const auto bb_key = std::string(*remapped_key);
     auto result = _p->config.blackboard->getAnyLocked(bb_key);
-    if(!result && _p->config.manifest)
+    if(!result && _p->config.manifest != nullptr)
     {
       // Entry doesn't exist yet. Create it using the port's type info
       // from the manifest so that getLockedPortContent works even when

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;