Tweaks to recursive listener behaviour

simgear/props/props.cxx

@@ -2361,8 +2361,14 @@ SGPropertyNode::addChangeListener (SGPropertyChangeListener * listener,
 {
   if (_listeners == 0)
     _listeners = new vector<SGPropertyChangeListener*>;
-  _listeners->push_back(listener);
+
-  listener->register_property(this);
+    auto it = std::find(_listeners->begin(), _listeners->end(), listener);
+    if (it != _listeners->end()) {
+        return;
+    }
+
+    _listeners->push_back(listener);
+    listener->register_property(this);
 
 #ifdef ALTERNATE_RECURSIVE
     if (listener->isRecursive()) {
@@ -2570,21 +2576,27 @@ SGPropertyChangeListener::childRemoved (SGPropertyNode * parent,
 void
 SGPropertyChangeListener::register_property (SGPropertyNode * node)
 {
-    auto it = std::find(_properties.begin(), _properties.end(), node);
+    auto it = std::lower_bound(_properties.begin(), _properties.end(), node);
     if (it != _properties.end()) {
-        return; // duplicate listen
+        if (*it == node) {
+            // already exists, duplicate listent
+            return;
+        }
     }
-   _properties.push_back(node);
+
+    _properties.insert(it, node);
 }
 
 void
 SGPropertyChangeListener::unregister_property (SGPropertyNode * node)
 {
-  vector<SGPropertyNode *>::iterator it =
+    auto it = std::lower_bound(_properties.begin(), _properties.end(), node);
-    find(_properties.begin(), _properties.end(), node);
+    if (it == _properties.end()) {
-    if (it != _properties.end()) {
+        // not found, warn?
-        _properties.erase(it);
+        return;
     }
+
+    _properties.erase(it);
 }
 
 #if !PROPS_STANDALONE

simgear/props/props_test.cxx

@@ -390,6 +390,22 @@ void test_addChild()
   dump_node(&root);
 }
 
+
+bool ensureNListeners(SGPropertyNode* node, unsigned int n)
+{
+    if (node->nListeners() != n) {
+        return false;
+    }
+
+    for (int c=0; c < node->nChildren(); ++c) {
+        if (!ensureNListeners(node->getChild(c), n)) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void defineSamplePropertyTree(SGPropertyNode_ptr root)
 {
     root->setIntValue("position/body/a", 42);
@@ -668,6 +684,13 @@ void testListener()
         COMPARE(l.checkValueChangeCount("controls/engines[1]/fuel-cutoff"), 1);
         COMPARE(l.checkValueChangeCount("controls/engines[9]/fuel-cutoff"), 0);
 
+        // ensure additional calls to fireChildrenRecursive don't cause multiple adds
+
+        VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
+        VERIFY(ensureNListeners(tree->getNode("controls"), 0));
+
+        tree->getNode("position/body")->fireCreatedRecursive();
+        VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
     }
 
 }
@@ -840,6 +863,39 @@ void tiedPropertiesListeners()
 
 }
 
+
+void testDeleterListener()
+{
+    SGPropertyNode_ptr tree = new SGPropertyNode;
+    defineSamplePropertyTree(tree);
+
+
+    // recursive listen
+    {
+        TestListener* l = new TestListener(tree.get(), true /* recursive */);
+        tree->getNode("position/body")->addChangeListener(l);
+        tree->getNode("controls/")->addChangeListener(l);
+
+        COMPARE(l->checkValueChangeCount("position/body/a"), 0);
+
+        // create additional children
+        tree->setFloatValue("position/body/sub/theta", 0.1234);
+
+        VERIFY(l->checkAdded("position/body/sub", tree->getNode("position/body/sub/theta")));
+
+        COMPARE(l->checkValueChangeCount("position/body/sub/theta"), 1);
+
+        delete l;
+
+        tree->setFloatValue("position/body/sub/theta", 99.123);
+        tree->setIntValue("position/body/a", 33);
+
+        // verify no listeners at all
+        VERIFY(ensureNListeners(tree, 0));
+    }
+
+}
+
 int main (int ac, char ** av)
 {
   test_value();
@@ -862,6 +918,8 @@ int main (int ac, char ** av)
     testListener();
     tiedPropertiesTest();
     tiedPropertiesListeners();
+    testDeleterListener();
+    
     // disable test for the moment
    // testAliasedListeners();