throw instead of wrapping the 16-bit UID counter in Tree::getUID

src/bt_factory.cpp

@@ -20,6 +20,7 @@
 
 #include <filesystem>
 #include <functional>
+#include <limits>
 
 namespace BT
 {
@@ -755,6 +756,13 @@ void Tree::applyVisitor(const std::function<void(TreeNode*)>& visitor)
 
 uint16_t Tree::getUID()
 {
+  // UIDs are 16 bits wide (and serialized as such by Groot2 and the loggers):
+  // wrapping around would silently give two nodes the same UID.
+  if(uid_counter_ == std::numeric_limits<uint16_t>::max())
+  {
+    throw RuntimeError("Tree::getUID(): too many nodes, a tree can't have more than ",
+                       std::to_string(std::numeric_limits<uint16_t>::max()), " nodes");
+  }
   auto uid = ++uid_counter_;
   return uid;
 }

tests/gtest_factory.cpp

@@ -1,6 +1,7 @@
 #include "behaviortree_cpp/xml_parsing.h"
 
 #include <filesystem>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -777,3 +778,18 @@ TEST(BehaviorTreeFactory, MalformedXML_UnknownNodeType)
   BehaviorTreeFactory factory;
   EXPECT_THROW((void)factory.createTreeFromText(xml), RuntimeError);
 }
+
+TEST(BehaviorTreeFactory, TooManyNodesForUID)
+{
+  // UIDs are 16 bits wide. Past 65535 nodes the counter used to wrap around
+  // and the next nodes got the same UID (and path) as the first ones.
+  Tree tree;
+  std::set<uint16_t> uids;
+  for(int i = 0; i < 65535; i++)
+  {
+    uids.insert(tree.getUID());
+  }
+  EXPECT_EQ(uids.size(), 65535u);
+  EXPECT_EQ(uids.count(0), 0u);
+  EXPECT_THROW((void)tree.getUID(), RuntimeError);
+}