diff --git a/FIXME.md b/FIXME.md new file mode 100644 index 000000000..8c6a35279 --- /dev/null +++ b/FIXME.md @@ -0,0 +1,261 @@ +# FIXME - Bug Tracker + +This file tracks reported bugs from GitHub issues. For each bug, the workflow is: +1. Write a unit test that reproduces the issue +2. Evaluate whether the behavior diverges from intended library semantics +3. Fix the issue (if confirmed as a bug) + +**IMPORTANT:** NEVER modify existing tests that were not created in this branch. Only add new tests. + +--- + +## Scripting / Script Parser + +### [x] #1029 - Mathematical order of operations in script is wrong +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1029 +- **Summary:** `A - B + C` is evaluated as `A - (B + C)` instead of left-to-right. The expression parser applies incorrect associativity for addition and subtraction at the same precedence level. +- **Component:** `src/scripting/` +- **Test file:** `tests/gtest_scripting.cpp` (or new) + +### [x] #923 - Out-of-bounds read in script validation +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/923 +- **Summary:** `ValidateScript` can create a `std::string` from a non-NULL-terminated fixed-size stack buffer when parsing invalid scripts that generate large error messages, causing `strlen` to read out of bounds. +- **Component:** `src/scripting/` +- **Test file:** `tests/gtest_scripting.cpp` (or new) + +### [x] #832 - Script compare with negative number throws exception +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/832 +- **Summary:** Using a negative number in a script precondition (e.g., `_failureIf="A!=-1"`) throws a runtime exception. The scripting parser cannot handle negative number literals in comparisons. +- **Component:** `src/scripting/` +- **Test file:** `tests/gtest_scripting.cpp` (or new) + +--- + +## Ports / Type Conversion + +### [x] #1065 - Type conversion problem with subtree +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1065 +- **Summary:** Passing a `std::deque` through a SubTree port via a string literal (e.g., `"1;2;3"`) throws `std::runtime_error` about no safe conversion between `std::string` and `std::shared_ptr>`. String-to-deque conversion fails at subtree boundary. +- **Component:** `src/xml_parsing.cpp`, `include/behaviortree_cpp/basic_types.h` +- **Test file:** `tests/gtest_ports.cpp` (or new) + +### [x] #982 - Vectors initialized with `json:[]` string instead of empty +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/982 +- **Summary:** When a port of type `std::vector` has a default empty value `{}` and no input is specified in XML, the vector is initialized with the literal string `"json:[]"` instead of being an empty vector. +- **Component:** `include/behaviortree_cpp/basic_types.h`, `src/xml_parsing.cpp` +- **Test file:** `tests/gtest_ports.cpp` + +### [x] #969 - LoopNode: std::vector vs std::deque type mismatch +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/969 +- **Summary:** `LoopNode` uses `std::shared_ptr>` for its queue port, but upstream nodes often produce `std::vector`. This creates a port type conflict during tree creation. +- **Component:** `include/behaviortree_cpp/decorators/loop_node.h` +- **Test file:** `tests/gtest_decorator.cpp` (or new) + +### [x] #948 - ParseString errors with custom enum types +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/948 +- **Summary:** `parseString` in `tree_node.h` has an unsafe `find` on an unordered_map that may not be instantiated yet if `getInput` is called during construction. Also, custom enum types cannot be properly parsed via `convertFromString`. +- **Component:** `include/behaviortree_cpp/tree_node.h`, `include/behaviortree_cpp/basic_types.h` +- **Test file:** `tests/gtest_ports.cpp` + +### [x] #858 - getInput throws when default parameter is passed +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/858 +- **Summary:** Calling `getInput` with a default parameter value throws `std::out_of_range` (`_Map_base::at`). The internal map lookup fails when the port has a default value but the entry does not exist in the expected location. +- **Component:** `include/behaviortree_cpp/tree_node.h` +- **Test file:** `tests/gtest_ports.cpp` + +--- + +## Blackboard + +### [x] #974 - Blackboard set() stores values as string type +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/974 +- **Summary:** When using `blackboard->set(key, value.dump())` to set numeric values from JSON, the blackboard stores them as strings. Subsequent scripting operations with numeric operators fail because string types cannot be compared numerically. +- **Component:** `src/blackboard.cpp`, `include/behaviortree_cpp/blackboard.h` +- **Test file:** `tests/gtest_blackboard.cpp` + +### [x] #942 - Default blackboard entry not created for locked content +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/942 +- **Summary:** When using `getLockedPortContent` on a port with a default blackboard entry, no blackboard entry is actually created on the first call. The returned `AnyPtrLocked` is a default/empty value not placed into the blackboard. +- **Component:** `src/blackboard.cpp`, `include/behaviortree_cpp/blackboard.h` +- **Test file:** `tests/gtest_blackboard.cpp` + +### [x] #408 - debugMessage does not print parent values in SubTree +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/408 +- **Summary:** `config().blackboard->debugMessage()` inside a SubTree node's constructor does not show remapped entries from the parent blackboard. The function only checks `internal_to_external_` after finding an entry in `storage_`. +- **Component:** `src/blackboard.cpp` +- **Test file:** `tests/gtest_blackboard.cpp` + +--- + +## Control Nodes (Parallel) + +### [ ] #1045 - Test PauseWithRetry is flaky +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1045 +- **Summary:** The `Parallel.PauseWithRetry` test fails intermittently due to tight timing assertions. Measured time drifts beyond the allowed margin. +- **Component:** `tests/gtest_parallel.cpp` +- **Test file:** `tests/gtest_parallel.cpp` + +### [ ] #1041 - Performance degradation after prolonged operation in parallel nodes +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1041 +- **Summary:** After prolonged operation, when a parallel node's child detects an exception and exits, the interval before other nodes wait for execution becomes increasingly longer. Likely related to resource accumulation or thread management. +- **Component:** `src/controls/parallel_node.cpp` +- **Test file:** `tests/gtest_parallel.cpp` (or new) + +### [ ] #966 - ParallelAll max_failures off-by-one semantics +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/966 +- **Summary:** `ParallelAll` documents `max_failures` as "if the number of children returning FAILURE *exceeds* this value" but implements `>=` (greater-than-or-equal). Documentation and implementation disagree. +- **Component:** `src/controls/parallel_all_node.cpp` +- **Test file:** `tests/gtest_parallel.cpp` + +--- + +## Control Nodes (Reactive) + +### [ ] #1031 - Race condition in ReactiveSequence/ReactiveFallback with async children +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1031 +- **Summary:** With backchaining patterns, a previously `RUNNING` child is halted only *after* another child's `onStart` is executed. Two children are in `RUNNING` state simultaneously briefly, causing race conditions. +- **Component:** `src/controls/reactive_sequence.cpp`, `src/controls/reactive_fallback.cpp` +- **Test file:** `tests/gtest_reactive.cpp` (or new) + +### [ ] #917 - Precondition not re-checked in ReactiveSequence +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/917 +- **Summary:** When a node has a `_successIf` precondition inside a `ReactiveSequence`, the precondition is never re-evaluated after the node enters `RUNNING` state. The node continues returning `RUNNING` instead of `SUCCESS`. +- **Component:** `src/controls/reactive_sequence.cpp`, `src/tree_node.cpp` +- **Test file:** `tests/gtest_reactive.cpp` + +--- + +## XML Parsing + +### [ ] #979 - Recursive behavior trees cause stack overflow +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/979 +- **Summary:** A behavior tree can reference its own ID as a SubTree, causing infinite recursion and stack overflow during XML parsing. The parser does not detect or prevent cyclic subtree references. +- **Component:** `src/xml_parsing.cpp` +- **Test file:** `tests/gtest_factory.cpp` (or new) + +### [ ] #883 - JSON string in port value interpreted as blackboard reference +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/883 +- **Summary:** Setting a port value to a JSON string in XML (e.g., `settings='{{"a": 0.8}}'`) causes `{}` content to be interpreted as blackboard variable references instead of literal JSON. +- **Component:** `src/xml_parsing.cpp`, `src/tree_node.cpp` +- **Test file:** `tests/gtest_factory.cpp` (or new) + +### [ ] #880 - External subtree from file not found when loaded via inline XML +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/880 +- **Summary:** When a subtree is registered via `registerBehaviorTreeFromFile()` and then referenced from inline XML via `createTreeFromText()`, the factory throws `"Can't find a tree with name: MyTree"`. +- **Component:** `src/xml_parsing.cpp`, `src/bt_factory.cpp` +- **Test file:** `tests/gtest_factory.cpp` + +### [ ] #672 - Stack buffer overflow in xml_parsing.cpp +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/672 +- **Summary:** AddressSanitizer detects a stack-buffer-overflow in `xml_parsing.cpp` when parsing certain XML tree definitions. Memory safety issue in the XML parser. +- **Component:** `src/xml_parsing.cpp` +- **Test file:** `tests/gtest_factory.cpp` (or new) + +--- + +## BehaviorTreeFactory + +### [ ] #1046 - Heap use-after-free when factory is destroyed before ticking +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1046 +- **Summary:** If `BehaviorTreeFactory` is destroyed before its created tree is ticked, `NodeConfig` holds a dangling pointer to `TreeNodeManifest` owned by the factory. Accessing it causes heap-use-after-free. +- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/tree_node.h` +- **Test file:** `tests/gtest_factory.cpp` + +### [ ] #937 - BehaviorTreeFactory cannot be returned by value +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/937 +- **Summary:** Making `BehaviorTreeFactory` non-copyable also inadvertently prevents returning it by value from functions. Some compilers do not apply guaranteed copy elision in all contexts. +- **Component:** `include/behaviortree_cpp/bt_factory.h` +- **Test file:** `tests/gtest_factory.cpp` (or new) + +### [ ] #934 - Segfault when substituting subtree +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/934 +- **Summary:** `factory.loadSubstitutionRuleFromJSON()` followed by `factory.createTree()` causes a segmentation fault when the substitution target is a subtree node. +- **Component:** `src/bt_factory.cpp` +- **Test file:** `tests/gtest_factory.cpp` + +### [ ] #930 - Mock substitution with registerSimpleAction causes tree to hang +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/930 +- **Summary:** When using mock node substitution rules with `registerSimpleAction()`, the behavior tree gets stuck after an async delay finishes. The substituted node does not behave like the original. +- **Component:** `src/bt_factory.cpp` +- **Test file:** `tests/gtest_factory.cpp` (or new) + +--- + +## Loggers + +### [ ] #1057 - Groot2Publisher enters infinite loop on exception +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1057 +- **Summary:** `Groot2Publisher` can enter an infinite loop during destruction. When `active_server` is set to `false`, it can be reset to `true` depending on timing, causing `server_thread.join()` to hang indefinitely. +- **Component:** `src/loggers/groot2_publisher.cpp` +- **Test file:** New test needed + +--- + +## Tree Execution + +### [ ] #686 - haltTree doesn't effectively halt tickWhileRunning +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/686 +- **Summary:** `haltTree()` resets tree status to `IDLE`, which only breaks the inner loop of `tickRoot`. The outer loop then restarts execution because `IDLE` satisfies its condition, so the tree is never truly halted with `tickWhileRunning()`. +- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/bt_factory.h` +- **Test file:** `tests/gtest_tree.cpp` (or new) + +--- + +## TreeNode + +### [ ] #861 - Tick time monitoring can measure 0 due to instruction reordering +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/861 +- **Summary:** The `std::chrono` calls around `executeTick()` in `tree_node.cpp` can be reordered by the compiler, resulting in a measured tick duration of 0. +- **Component:** `src/tree_node.cpp` +- **Test file:** `tests/gtest_tree.cpp` (or new) + +--- + +## JSON Exporter + +### [ ] #989 - JsonExporter throws during vector conversion +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/989 +- **Summary:** A `bad_function_call` exception is thrown when converting `std::vector` types to JSON via `JsonExporter`. The conversion function is not properly registered for vector types. +- **Component:** `include/behaviortree_cpp/json_export.h` +- **Test file:** New test needed + +--- + +## Build / Dependencies + +### [ ] #959 - Version conflict with embedded nlohmann::json causes UB +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/959 +- **Summary:** The library embeds `nlohmann::json` v3.11.3 in its headers. If the user's codebase uses a different version, ODR violations occur depending on header include order, potentially causing undefined behavior. +- **Component:** `3rdparty/`, `CMakeLists.txt` +- **Test file:** Build system test / CMake configuration + +--- + +## Platform-Specific + +### [ ] #869 - Read access violation on Windows debug build +- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/869 +- **Summary:** Running on VS 2022 in Debug mode causes a read access violation in string hashing within the library DLL. Release mode works fine, suggesting a debug-mode-specific memory issue. +- **Component:** Core library (platform-specific) +- **Test file:** Requires Windows environment + +--- + +## Summary + +| Category | Count | Issue Numbers | +|----------|-------|---------------| +| Scripting / Script Parser | 3 | #1029, #923, #832 | +| Ports / Type Conversion | 5 | #1065, #982, #969, #948, #858 | +| Blackboard | 3 | #974, #942, #408 | +| Control Nodes (Parallel) | 3 | #1045, #1041, #966 | +| Control Nodes (Reactive) | 2 | #1031, #917 | +| XML Parsing | 4 | #979, #883, #880, #672 | +| BehaviorTreeFactory | 4 | #1046, #937, #934, #930 | +| Loggers | 1 | #1057 | +| Tree Execution | 1 | #686 | +| TreeNode | 1 | #861 | +| JSON Exporter | 1 | #989 | +| Build / Dependencies | 1 | #959 | +| Platform-Specific | 1 | #869 | +| **Total** | **30** | | diff --git a/include/behaviortree_cpp/decorators/loop_node.h b/include/behaviortree_cpp/decorators/loop_node.h index 71a032b10..de06d0165 100644 --- a/include/behaviortree_cpp/decorators/loop_node.h +++ b/include/behaviortree_cpp/decorators/loop_node.h @@ -15,6 +15,7 @@ #include "behaviortree_cpp/decorator_node.h" #include +#include namespace BT { @@ -75,7 +76,37 @@ class LoopNode : public DecoratorNode static_queue_ ? AnyPtrLocked() : getLockedPortContent("queue"); if(any_ref) { - current_queue_ = any_ref.get()->cast>(); + // Try SharedQueue first, then fall back to std::vector. + // This allows upstream nodes that output vector to be used + // directly with LoopNode without manual conversion. Issue #969. + auto queue_result = any_ref.get()->tryCast>(); + if(queue_result) + { + current_queue_ = queue_result.value(); + } + else if(!current_queue_) + { + // Only convert on first read; after that, use the + // already-converted current_queue_ (which gets popped). + auto vec_result = any_ref.get()->tryCast>(); + if(vec_result) + { + // Accept std::vector from upstream nodes. Issue #969. + const auto& vec = vec_result.value(); + current_queue_ = std::make_shared>(vec.begin(), vec.end()); + } + else if(any_ref.get()->isString()) + { + // Accept string values (e.g. from subtree remapping). Issue #1065. + auto str = any_ref.get()->cast(); + current_queue_ = convertFromString>(str); + } + else + { + throw RuntimeError("LoopNode: port 'queue' must contain either " + "SharedQueue, std::vector, or a string"); + } + } } if(current_queue_ && !current_queue_->empty()) @@ -114,8 +145,9 @@ class LoopNode : public DecoratorNode static PortsList providedPorts() { - // we mark "queue" as BidirectionalPort, because the original element is modified - return { BidirectionalPort>("queue"), + // Use an untyped BidirectionalPort to accept both SharedQueue + // and std::vector without triggering a port type mismatch. Issue #969. + return { BidirectionalPort("queue"), InputPort("if_empty", NodeStatus::SUCCESS, "Status to return if queue is empty: " "SUCCESS, FAILURE, SKIPPED"), diff --git a/include/behaviortree_cpp/scripting/operators.hpp b/include/behaviortree_cpp/scripting/operators.hpp index bd6bfc863..6aaa15b50 100644 --- a/include/behaviortree_cpp/scripting/operators.hpp +++ b/include/behaviortree_cpp/scripting/operators.hpp @@ -16,6 +16,7 @@ #include "behaviortree_cpp/scripting/script_parser.hpp" #include +#include #include #include #include @@ -407,11 +408,32 @@ struct ExprComparison : ExprBase } else if(lhs_v.isString() && rhs_v.isString()) { - auto lv = lhs_v.cast(); - auto rv = rhs_v.cast(); - if(!SwitchImpl(lv, rv, ops[i])) + // Try numeric comparison when both strings are parseable as numbers. + // This handles values set via blackboard->set(key, "42"). Issue #974. + auto ls = lhs_v.cast(); + auto rs = rhs_v.cast(); + char* lend = nullptr; + char* rend = nullptr; + double ld = std::strtod(ls.c_str(), &lend); + double rd = std::strtod(rs.c_str(), &rend); + bool l_numeric = (lend == ls.c_str() + ls.size()) && !ls.empty(); + bool r_numeric = (rend == rs.c_str() + rs.size()) && !rs.empty(); + + if(l_numeric && r_numeric) { - return False; + if(!SwitchImpl(ld, rd, ops[i])) + { + return False; + } + } + else + { + auto lv = lhs_v.cast(); + auto rv = rhs_v.cast(); + if(!SwitchImpl(lv, rv, ops[i])) + { + return False; + } } } else if(lhs_v.isString() && rhs_v.isNumber()) @@ -747,7 +769,10 @@ struct Expression : lexy::expression_production return dsl::op(LEXY_LIT("..")); }(); - using operand = math_sum; + // Use math_prefix (not math_sum) to avoid duplicating math_sum/math_product + // in the operation list, which would break left-associativity of +/-/*// + // due to conflicting binding power levels in lexy's _operation_list_of. + using operand = math_prefix; }; // ~x diff --git a/include/behaviortree_cpp/tree_node.h b/include/behaviortree_cpp/tree_node.h index d91e36e15..d95d4e017 100644 --- a/include/behaviortree_cpp/tree_node.h +++ b/include/behaviortree_cpp/tree_node.h @@ -20,6 +20,7 @@ #include "behaviortree_cpp/utils/strcat.hpp" #include "behaviortree_cpp/utils/wakeup_signal.hpp" +#include #include #include #include @@ -432,17 +433,25 @@ T TreeNode::parseString(const std::string& str) const { if constexpr(std::is_enum_v && !std::is_same_v) { - auto it = config().enums->find(str); - // conversion available - if(it != config().enums->end()) + // Check the ScriptingEnumsRegistry first, if available. + if(config().enums) { - return static_cast(it->second); + auto it = config().enums->find(str); + if(it != config().enums->end()) + { + return static_cast(it->second); + } } - else + // Try numeric conversion (e.g. "2" for an enum value). + int tmp = 0; + auto [ptr, ec] = std::from_chars(str.data(), str.data() + str.size(), tmp); + if(ec == std::errc() && ptr == str.data() + str.size()) { - // hopefully str contains a number that can be parsed. May throw - return static_cast(convertFromString(str)); + return static_cast(tmp); } + // Fall back to convertFromString, which uses a user-provided + // specialization if one exists. Issue #948. + return convertFromString(str); } return convertFromString(str); } diff --git a/src/basic_types.cpp b/src/basic_types.cpp index 284e2eb77..714247b5e 100644 --- a/src/basic_types.cpp +++ b/src/basic_types.cpp @@ -222,6 +222,11 @@ float convertFromString(StringView str) template <> std::vector convertFromString>(StringView str) { + if(StartWith(str, "json:")) + { + str.remove_prefix(5); + return nlohmann::json::parse(str).get>(); + } auto parts = splitString(str, ';'); std::vector output; output.reserve(parts.size()); @@ -235,6 +240,11 @@ std::vector convertFromString>(StringView str) template <> std::vector convertFromString>(StringView str) { + if(StartWith(str, "json:")) + { + str.remove_prefix(5); + return nlohmann::json::parse(str).get>(); + } auto parts = splitString(str, ';'); std::vector output; output.reserve(parts.size()); @@ -248,6 +258,11 @@ std::vector convertFromString>(StringView str) template <> std::vector convertFromString>(StringView str) { + if(StartWith(str, "json:")) + { + str.remove_prefix(5); + return nlohmann::json::parse(str).get>(); + } auto parts = splitString(str, ';'); std::vector output; output.reserve(parts.size()); @@ -261,6 +276,11 @@ std::vector convertFromString>(StringView str) template <> std::vector convertFromString>(StringView str) { + if(StartWith(str, "json:")) + { + str.remove_prefix(5); + return nlohmann::json::parse(str).get>(); + } auto parts = splitString(str, ';'); std::vector output; output.reserve(parts.size()); diff --git a/src/blackboard.cpp b/src/blackboard.cpp index 64163a7d6..ad9db06e6 100644 --- a/src/blackboard.cpp +++ b/src/blackboard.cpp @@ -116,8 +116,21 @@ void Blackboard::debugMessage() const for(const auto& [from, to] : internal_to_external_) { - std::cout << "[" << from << "] remapped to port of parent tree [" << to << "]" - << std::endl; + std::cout << "[" << from << "] remapped to port of parent tree [" << to << "]"; + // Show the type of the remapped entry from the parent. Issue #408. + if(auto parent = parent_bb_.lock()) + { + if(auto entry = parent->getEntry(to)) + { + auto port_type = entry->info.type(); + if(port_type == typeid(void)) + { + port_type = entry->value.type(); + } + std::cout << " (" << BT::demangle(port_type) << ")"; + } + } + std::cout << std::endl; } } diff --git a/src/tree_node.cpp b/src/tree_node.cpp index 52b2d087f..d464d6af9 100644 --- a/src/tree_node.cpp +++ b/src/tree_node.cpp @@ -479,7 +479,21 @@ AnyPtrLocked BT::TreeNode::getLockedPortContent(const std::string& key) { if(auto remapped_key = getRemappedKey(key, getRawPortValue(key))) { - return _p->config.blackboard->getAnyLocked(std::string(*remapped_key)); + const auto bb_key = std::string(*remapped_key); + auto result = _p->config.blackboard->getAnyLocked(bb_key); + if(!result && _p->config.manifest) + { + // Entry doesn't exist yet. Create it using the port's type info + // from the manifest so that getLockedPortContent works even when + // the port is not explicitly declared in XML. Issue #942. + auto port_it = _p->config.manifest->ports.find(key); + if(port_it != _p->config.manifest->ports.end()) + { + _p->config.blackboard->createEntry(bb_key, port_it->second); + result = _p->config.blackboard->getAnyLocked(bb_key); + } + } + return result; } return {}; } diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index a11383041..b257c6b4a 100644 --- a/src/xml_parsing.cpp +++ b/src/xml_parsing.cpp @@ -929,6 +929,31 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element, demangle(prev_info->type()), "] and, later type [", demangle(port_info.type()), "] was used somewhere else."); } + + // If the existing entry is not strongly typed (e.g. set as a plain + // string from subtree remapping) but the port IS strongly typed, + // upgrade the entry by converting the string value. Issue #1065. + if(!prev_info->isStronglyTyped() && port_info.isStronglyTyped()) + { + auto entry = blackboard->getEntry(port_key); + if(entry) + { + std::scoped_lock lock(entry->entry_mutex); + if(!entry->value.empty() && entry->value.isString()) + { + auto str_val = entry->value.tryCast(); + if(str_val) + { + auto typed_val = port_info.parseString(*str_val); + if(!typed_val.empty()) + { + entry->info = port_info; + entry->value = typed_val; + } + } + } + } + } } else { diff --git a/tests/gtest_blackboard.cpp b/tests/gtest_blackboard.cpp index 2c10e78d8..f53ed9be8 100644 --- a/tests/gtest_blackboard.cpp +++ b/tests/gtest_blackboard.cpp @@ -745,3 +745,123 @@ TEST(BlackboardTest, SetBlackboard_WithPortRemapping) // Tick till the end with no crashes ASSERT_NO_THROW(tree.tickWhileRunning();); } + +// Issue #942: getLockedPortContent should return a valid locked reference +// even when the port is not explicitly declared in XML, as long as there is +// a default blackboard remapping (e.g., "{=}"). +class ActionWithLockedPort : public SyncActionNode +{ +public: + ActionWithLockedPort(const std::string& name, const NodeConfig& config) + : SyncActionNode(name, config) + {} + + NodeStatus tick() override + { + auto any_locked = getLockedPortContent("value"); + if(!any_locked) + { + locked_valid = false; + return NodeStatus::FAILURE; + } + locked_valid = true; + // Assign a value through the locked reference + any_locked.assign(42); + return NodeStatus::SUCCESS; + } + + static PortsList providedPorts() + { + return { BidirectionalPort("value", "{=}", "A value with default BB remap") }; + } + + bool locked_valid = false; +}; + +TEST(BlackboardTest, GetLockedPortContentWithDefault_Issue942) +{ + // XML does NOT specify the "value" port — relies on default "{=}" + std::string xml_txt = R"( + + + + + )"; + + BehaviorTreeFactory factory; + factory.registerNodeType("ActionWithLockedPort"); + auto tree = factory.createTreeFromText(xml_txt); + auto status = tree.tickWhileRunning(); + + ASSERT_EQ(status, NodeStatus::SUCCESS); + + for(const auto& node : tree.subtrees.front()->nodes) + { + if(auto action = dynamic_cast(node.get())) + { + ASSERT_TRUE(action->locked_valid); + } + } + + // The value should be accessible from the blackboard + ASSERT_EQ(tree.rootBlackboard()->get("value"), 42); +} + +// Issue #974: blackboard->set(key, "42") stores a string. +// When two string-valued blackboard entries are compared in a script, +// numeric comparison should be used if both strings are parseable as numbers. +// "9" < "10" is false lexicographically but true numerically. +TEST(BlackboardTest, StringSetNumericScriptComparison_Issue974) +{ + auto bb = Blackboard::create(); + bb->set("a", std::string("9")); + bb->set("b", std::string("10")); + + BehaviorTreeFactory factory; + + std::string xml_txt = R"( + + + +