diff --git a/include/behaviortree_cpp/basic_types.h b/include/behaviortree_cpp/basic_types.h index b61d49c16..2731f1974 100644 --- a/include/behaviortree_cpp/basic_types.h +++ b/include/behaviortree_cpp/basic_types.h @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include #include @@ -348,6 +350,12 @@ struct Timestamp [[nodiscard]] bool IsReservedAttribute(StringView str); +/// Throws RuntimeError if the string contains any whitespace character. +/// Used by port-creation paths (CreatePort and TreeNodesModel XML parsing) +/// to reject port names like "my port" that would be ambiguous in blackboard +/// remappings. +void ThrowIfPortNameContainsWhitespace(StringView name); + class TypeInfo { public: @@ -447,6 +455,7 @@ template "and must start with an alphabetic character. " "Underscore is reserved."); } + ThrowIfPortNameContainsWhitespace(sname); std::pair out; diff --git a/src/basic_types.cpp b/src/basic_types.cpp index ab5e016bb..5a387ab44 100644 --- a/src/basic_types.cpp +++ b/src/basic_types.cpp @@ -2,6 +2,8 @@ #include "behaviortree_cpp/tree_node.h" #include "behaviortree_cpp/json_export.h" +#include +#include #include #include #include @@ -465,6 +467,17 @@ bool IsReservedAttribute(StringView str) return str == "name" || str == "ID" || str == "_autoremap"; } +void ThrowIfPortNameContainsWhitespace(StringView name) +{ + const auto has_whitespace = std::any_of( + name.begin(), name.end(), [](unsigned char c) { return std::isspace(c); }); + if(has_whitespace) + { + throw RuntimeError( + StrCat("The name of a port must not contain whitespace: '", name, "'")); + } +} + Any convertFromJSON(StringView json_text, std::type_index type) { nlohmann::json json = nlohmann::json::parse(json_text); diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 5e38910d9..46ca79749 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -10,14 +10,20 @@ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include +#include +#include #include #include #include #include +#include #include #include #include +#include #include + #include "behaviortree_cpp/basic_types.h" #include "behaviortree_cpp/utils/strcat.hpp" @@ -214,6 +220,8 @@ void BT::XMLParser::PImpl::loadSubtreeModel(const XMLElement* xml_root) { throw RuntimeError("Missing attribute [name] in port (SubTree model)"); } + std::string_view sname(name); + ThrowIfPortNameContainsWhitespace(sname); if(auto default_value = port_node->Attribute("default")) { port.setDefaultValue(default_value); @@ -1000,10 +1008,59 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree(const std::string& tree_ID, } else { - // constant string: just set that constant value into the BB + // constant value: set it into the BB with appropriate type // IMPORTANT: this must not be autoremapped!!! new_bb->enableAutoRemapping(false); - new_bb->set(port_name, static_cast(port_value)); + const std::string str_value(port_value); + + // Try to preserve numeric types so that Script expressions + // can perform arithmetic without type-mismatch errors. + // Use std::from_chars with strict full-string validation to avoid + // false positives on compound strings like "1;2;3" or "2.2;2.4". + bool stored = false; + if(!str_value.empty()) + { + const char* begin = str_value.data(); + const char* end = begin + str_value.size(); + // Try integer first (no decimal point, no exponent notation). + // Use int when the value fits, to match the most common port + // declarations. Fall back to int64_t for larger values. + if(str_value.find('.') == std::string::npos && + str_value.find('e') == std::string::npos && + str_value.find('E') == std::string::npos) + { + int64_t int_val = 0; + auto [ptr, ec] = std::from_chars(begin, end, int_val); + if(ec == std::errc() && ptr == end) + { + if(int_val >= std::numeric_limits::min() && + int_val <= std::numeric_limits::max()) + { + new_bb->set(port_name, static_cast(int_val)); + } + else + { + new_bb->set(port_name, int_val); + } + stored = true; + } + } + // Try double + if(!stored) + { + double dbl_val = 0; + auto [ptr, ec] = std::from_chars(begin, end, dbl_val); + if(ec == std::errc() && ptr == end) + { + new_bb->set(port_name, dbl_val); + stored = true; + } + } + } + if(!stored) + { + new_bb->set(port_name, str_value); + } new_bb->enableAutoRemapping(do_autoremap); } } diff --git a/tests/gtest_ports.cpp b/tests/gtest_ports.cpp index eaaf9733a..abb2809fc 100644 --- a/tests/gtest_ports.cpp +++ b/tests/gtest_ports.cpp @@ -861,3 +861,13 @@ TEST(PortTest, VectorAny) ASSERT_NO_THROW(status = tree.tickOnce()); ASSERT_EQ(status, NodeStatus::FAILURE); } + +TEST(PortTest, WhitespaceInPortName) +{ + ASSERT_ANY_THROW(BT::InputPort("port name")); + ASSERT_ANY_THROW(BT::InputPort("port\tname")); + ASSERT_ANY_THROW(BT::InputPort("port\nname")); + ASSERT_ANY_THROW(BT::InputPort(" leading")); + ASSERT_ANY_THROW(BT::InputPort("trailing ")); + ASSERT_NO_THROW(BT::InputPort("valid_port_name")); +} diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp index c90a2597f..8d15893ed 100644 --- a/tests/gtest_subtree.cpp +++ b/tests/gtest_subtree.cpp @@ -620,6 +620,48 @@ TEST(SubTree, SubtreeModels) tree.tickWhileRunning(); } +TEST(SubTree, WhitespaceInSubtreeModel) +{ + // clang-format off + + static const char* xml_text = R"( + + + + + + + + + + + + + + + + + + + + )"; + + // clang-format on + + BehaviorTreeFactory factory; + try + { + auto _ = factory.createTreeFromText(xml_text); + } + catch(RuntimeError e) + { + EXPECT_NE(std::string_view(e.what()).find("not contain whitespace"), + std::string_view::npos); + return; + } + FAIL() << "Exception was not thrown."; +} + class PrintToConsole : public BT::SyncActionNode { public: @@ -757,3 +799,41 @@ TEST(SubTree, SubtreeNameNotRegistered) ASSERT_ANY_THROW(auto tree = factory.createTreeFromText(xml_text)); ASSERT_ANY_THROW(factory.registerBehaviorTreeFromText(xml_text)); } + +// Regression test: literal numeric values passed to subtrees should preserve +// their numeric type so that Script expressions can do arithmetic. +TEST(SubTree, LiteralNumericPortsPreserveType) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + + auto tree = factory.createTreeFromText(xml_text); + + // Set the remapped parent value as an integer + tree.rootBlackboard()->set("from_parent", 100); + + const auto status = tree.tickWhileRunning(); + ASSERT_EQ(status, NodeStatus::SUCCESS); +}