Current design of BT::NodeConfig requires to keep the factory alive even after tree instantiation.

[robin-mueller]

I've been working with BehaviorTree.CPP myself a lot lately and I came across an issue revolving around the BT::NodeConfig data structure and how it is created and passed to individual behavior tree nodes.

To get you up to speed, look at the structure's member called manifest. It is supposed to enable the behavior tree node to query information about its ports at runtime. This data is collected by an instance of BT:BehaviorTreeFactory using one of the methods to register behavior tree nodes (like registerNodeType). When the user calls these methods, the data port types are collected using the static method providedPorts of a behavior tree node and the information is stored by the BT:BehaviorTreeFactory. You may have noticed, that manifest is a pointer (!) to this information, which means that for individual nodes to be able to query port information the memory pointed at MUST be valid throughout the execution of the tree. However, BT:BehaviorTreeFactory complies with the builder design pattern where it is counter intuitive to keep the builder (in this case the BT:BehaviorTreeFactory instance) alive after the creation of the product (here the BT::Tree instance).

Disregarding this requirement becomes fatal when the source code crosses for example this section inside TreeNode::getInputStamped or this section inside TreeNode::setOutput where the port manifest is to be parsed to validate the template variable. It turns out that exactly this issue bit my *** because I was only holding on to the BT:BehaviorTreeFactory until the tree was created.

My question now is, if it's really intentional to require the user to keep the factory alive as long as the tree executes. I would suggest to not have that requirement, since it is counter intuitive in my opinion. I see, that copying the port manifest for every node hits performance, but it's a safer solution.

I'd also be happy to hear from @facontidavide why manifest was declared a const BT::TreeNodeManifest* in the first place.

[facontidavide]

Interesting. I think you are right, this need to be fixed.