From c1a7ce97fab28c6c881f5369c2d501dcbfa77ba2 Mon Sep 17 00:00:00 2001
From: Davide Faconti <davide.faconti@gmail.com>
Date: Sun, 25 Oct 2026 20:31:05 +0100
Subject: [PATCH] Sort ports in the TreeNodesModel XML (#851)

PortsList is an unordered_map, so writeTreeNodesModelXML() emitted the
ports in hash order, which differs across builds and shows up in Groot2.
Sort them by direction (input, output, inout), then by name, as was done
before d5ea17b.

Keeping the order of providedPorts() would need to change the PortsList
type (#870), which breaks API/ABI; this is the compatible subset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 src/xml_parsing.cpp     | 18 ++++++++++++++++-
 tests/gtest_factory.cpp | 43 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp
index b0f57d999..535af9ff9 100644
--- a/src/xml_parsing.cpp
+++ b/src/xml_parsing.cpp
@@ -1309,8 +1309,24 @@ void addNodeModelToXML(const TreeNodeManifest& model, XMLDocument& doc,
   XMLElement* element = doc.NewElement(toStr(model.type).c_str());
   element->SetAttribute("ID", model.registration_ID.c_str());
 
-  for(const auto& [port_name, port_info] : model.ports)
+  // PortsList is unordered: sort by direction, then by name, so that the
+  // output (and the order shown in Groot2) is deterministic.
+  std::vector<const PortsList::value_type*> sorted_ports;
+  sorted_ports.reserve(model.ports.size());
+  for(const auto& port : model.ports)
   {
+    sorted_ports.push_back(&port);
+  }
+  std::sort(sorted_ports.begin(), sorted_ports.end(),
+            [](const auto* lhs, const auto* rhs) {
+              const auto lhs_dir = lhs->second.direction();
+              const auto rhs_dir = rhs->second.direction();
+              return lhs_dir != rhs_dir ? lhs_dir < rhs_dir : lhs->first < rhs->first;
+            });
+
+  for(const auto* port : sorted_ports)
+  {
+    const auto& [port_name, port_info] = *port;
     XMLElement* port_element = nullptr;
     switch(port_info.direction())
     {
diff --git a/tests/gtest_factory.cpp b/tests/gtest_factory.cpp
index 2f1559681..96150d8a7 100644
--- a/tests/gtest_factory.cpp
+++ b/tests/gtest_factory.cpp
@@ -505,6 +505,49 @@ TEST(BehaviorTreeFactory, WrittenXMLHasLibraryVersion)
   EXPECT_EQ(reloaded.tickWhileRunning(), NodeStatus::SUCCESS);
 }
 
+class ActionWithManyPorts : public SyncActionNode
+{
+public:
+  ActionWithManyPorts(const std::string& name, const NodeConfig& config)
+    : SyncActionNode(name, config)
+  {}
+
+  BT::NodeStatus tick() override
+  {
+    return NodeStatus::SUCCESS;
+  }
+
+  static PortsList providedPorts()
+  {
+    return { OutputPort<int>("out_b"),       InputPort<int>("in_c"),
+             BidirectionalPort<int>("io_a"), InputPort<int>("in_a"),
+             OutputPort<int>("out_a"),       InputPort<int>("in_b") };
+  }
+};
+
+// Issue #851: PortsList is an unordered_map, so the model must sort the ports
+// itself, otherwise Groot2 shows them in a different order on every build.
+TEST(BehaviorTreeFactory, ModelPortsAreSorted)
+{
+  BehaviorTreeFactory factory;
+  factory.registerNodeType<ActionWithManyPorts>("ActionWithManyPorts");
+
+  const auto xml = writeTreeNodesModelXML(factory, false);
+  const std::vector<std::string> expected_order = {
+    R"(<input_port name="in_a")",   R"(<input_port name="in_b")",
+    R"(<input_port name="in_c")",   R"(<output_port name="out_a")",
+    R"(<output_port name="out_b")", R"(<inout_port name="io_a")"
+  };
+  size_t prev_pos = 0;
+  for(const auto& port : expected_order)
+  {
+    const auto pos = xml.find(port);
+    ASSERT_NE(pos, std::string::npos) << port;
+    EXPECT_GT(pos, prev_pos) << port;
+    prev_pos = pos;
+  }
+}
+
 TEST(BehaviorTreeFactory, ReturnByValue)
 {
   // Issue #937: BehaviorTreeFactory cannot be returned by value from