diff --git a/simgear/props/props.cxx b/simgear/props/props.cxx index 9b2a9f98..8dc3477e 100644 --- a/simgear/props/props.cxx +++ b/simgear/props/props.cxx @@ -2361,8 +2361,14 @@ SGPropertyNode::addChangeListener (SGPropertyChangeListener * listener, { if (_listeners == 0) _listeners = new vector; - _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::iterator it = - find(_properties.begin(), _properties.end(), node); - if (it != _properties.end()) { - _properties.erase(it); + auto it = std::lower_bound(_properties.begin(), _properties.end(), node); + if (it == _properties.end()) { + // not found, warn? + return; } + + _properties.erase(it); } #if !PROPS_STANDALONE diff --git a/simgear/props/props_test.cxx b/simgear/props/props_test.cxx index 7e3c8721..e0eba17a 100644 --- a/simgear/props/props_test.cxx +++ b/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();