A SubTree port declaration whose name cannot be bound loses its remapping silently

[dv-picknik]

Describe the bug

A SubTree model can declare a port that nothing is able to bind, and the failure is silent.

validatePortName (src/xml_parsing.cpp) rejects only a leading digit, so <input_port name="_myPort"/> is accepted as a declaration. IsAllowedPortName (src/basic_types.cpp) requires an alphabetic first character, and createNodeFromXML uses that to classify every instance attribute. So _myPort="{outer}" on the instance is diverted into other_attributes, the remapping is dropped, and the SubTree reads its declared default instead of the parent's value. No exception, no log.

The two rules disagree about the same name. InputPort("_myPort") already throws "The name of a port must not be name or ID and must start with an alphabetic character. Underscore is reserved.", so the C++ API refuses what the XML path accepts.

I hit this because a code generator wrote editor state into a <TreeNodesModel> port list. The declaration parsed, so nothing complained, and the port could never work.

How to Reproduce

Reproduced on master at ca4edbf. Two new tests in tests/gtest_subtree.cpp are in the pull request below; the first fails on master and passes with the fix, the other two pass either way and pin the cases that must keep working.

Standalone reproduction:

#include "behaviortree_cpp/bt_factory.h"
#include <iostream>

static void run(const std::string& port) {
  std::string xml =
    "<root BTCPP_format=\"4\" main_tree_to_execute=\"Main\">"
    "  <BehaviorTree ID=\"Main\">"
    "    <Sequence>"
    "      <Script code=\"outer:='FROM_PARENT'\"/>"
    "      <SubTree ID=\"Sub\" " + port + "=\"{outer}\"/>"
    "    </Sequence>"
    "  </BehaviorTree>"
    "  <BehaviorTree ID=\"Sub\"><AlwaysSuccess/></BehaviorTree>"
    "  <TreeNodesModel><SubTree ID=\"Sub\">"
    "    <input_port name=\"" + port + "\" default=\"DEFAULT_NOT_WIRED\"/>"
    "  </SubTree></TreeNodesModel>"
    "</root>";
  BT::BehaviorTreeFactory f;
  auto tree = f.createTreeFromText(xml);
  tree.tickWhileRunning();
  for (auto& sub : tree.subtrees) {
    if (sub->tree_ID != "Sub") continue;
    auto entry = sub->blackboard->getEntry(port);
    std::cout << "  " << port << " inside SubTree -> "
              << (entry ? BT::toStr(entry->value) : "<absent>") << "\n";
  }
}

int main() {
  std::cout << "Parent sets outer='FROM_PARENT'. Declared default is DEFAULT_NOT_WIRED.\n";
  run("myPort");
  run("_myPort");
}

Output on master:

Parent sets outer='FROM_PARENT'. Declared default is DEFAULT_NOT_WIRED.
  myPort inside SubTree -> json:"FROM_PARENT"
  _myPort inside SubTree -> json:"DEFAULT_NOT_WIRED"

The second line should not read the default. Either the declaration should be refused, or the lost remapping should be reported.

Possible fixes

1. Reject a leading underscore in validatePortName, matching IsAllowedPortName. One line, and it removes the inconsistency at its source. It also stops XML that loads today, since a declaration nobody remaps is inert and generated trees can carry one.
2. Report only where an instance attribute names a declared port that cannot be bound. Leaves inert declarations alone, but treats the symptom rather than the disagreement between the two rules.

The pull request does 2, because it breaks nothing that works today. The call is yours though, and I will switch it to 1 if you prefer that.