diff --git a/include/behaviortree_cpp/scripting/operators.hpp b/include/behaviortree_cpp/scripting/operators.hpp index 0f7d74c78..a1b2b64a7 100644 --- a/include/behaviortree_cpp/scripting/operators.hpp +++ b/include/behaviortree_cpp/scripting/operators.hpp @@ -15,6 +15,7 @@ #include "behaviortree_cpp/scripting/any_types.hpp" #include "behaviortree_cpp/scripting/script_parser.hpp" +#include #include #include #include @@ -50,6 +51,25 @@ inline double StringToDouble(const Any& value, const Environment& env) return value.cast(); } +// Replace a string holding a whole number (e.g. "42" or "1.5") with that number. +// Unlike StringToDouble, it rejects numeric prefixes, booleans and enums. +inline void NumericStringToNumber(Any& value) +{ + const auto str = value.cast(); + const char* end = str.data() + str.size(); + int64_t int_val = 0; + const auto [ptr, ec] = std::from_chars(str.data(), end, int_val); + double dbl_val = 0; + if(ec == std::errc() && ptr == end) + { + value = Any(int_val); + } + else if(parseDouble(str, dbl_val, /*require_full_consumption=*/true)) + { + value = Any(dbl_val); + } +} + struct ExprBase { using Ptr = std::shared_ptr; @@ -233,6 +253,20 @@ struct ExprBinaryArithmetic : ExprBase throw RuntimeError(ErrorNotInit("right", opStr())); } + // A numeric string mixed with a number is used as that number, e.g. the + // literal SubTree port int_val="42" in "int_val + 1" + if(op == plus || op == minus || op == times || op == div) + { + if(lhs_v.isString() && rhs_v.isNumber()) + { + NumericStringToNumber(lhs_v); + } + else if(lhs_v.isNumber() && rhs_v.isString()) + { + NumericStringToNumber(rhs_v); + } + } + if(rhs_v.isNumber() && lhs_v.isNumber()) { auto lv = lhs_v.cast(); diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index 2470ee602..6ab1b1d49 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -13,12 +13,10 @@ #include "behaviortree_cpp/basic_types.h" #include -#include #include #include #include #include -#include #include #include #include @@ -1215,58 +1213,12 @@ void BT::XMLParser::PImpl::recursivelyCreateSubtree( } else { - // constant value: set it into the BB with appropriate type + // constant string: just set that constant value into the BB. + // It must stay a string, so that each port converts it to its own type + // (#1246); Script arithmetic converts numeric strings when needed. // IMPORTANT: this must not be autoremapped!!! new_bb->enableAutoRemapping(false); - const std::string str_value(attr_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(attr_name, static_cast(int_val)); - } - else - { - new_bb->set(attr_name, int_val); - } - stored = true; - } - } - // Try double - if(!stored) - { - double dbl_val = 0; - if(parseDouble(str_value, dbl_val, /*require_full_consumption=*/true)) - { - new_bb->set(attr_name, dbl_val); - stored = true; - } - } - } - if(!stored) - { - new_bb->set(attr_name, str_value); - } + new_bb->set(attr_name, static_cast(attr_value)); new_bb->enableAutoRemapping(do_autoremap); } } diff --git a/tests/gtest_subtree.cpp b/tests/gtest_subtree.cpp index 0709a316f..1808d87e4 100644 --- a/tests/gtest_subtree.cpp +++ b/tests/gtest_subtree.cpp @@ -1017,8 +1017,8 @@ TEST(SubTree, NestedDuplicateNames_ShouldFail) ASSERT_THROW((void)factory.createTreeFromText(xml_text), RuntimeError); } -// Regression test: literal numeric values passed to subtrees should preserve -// their numeric type so that Script expressions can do arithmetic. +// Regression test (#1121): Script expressions can do arithmetic with literal +// numeric values passed to subtrees. TEST(SubTree, LiteralNumericPortsPreserveType) { // clang-format off @@ -1055,6 +1055,62 @@ TEST(SubTree, LiteralNumericPortsPreserveType) ASSERT_EQ(status, NodeStatus::SUCCESS); } +// Issue #1246: a literal passed to a SubTree stays a string, so ports of different +// types can read it, and its spelling is preserved. +TEST(SubTree, LiteralPortsAreReadByAnyPortType) +{ + // clang-format off + static const char* xml_text = R"( + + + + + + + + + + + + + + + + +)"; + // clang-format on + + BehaviorTreeFactory factory; + std::vector strings; + int int_val = 0; + double dbl_val = 0; + factory.registerSimpleAction("ReadString", + [&](TreeNode& node) { + strings.push_back( + node.getInput("value").value()); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + factory.registerSimpleAction("ReadInt", + [&](TreeNode& node) { + int_val = node.getInput("value").value(); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + factory.registerSimpleAction("ReadDouble", + [&](TreeNode& node) { + dbl_val = node.getInput("value").value(); + return NodeStatus::SUCCESS; + }, + { InputPort("value") }); + + auto tree = factory.createTreeFromText(xml_text); + ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS); + ASSERT_EQ(strings, (std::vector{ "1", "007" })); + ASSERT_EQ(int_val, 1); + ASSERT_EQ(dbl_val, 1.0); +} + // Regression test: logical NOT should accept literal boolean values passed to // subtrees without changing their string representation in the blackboard. TEST(SubTree, LiteralBooleanPortsSupportLogicalNot) diff --git a/tests/script_parser_test.cpp b/tests/script_parser_test.cpp index ac0822b9b..9a4360eff 100644 --- a/tests/script_parser_test.cpp +++ b/tests/script_parser_test.cpp @@ -270,6 +270,37 @@ TEST(ParserTest, Equations) EXPECT_EQ(GetResult(" par1:='3'; par2:=4; par1!=par2").cast(), 1); } +// Issue #1246: a numeric string mixed with a number is used as that number +TEST(ParserTest, NumericStringArithmetic) +{ + BT::Ast::Environment environment = { BT::Blackboard::create(), {} }; + + auto GetResult = [&environment](const char* text) -> BT::Any { + return GetScriptResult(environment, text); + }; + environment.vars->set("int_str", std::string("42")); + environment.vars->set("dbl_str", std::string("1.5")); + environment.vars->set("text", std::string("hello")); + + EXPECT_EQ(GetResult("int_str + 1").cast(), 43.0); + EXPECT_EQ(GetResult("1 + int_str").cast(), 43.0); + EXPECT_EQ(GetResult("int_str - 2").cast(), 40.0); + EXPECT_EQ(GetResult("int_str * 2").cast(), 84.0); + EXPECT_EQ(GetResult("3 / dbl_str").cast(), 2.0); + EXPECT_EQ(GetResult("'1e3' + 1").cast(), 1001.0); + + // the whole string must be a number + EXPECT_ANY_THROW(GetResult("text + 1")); + EXPECT_ANY_THROW(GetResult("'42abc' + 1")); + EXPECT_ANY_THROW(GetResult("'true' + 1")); + EXPECT_ANY_THROW(GetResult("' 42' + 1")); + + // unchanged: two strings concatenate, bitwise operators require numbers + EXPECT_EQ(GetResult("int_str + int_str").cast(), "4242"); + EXPECT_EQ(GetResult("int_str .. 1").cast(), "421"); + EXPECT_ANY_THROW(GetResult("int_str & 1")); +} + TEST(ParserTest, NotInitializedComparison) { BT::Ast::Environment environment = { BT::Blackboard::create(), {} };