diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp
index e37836d0f..fc99795a1 100644
--- a/src/xml_parsing.cpp
+++ b/src/xml_parsing.cpp
@@ -857,6 +857,29 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
}
else if(!IsReservedAttribute(port_name))
{
+ // The name may still match a declared port. validatePortName only rejects
+ // a leading digit, so a SubTree model can declare
+ // , a name IsAllowedPortName refuses and no
+ // C++ InputPort() can create. Falling through to other_attributes would
+ // drop the remapping without a word and leave the node on its declared
+ // default, so report it instead.
+ bool is_declared_port = manifest != nullptr && manifest->ports.count(port_name) > 0;
+ if(!is_declared_port && node_type == NodeType::SUBTREE)
+ {
+ auto model_it = subtree_models.find(type_ID);
+ is_declared_port = model_it != subtree_models.end() &&
+ model_it->second.ports.count(port_name) > 0;
+ }
+ if(is_declared_port)
+ {
+ const std::string reason = " and is declared, but the name cannot be used"
+ " as a port. A port name must begin with an"
+ " alphabetic character. Rename the port, for"
+ " example [_my_port] to [my_port].";
+ throw RuntimeError(StrCat("A port with name [", port_name,
+ "] is found in the XML (", type_ID, ", line ",
+ std::to_string(element->GetLineNum()), ")", reason));
+ }
other_attributes[port_name] = port_value;
}
}
diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp
index 82a0a23bb..0d4413305 100644
--- a/tests/gtest_subtree.cpp
+++ b/tests/gtest_subtree.cpp
@@ -1090,3 +1090,79 @@ TEST(SubTree, LiteralNonBooleanPortsRejectLogicalNot)
ASSERT_THROW((void)tree.tickWhileRunning(), RuntimeError);
ASSERT_EQ(tree.subtrees[1]->blackboard->get("value"), "1not_bool");
}
+
+// A SubTree model can declare a port whose name no node can bind.
+// validatePortName only rejects a leading digit, while IsAllowedPortName, which
+// classifies instance attributes and which InputPort()/OutputPort() enforce,
+// requires an alphabetic first character. Before the fix the remapping below was
+// diverted into other_attributes and the SubTree read its declared default, with
+// no exception and no log.
+TEST(SubTree, DeclaredPortNameThatCannotBeBound)
+{
+ static const char* xml_text = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+)";
+
+ BehaviorTreeFactory factory;
+ EXPECT_THROW(factory.createTreeFromText(xml_text), RuntimeError);
+}
+
+// The same underscore attribute on a SubTree that does not declare it is not a
+// port. It must keep landing in other_attributes and must not throw.
+TEST(SubTree, UndeclaredUnderscoreAttributeIsNotAPort)
+{
+ static const char* xml_text = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+)";
+
+ BehaviorTreeFactory factory;
+ EXPECT_NO_THROW(factory.createTreeFromText(xml_text));
+}
+
+// A declared port nobody remaps stays inert and keeps loading, so existing trees
+// carrying such a declaration are unaffected.
+TEST(SubTree, DeclaredUnbindablePortLoadsWhenNotRemapped)
+{
+ static const char* xml_text = R"(
+
+
+
+
+
+
+
+
+
+
+
+
+)";
+
+ BehaviorTreeFactory factory;
+ EXPECT_NO_THROW(factory.createTreeFromText(xml_text));
+}