Store SubTree literals as strings again; numeric strings work in script arithmetic (#1246)

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 <charconv>
 #include <cmath>
 #include <memory>
 #include <string>
@@ -50,6 +51,25 @@ inline double StringToDouble(const Any& value, const Environment& env)
   return value.cast<double>();
 }
 
+// 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<std::string>();
+  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<ExprBase>;
@@ -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<double>();

src/xml_parsing.cpp

@@ -13,12 +13,10 @@
 #include "behaviortree_cpp/basic_types.h"
 
 #include <algorithm>
-#include <charconv>
 #include <cstdio>
 #include <cstring>
 #include <functional>
 #include <iostream>
-#include <limits>
 #include <list>
 #include <sstream>
 #include <string>
@@ -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<int>::min() &&
-                   int_val <= std::numeric_limits<int>::max())
-                {
-                  new_bb->set(attr_name, static_cast<int>(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<std::string>(attr_value));
           new_bb->enableAutoRemapping(do_autoremap);
         }
       }

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"(
+<root BTCPP_format="4" main_tree_to_execute="MainTree">
+
+    <BehaviorTree ID="MainTree">
+        <SubTree ID="Sub" mode="1" code="007" />
+    </BehaviorTree>
+
+    <BehaviorTree ID="Sub">
+        <Sequence>
+            <ReadString value="{mode}" />
+            <ReadInt value="{mode}" />
+            <ReadDouble value="{mode}" />
+            <ReadString value="{code}" />
+        </Sequence>
+    </BehaviorTree>
+
+</root>
+)";
+  // clang-format on
+
+  BehaviorTreeFactory factory;
+  std::vector<std::string> strings;
+  int int_val = 0;
+  double dbl_val = 0;
+  factory.registerSimpleAction("ReadString",
+                               [&](TreeNode& node) {
+                                 strings.push_back(
+                                     node.getInput<std::string>("value").value());
+                                 return NodeStatus::SUCCESS;
+                               },
+                               { InputPort<std::string>("value") });
+  factory.registerSimpleAction("ReadInt",
+                               [&](TreeNode& node) {
+                                 int_val = node.getInput<int>("value").value();
+                                 return NodeStatus::SUCCESS;
+                               },
+                               { InputPort<int>("value") });
+  factory.registerSimpleAction("ReadDouble",
+                               [&](TreeNode& node) {
+                                 dbl_val = node.getInput<double>("value").value();
+                                 return NodeStatus::SUCCESS;
+                               },
+                               { InputPort<double>("value") });
+
+  auto tree = factory.createTreeFromText(xml_text);
+  ASSERT_EQ(tree.tickWhileRunning(), NodeStatus::SUCCESS);
+  ASSERT_EQ(strings, (std::vector<std::string>{ "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)

tests/script_parser_test.cpp

@@ -270,6 +270,37 @@ TEST(ParserTest, Equations)
   EXPECT_EQ(GetResult(" par1:='3'; par2:=4; par1!=par2").cast<int>(), 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<double>(), 43.0);
+  EXPECT_EQ(GetResult("1 + int_str").cast<double>(), 43.0);
+  EXPECT_EQ(GetResult("int_str - 2").cast<double>(), 40.0);
+  EXPECT_EQ(GetResult("int_str * 2").cast<double>(), 84.0);
+  EXPECT_EQ(GetResult("3 / dbl_str").cast<double>(), 2.0);
+  EXPECT_EQ(GetResult("'1e3' + 1").cast<double>(), 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<std::string>(), "4242");
+  EXPECT_EQ(GetResult("int_str .. 1").cast<std::string>(), "421");
+  EXPECT_ANY_THROW(GetResult("int_str & 1"));
+}
+
 TEST(ParserTest, NotInitializedComparison)
 {
   BT::Ast::Environment environment = { BT::Blackboard::create(), {} };