From 9f0f23a4bb0842687fcbfe094fc253a77dc1adc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduard=20Frigola=20Bague=CC=81?= Date: Wed, 2 Sep 2020 19:04:05 +0200 Subject: [PATCH 1/9] [ofParameter] Notify parents about parameter changing name. [ofParameterGroup] listen to nameChange event and update parametersIndex. --- libs/openFrameworks/types/ofParameter.cpp | 22 ++++++++++++++++++ libs/openFrameworks/types/ofParameter.h | 23 +++++++++++++++++++ .../openFrameworks/types/ofParameterGroup.cpp | 7 ++++++ 3 files changed, 52 insertions(+) diff --git a/libs/openFrameworks/types/ofParameter.cpp b/libs/openFrameworks/types/ofParameter.cpp index fd6b46efd35..c3d7534b5e7 100644 --- a/libs/openFrameworks/types/ofParameter.cpp +++ b/libs/openFrameworks/types/ofParameter.cpp @@ -80,7 +80,29 @@ ofParameter::ofParameter(const string& name) } void ofParameter::setName(const string & name){ + std::string oldName = getEscapedName(); obj->name = name; + + // Notify all parents, if there are any. + if(!obj->parents.empty()) + { + // Erase each invalid parent + obj->parents.erase(std::remove_if(obj->parents.begin(), + obj->parents.end(), + [this](const std::weak_ptr & p){ return p.expired(); }), + obj->parents.end()); + + // notify all leftover (valid) parents of this object's changed value. + // this can't happen in the same iterator as above, because a notified listener + // might perform similar cleanups that would corrupt our iterator + // (which appens for example if the listener calls getFirstParent on us) + for(auto & parent: obj->parents){ + auto p = parent.lock(); + if(p){ + p->notifyParameterNameChanged(oldName, getEscapedName()); + } + } + } } string ofParameter::getName() const{ diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 29102c65bc5..773c6f3e27c 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -265,6 +265,7 @@ class ofParameterGroup: public ofAbstractParameter { :serializable(true){} void notifyParameterChanged(ofAbstractParameter & param); + void notifyParameterNameChanged(const std::string oldName, const std::string newName); std::map parametersIndex; std::vector > parameters; @@ -818,7 +819,29 @@ inline ofParameter::operator const ParameterType & () const{ template void ofParameter::setName(const std::string & name){ + std::string oldName = getEscapedName(); obj->name = name; + + // Notify all parents, if there are any. + if(!obj->parents.empty()) + { + // Erase each invalid parent + obj->parents.erase(std::remove_if(obj->parents.begin(), + obj->parents.end(), + [this](const std::weak_ptr & p){ return p.expired(); }), + obj->parents.end()); + + // notify all leftover (valid) parents of this object's changed value. + // this can't happen in the same iterator as above, because a notified listener + // might perform similar cleanups that would corrupt our iterator + // (which appens for example if the listener calls getFirstParent on us) + for(auto & parent: obj->parents){ + auto p = parent.lock(); + if(p){ + p->notifyParameterNameChanged(oldName, getEscapedName()); + } + } + } } template diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index 3df027bbad2..22e735b0f5d 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -432,6 +432,13 @@ void ofParameterGroup::Value::notifyParameterChanged(ofAbstractParameter & param }),parents.end()); } +void ofParameterGroup::Value::notifyParameterNameChanged(const std::string oldName, const std::string newName){ + if(oldName != newName){ + parametersIndex[newName] = parametersIndex[oldName]; + parametersIndex.erase(oldName); + } +} + const ofParameterGroup ofParameterGroup::getFirstParent() const{ auto first = std::find_if(obj->parents.begin(),obj->parents.end(),[](const weak_ptr & p){return p.lock()!=nullptr;}); if(first!=obj->parents.end()){ From 5613bece2f918ce957e490588090908b7e7ebd41 Mon Sep 17 00:00:00 2001 From: Roy Macdonald Date: Fri, 4 Sep 2020 17:23:56 -0400 Subject: [PATCH 2/9] ofParameter name change improvements --- libs/openFrameworks/types/ofParameter.cpp | 62 +++++++------- libs/openFrameworks/types/ofParameter.h | 84 ++++++++++++------- .../openFrameworks/types/ofParameterGroup.cpp | 33 ++++++++ 3 files changed, 123 insertions(+), 56 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.cpp b/libs/openFrameworks/types/ofParameter.cpp index c3d7534b5e7..070cc0a9d91 100644 --- a/libs/openFrameworks/types/ofParameter.cpp +++ b/libs/openFrameworks/types/ofParameter.cpp @@ -83,26 +83,30 @@ void ofParameter::setName(const string & name){ std::string oldName = getEscapedName(); obj->name = name; - // Notify all parents, if there are any. - if(!obj->parents.empty()) - { - // Erase each invalid parent - obj->parents.erase(std::remove_if(obj->parents.begin(), - obj->parents.end(), - [this](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); - - // notify all leftover (valid) parents of this object's changed value. - // this can't happen in the same iterator as above, because a notified listener - // might perform similar cleanups that would corrupt our iterator - // (which appens for example if the listener calls getFirstParent on us) - for(auto & parent: obj->parents){ - auto p = parent.lock(); - if(p){ - p->notifyParameterNameChanged(oldName, getEscapedName()); - } - } - } + ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); + + +// // Notify all parents, if there are any. +// if(!obj->parents.empty()) +// { +// // Erase each invalid parent +// ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); +//// obj->parents.erase(std::remove_if(obj->parents.begin(), +//// obj->parents.end(), +//// [this](const std::weak_ptr & p){ return p.expired(); }), +//// obj->parents.end()); +// +// // notify all leftover (valid) parents of this object's changed value. +// // this can't happen in the same iterator as above, because a notified listener +// // might perform similar cleanups that would corrupt our iterator +// // (which appens for example if the listener calls getFirstParent on us) +// for(auto & parent: obj->parents){ +// auto p = parent.lock(); +// if(p){ +// p->notifyParameterNameChanged(oldName, getEscapedName()); +// } +// } +// } } string ofParameter::getName() const{ @@ -131,11 +135,12 @@ void ofParameter::trigger(){ // Notify all parents, if there are any. if(!obj->parents.empty()) { + ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); // Erase each invalid parent - obj->parents.erase(std::remove_if(obj->parents.begin(), - obj->parents.end(), - [this](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); +// obj->parents.erase(std::remove_if(obj->parents.begin(), +// obj->parents.end(), +// [this](const std::weak_ptr & p){ return p.expired(); }), +// obj->parents.end()); // notify all leftover (valid) parents of this object's changed value. // this can't happen in the same iterator as above, because a notified listener @@ -156,10 +161,11 @@ void ofParameter::trigger(const void * sender){ if(!obj->parents.empty()) { // Erase each invalid parent - obj->parents.erase(std::remove_if(obj->parents.begin(), - obj->parents.end(), - [this](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); +// obj->parents.erase(std::remove_if(obj->parents.begin(), +// obj->parents.end(), +// [this](const std::weak_ptr & p){ return p.expired(); }), +// obj->parents.end()); + ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); // notify all leftover (valid) parents of this object's changed value. // this can't happen in the same iterator as above, because a notified listener diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 773c6f3e27c..7e5ac85a8c5 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -67,11 +67,16 @@ class ofAbstractParameter{ virtual bool isReferenceTo(const ofAbstractParameter& other) const; + ofEvent nameChangedEvent; + protected: virtual const ofParameterGroup getFirstParent() const = 0; virtual void setSerializable(bool serializable)=0; virtual std::string escape(const std::string& str) const; virtual const void* getInternalObject() const = 0; + + + }; @@ -255,10 +260,10 @@ class ofParameterGroup: public ofAbstractParameter { std::vector >::const_reverse_iterator rbegin() const; std::vector >::const_reverse_iterator rend() const; -protected: +//protected: const void* getInternalObject() const; -private: +//private: class Value{ public: Value() @@ -278,6 +283,31 @@ class ofParameterGroup: public ofAbstractParameter { ofParameterGroup(std::shared_ptr obj) :obj(obj){} + + static void checkAndRemoveExpiredParents(std::vector> & parents); +// { +// parents.erase(std::remove_if(parents.begin(), +// parents.end(), +// [](const std::weak_ptr & p){ return p.expired(); }), +// parents.end()); +// } + + + static void changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName); +// { +// +// if(oldName.empty()) return; +// +// checkAndRemoveExpiredParents(parents); +// +// for(auto & parent: parents){ +// auto p = parent.lock(); +// if(p){ +// p->notifyParameterNameChanged(oldName, newName); +// } +// } +// } + template friend class ofParameter; @@ -581,9 +611,11 @@ class ofParameter: public ofAbstractParameter{ void setParent(ofParameterGroup & _parent); const ofParameterGroup getFirstParent() const{ - obj->parents.erase(std::remove_if(obj->parents.begin(),obj->parents.end(), - [](std::weak_ptr p){return p.lock()==nullptr;}), - obj->parents.end()); +// obj->parents.erase(std::remove_if(obj->parents.begin(),obj->parents.end(), +// [](std::weak_ptr p){return p.lock()==nullptr;}), +// obj->parents.end()); +// + ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); if(!obj->parents.empty()){ return obj->parents.front().lock(); }else{ @@ -746,10 +778,11 @@ inline void ofParameter::eventsSetValue(const ParameterType & v){ if(!obj->parents.empty()) { // Erase each invalid parent - obj->parents.erase(std::remove_if(obj->parents.begin(), - obj->parents.end(), - [this](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); + ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); +// obj->parents.erase(std::remove_if(obj->parents.begin(), +// obj->parents.end(), +// [this](const std::weak_ptr & p){ return p.expired(); }), +// obj->parents.end()); // notify all leftover (valid) parents of this object's changed value. // this can't happen in the same iterator as above, because a notified listener @@ -822,26 +855,21 @@ void ofParameter::setName(const std::string & name){ std::string oldName = getEscapedName(); obj->name = name; + ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); + // Notify all parents, if there are any. - if(!obj->parents.empty()) - { - // Erase each invalid parent - obj->parents.erase(std::remove_if(obj->parents.begin(), - obj->parents.end(), - [this](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); - - // notify all leftover (valid) parents of this object's changed value. - // this can't happen in the same iterator as above, because a notified listener - // might perform similar cleanups that would corrupt our iterator - // (which appens for example if the listener calls getFirstParent on us) - for(auto & parent: obj->parents){ - auto p = parent.lock(); - if(p){ - p->notifyParameterNameChanged(oldName, getEscapedName()); - } - } - } +// if(!obj->parents.empty() && !oldName.empty()) +// { +// +// ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); +// +// for(auto & parent: obj->parents){ +// auto p = parent.lock(); +// if(p){ +// p->notifyParameterNameChanged(oldName, getEscapedName()); +// } +// } +// } } template diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index 22e735b0f5d..45a9f5f7e8a 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -344,7 +344,10 @@ string ofParameterGroup::getName() const{ } void ofParameterGroup::setName(const string & name){ + std::string oldName = getEscapedName(); obj->name = name; + + changeChildName(this, obj->parents, oldName, getEscapedName()); } string ofParameterGroup::getEscapedName() const{ @@ -529,3 +532,33 @@ vector >::const_reverse_iterator ofParameterGrou } +void ofParameterGroup::checkAndRemoveExpiredParents(std::vector> & parents) +{ + parents.erase(std::remove_if(parents.begin(), + parents.end(), + [](const std::weak_ptr & p){ return p.expired(); }), + parents.end()); +} + + +void ofParameterGroup::changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName) +{ + if(!child) return; + // Currently it is working with empty names, so I think it is a good idea to allow these +// if(oldName.empty()) return; + + // name has not change, no need to notify anything + if(oldName == newName) return; + + checkAndRemoveExpiredParents(parents); + + for(auto & parent: parents){ + auto p = parent.lock(); + if(p){ + p->notifyParameterNameChanged(oldName, newName); + } + } + ofNotifyEvent(child->nameChangedEvent, newName, child); + + +} From 2bc6183da737cb8a830700953e847e9fa88c1bfa Mon Sep 17 00:00:00 2001 From: Roy Macdonald Date: Fri, 4 Sep 2020 17:51:08 -0400 Subject: [PATCH 3/9] ofParameter: fixed changes made for debuging --- libs/openFrameworks/types/ofParameter.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 7e5ac85a8c5..292d21ae299 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -260,10 +260,10 @@ class ofParameterGroup: public ofAbstractParameter { std::vector >::const_reverse_iterator rbegin() const; std::vector >::const_reverse_iterator rend() const; -//protected: +protected: const void* getInternalObject() const; -//private: +private: class Value{ public: Value() From 91602539b86c510df16fcf818157dfa1c3cd8608 Mon Sep 17 00:00:00 2001 From: Roy Macdonald Date: Sat, 5 Sep 2020 23:35:26 -0400 Subject: [PATCH 4/9] ofParameter: nameChangedEvent is now accessed through virtual function. ofParameterGroup: event for child name change. Cleanup --- libs/openFrameworks/types/ofParameter.cpp | 38 ++-------- libs/openFrameworks/types/ofParameter.h | 75 ++++++++----------- .../openFrameworks/types/ofParameterGroup.cpp | 37 +++++++-- 3 files changed, 69 insertions(+), 81 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.cpp b/libs/openFrameworks/types/ofParameter.cpp index 070cc0a9d91..99198bda553 100644 --- a/libs/openFrameworks/types/ofParameter.cpp +++ b/libs/openFrameworks/types/ofParameter.cpp @@ -85,28 +85,12 @@ void ofParameter::setName(const string & name){ ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); - -// // Notify all parents, if there are any. -// if(!obj->parents.empty()) -// { -// // Erase each invalid parent -// ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); -//// obj->parents.erase(std::remove_if(obj->parents.begin(), -//// obj->parents.end(), -//// [this](const std::weak_ptr & p){ return p.expired(); }), -//// obj->parents.end()); -// -// // notify all leftover (valid) parents of this object's changed value. -// // this can't happen in the same iterator as above, because a notified listener -// // might perform similar cleanups that would corrupt our iterator -// // (which appens for example if the listener calls getFirstParent on us) -// for(auto & parent: obj->parents){ -// auto p = parent.lock(); -// if(p){ -// p->notifyParameterNameChanged(oldName, getEscapedName()); -// } -// } -// } +} + + +ofEvent& ofParameter::nameChangedEvent() +{ + return obj->nameChangedEvent; } string ofParameter::getName() const{ @@ -135,12 +119,8 @@ void ofParameter::trigger(){ // Notify all parents, if there are any. if(!obj->parents.empty()) { + // Erase each invalid parent ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); - // Erase each invalid parent -// obj->parents.erase(std::remove_if(obj->parents.begin(), -// obj->parents.end(), -// [this](const std::weak_ptr & p){ return p.expired(); }), -// obj->parents.end()); // notify all leftover (valid) parents of this object's changed value. // this can't happen in the same iterator as above, because a notified listener @@ -161,10 +141,6 @@ void ofParameter::trigger(const void * sender){ if(!obj->parents.empty()) { // Erase each invalid parent -// obj->parents.erase(std::remove_if(obj->parents.begin(), -// obj->parents.end(), -// [this](const std::weak_ptr & p){ return p.expired(); }), -// obj->parents.end()); ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); // notify all leftover (valid) parents of this object's changed value. diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 292d21ae299..46e8ef8bad2 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -67,7 +67,7 @@ class ofAbstractParameter{ virtual bool isReferenceTo(const ofAbstractParameter& other) const; - ofEvent nameChangedEvent; + virtual ofEvent& nameChangedEvent() = 0; protected: virtual const ofParameterGroup getFirstParent() const = 0; @@ -250,7 +250,10 @@ class ofParameterGroup: public ofAbstractParameter { operator bool() const; ofEvent & parameterChangedE(); - + virtual ofEvent& nameChangedEvent(); + ofEvent& childNameChangedEvent(); + + std::vector >::iterator begin(); std::vector >::iterator end(); std::vector >::const_iterator begin() const; @@ -260,24 +263,27 @@ class ofParameterGroup: public ofAbstractParameter { std::vector >::const_reverse_iterator rbegin() const; std::vector >::const_reverse_iterator rend() const; -protected: +//protected: const void* getInternalObject() const; -private: +//private: class Value{ public: Value() :serializable(true){} void notifyParameterChanged(ofAbstractParameter & param); - void notifyParameterNameChanged(const std::string oldName, const std::string newName); - + void updateParameterName(const std::string oldName, const std::string newName); + void notifyParameterNameChanged(ofAbstractParameter & param); + std::map parametersIndex; std::vector > parameters; std::string name; bool serializable; std::vector> parents; ofEvent parameterChangedE; + ofEvent nameChangedEvent; + ofEvent childNameChangedEvent; }; std::shared_ptr obj; ofParameterGroup(std::shared_ptr obj) @@ -285,28 +291,8 @@ class ofParameterGroup: public ofAbstractParameter { static void checkAndRemoveExpiredParents(std::vector> & parents); -// { -// parents.erase(std::remove_if(parents.begin(), -// parents.end(), -// [](const std::weak_ptr & p){ return p.expired(); }), -// parents.end()); -// } - - + static void changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName); -// { -// -// if(oldName.empty()) return; -// -// checkAndRemoveExpiredParents(parents); -// -// for(auto & parent: parents){ -// auto p = parent.lock(); -// if(p){ -// p->notifyParameterNameChanged(oldName, newName); -// } -// } -// } template friend class ofParameter; @@ -623,6 +609,8 @@ class ofParameter: public ofAbstractParameter{ } } + virtual ofEvent& nameChangedEvent(); + size_t getNumListeners() const; const void* getInternalObject() const; @@ -664,6 +652,7 @@ class ofParameter: public ofAbstractParameter{ ParameterType value; ParameterType min, max; ofEvent changedE; + ofEvent nameChangedEvent; bool bInNotify; bool serializable; std::vector> parents; @@ -779,10 +768,6 @@ inline void ofParameter::eventsSetValue(const ParameterType & v){ { // Erase each invalid parent ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); -// obj->parents.erase(std::remove_if(obj->parents.begin(), -// obj->parents.end(), -// [this](const std::weak_ptr & p){ return p.expired(); }), -// obj->parents.end()); // notify all leftover (valid) parents of this object's changed value. // this can't happen in the same iterator as above, because a notified listener @@ -857,19 +842,11 @@ void ofParameter::setName(const std::string & name){ ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); - // Notify all parents, if there are any. -// if(!obj->parents.empty() && !oldName.empty()) -// { -// -// ofParameterGroup::checkAndRemoveExpiredParents(obj->parents); -// -// for(auto & parent: obj->parents){ -// auto p = parent.lock(); -// if(p){ -// p->notifyParameterNameChanged(oldName, getEscapedName()); -// } -// } -// } +} +template +ofEvent& ofParameter::nameChangedEvent() +{ + return obj->nameChangedEvent; } template @@ -1099,6 +1076,9 @@ class ofParameter: public ofAbstractParameter{ const void* getInternalObject() const{ return obj.get(); } + + virtual ofEvent& nameChangedEvent(); + protected: private: @@ -1113,6 +1093,7 @@ class ofParameter: public ofAbstractParameter{ std::string name; ofEvent changedE; + ofEvent nameChangedEvent; bool serializable; std::vector> parents; }; @@ -1169,6 +1150,7 @@ class ofReadOnlyParameter: public ofAbstractParameter{ std::string valueType() const; protected: + virtual ofEvent& nameChangedEvent(); void setName(const std::string & name); void enableEvents(); void disableEvents(); @@ -1335,6 +1317,11 @@ inline void ofReadOnlyParameter::setName(const std::string parameter.setName(name); } +template +inline ofEvent& ofReadOnlyParameter::nameChangedEvent(){ + return parameter.nameChangedEvent(); +} + template inline void ofReadOnlyParameter::enableEvents(){ parameter.enableEvents(); diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index 45a9f5f7e8a..bb51b325174 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -435,7 +435,17 @@ void ofParameterGroup::Value::notifyParameterChanged(ofAbstractParameter & param }),parents.end()); } -void ofParameterGroup::Value::notifyParameterNameChanged(const std::string oldName, const std::string newName){ +void ofParameterGroup::Value::notifyParameterNameChanged(ofAbstractParameter & param) +{ + ofNotifyEvent(childNameChangedEvent,param); + parents.erase(std::remove_if(parents.begin(),parents.end(),[¶m](const weak_ptr & p){ + auto parent = p.lock(); + if(parent) parent->notifyParameterNameChanged(param); + return !parent; + }),parents.end()); +} + +void ofParameterGroup::Value::updateParameterName(const std::string oldName, const std::string newName){ if(oldName != newName){ parametersIndex[newName] = parametersIndex[oldName]; parametersIndex.erase(oldName); @@ -544,8 +554,6 @@ void ofParameterGroup::checkAndRemoveExpiredParents(std::vector> & parents, const std::string& oldName, std::string newName) { if(!child) return; - // Currently it is working with empty names, so I think it is a good idea to allow these -// if(oldName.empty()) return; // name has not change, no need to notify anything if(oldName == newName) return; @@ -555,10 +563,27 @@ void ofParameterGroup::changeChildName(ofAbstractParameter* child, std::vectornotifyParameterNameChanged(oldName, newName); + p->updateParameterName(oldName, newName); } } - ofNotifyEvent(child->nameChangedEvent, newName, child); - + + ofNotifyEvent(child->nameChangedEvent(), newName, child); + + // we notify to the parents after updating the name in order to avoid any possible data race + for(auto & parent: parents){ + auto p = parent.lock(); + if(p){ + p->notifyParameterNameChanged(*child); + } + } +} + +ofEvent& ofParameterGroup::nameChangedEvent() +{ + return obj->nameChangedEvent; +} +ofEvent& ofParameterGroup::childNameChangedEvent() +{ + return obj->childNameChangedEvent; } From 314c5fbd2251b42d2f96545dc0c1425a9b9acf0a Mon Sep 17 00:00:00 2001 From: Roy Macdonald Date: Sat, 5 Sep 2020 23:37:28 -0400 Subject: [PATCH 5/9] ofxGui:: implemented listeners for parameter name change so now the gui updates accordingly. --- addons/ofxGui/src/ofxBaseGui.cpp | 12 ++++++++++++ addons/ofxGui/src/ofxBaseGui.h | 7 +++++++ addons/ofxGui/src/ofxButton.cpp | 4 ++-- addons/ofxGui/src/ofxColorPicker.cpp | 2 ++ addons/ofxGui/src/ofxGuiGroup.cpp | 1 + addons/ofxGui/src/ofxInputField.cpp | 1 + addons/ofxGui/src/ofxSlider.cpp | 2 +- addons/ofxGui/src/ofxSliderGroup.cpp | 5 +++-- addons/ofxGui/src/ofxToggle.cpp | 2 +- 9 files changed, 30 insertions(+), 6 deletions(-) diff --git a/addons/ofxGui/src/ofxBaseGui.cpp b/addons/ofxGui/src/ofxBaseGui.cpp index 1d6cb525464..851f25a4f34 100644 --- a/addons/ofxGui/src/ofxBaseGui.cpp +++ b/addons/ofxGui/src/ofxBaseGui.cpp @@ -483,3 +483,15 @@ void ofxBaseGui::disableHiDpi(){ bool ofxBaseGui::isHiDpiEnabled(){ return hiDpiScale == 2; } + +void ofxBaseGui::nameChanged( std::string& ) +{ + auto& p = getParameter(); + + setNeedsRedraw(); + +} +void ofxBaseGui::setNameListener() +{ + nameChangeListener = getParameter().nameChangedEvent().newListener(this, &ofxBaseGui::nameChanged); +} diff --git a/addons/ofxGui/src/ofxBaseGui.h b/addons/ofxGui/src/ofxBaseGui.h index 835c1c11e4c..9d0c80b0557 100644 --- a/addons/ofxGui/src/ofxBaseGui.h +++ b/addons/ofxGui/src/ofxBaseGui.h @@ -142,10 +142,17 @@ class ofxBaseGui { void setNeedsRedraw(); ofCoreEvents * events = nullptr; + + void setNameListener(); + private: bool needsRedraw; unsigned long currentFrame; bool bRegisteredForMouseEvents; + ofEventListener nameChangeListener; + + void nameChanged( std::string& ); + //std::vector coreListeners; }; diff --git a/addons/ofxGui/src/ofxButton.cpp b/addons/ofxGui/src/ofxButton.cpp index fbbeb2b6d85..901de399894 100644 --- a/addons/ofxGui/src/ofxButton.cpp +++ b/addons/ofxGui/src/ofxButton.cpp @@ -26,7 +26,7 @@ ofxButton* ofxButton::setup(ofParameter _bVal, float width, float height){ registerMouseEvents(); value.addListener(this,&ofxButton::valueChanged); - + setNameListener(); return this; } @@ -43,7 +43,7 @@ ofxButton* ofxButton::setup(const std::string& toggleName, float width, float he registerMouseEvents(); value.addListener(this,&ofxButton::valueChanged); - + setNameListener(); return this; } diff --git a/addons/ofxGui/src/ofxColorPicker.cpp b/addons/ofxGui/src/ofxColorPicker.cpp index 36cca596f23..57cc1fe9d0f 100644 --- a/addons/ofxGui/src/ofxColorPicker.cpp +++ b/addons/ofxGui/src/ofxColorPicker.cpp @@ -168,6 +168,8 @@ ofxColorPicker_ * ofxColorPicker_::setup(ofParameter diff --git a/addons/ofxGui/src/ofxGuiGroup.cpp b/addons/ofxGui/src/ofxGuiGroup.cpp index 82b99acc0e8..a077b6d9476 100644 --- a/addons/ofxGui/src/ofxGuiGroup.cpp +++ b/addons/ofxGui/src/ofxGuiGroup.cpp @@ -131,6 +131,7 @@ ofxGuiGroup * ofxGuiGroup::setup(const ofParameterGroup & _parameters, const std parameters = _parameters; registerMouseEvents(); + setNameListener(); setNeedsRedraw(); return this; diff --git a/addons/ofxGui/src/ofxInputField.cpp b/addons/ofxGui/src/ofxInputField.cpp index f2d09c053fd..1a496d70bac 100644 --- a/addons/ofxGui/src/ofxInputField.cpp +++ b/addons/ofxGui/src/ofxInputField.cpp @@ -148,6 +148,7 @@ ofxInputField* ofxInputField::setup(ofParameter _val, float wi listeners.push(value.newListener(this,&ofxInputField::valueChanged)); listeners.push(ofEvents().charEvent.newListener(this, &ofxInputField::charPressed, OF_EVENT_ORDER_BEFORE_APP)); listeners.push(ofEvents().keyPressed.newListener(this, &ofxInputField::keyPressed, OF_EVENT_ORDER_BEFORE_APP)); + setNameListener(); return this; } diff --git a/addons/ofxGui/src/ofxSlider.cpp b/addons/ofxGui/src/ofxSlider.cpp index 67a5dad1ba9..38dab0d20fc 100644 --- a/addons/ofxGui/src/ofxSlider.cpp +++ b/addons/ofxGui/src/ofxSlider.cpp @@ -57,7 +57,7 @@ ofxSlider* ofxSlider::setup(ofParameter _val, float width, flo value.addListener(this,&ofxSlider::valueChanged); registerMouseEvents(); - + setNameListener(); input.setup(_val, width, height); return this; } diff --git a/addons/ofxGui/src/ofxSliderGroup.cpp b/addons/ofxGui/src/ofxSliderGroup.cpp index 19897fd9324..8ac6c32781d 100644 --- a/addons/ofxGui/src/ofxSliderGroup.cpp +++ b/addons/ofxGui/src/ofxSliderGroup.cpp @@ -29,6 +29,7 @@ ofxVecSlider_ * ofxVecSlider_::setup(ofParameter valu } sliderChanging = false; + setNameListener(); return this; } @@ -154,7 +155,7 @@ ofxColorSlider_ * ofxColorSlider_::setup(ofParameter>().newListener(this, & ofxColorSlider_::changeValue)); - + setNameListener(); sliderChanging = false; minimize(); return this; @@ -294,7 +295,7 @@ ofxRectangleSlider * ofxRectangleSlider::setup(ofParameter value, f add(createGuiElement>(h, width, height)); listeners.push(h.newListener(this, & ofxRectangleSlider::changeSlider)); - + setNameListener(); sliderChanging = false; return this; diff --git a/addons/ofxGui/src/ofxToggle.cpp b/addons/ofxGui/src/ofxToggle.cpp index 2d25bc23c12..ef51bd74ba9 100644 --- a/addons/ofxGui/src/ofxToggle.cpp +++ b/addons/ofxGui/src/ofxToggle.cpp @@ -22,7 +22,7 @@ ofxToggle * ofxToggle::setup(ofParameter _bVal, float width, float height) value.addListener(this,&ofxToggle::valueChanged); registerMouseEvents(); setNeedsRedraw(); - + setNameListener(); return this; } From f21b715c68d3989729372eb8cb68aac278a2cead Mon Sep 17 00:00:00 2001 From: Roy Macdonald Date: Sat, 5 Sep 2020 23:41:26 -0400 Subject: [PATCH 6/9] ofParameter: fixed commentedout protected and private keywords --- libs/openFrameworks/types/ofParameter.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 46e8ef8bad2..281be39a411 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -263,10 +263,10 @@ class ofParameterGroup: public ofAbstractParameter { std::vector >::const_reverse_iterator rbegin() const; std::vector >::const_reverse_iterator rend() const; -//protected: +protected: const void* getInternalObject() const; -//private: +private: class Value{ public: Value() From 8b02c6a705b5563a1693ce5bb0004a9c824d3a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduard=20Frigola=20Bague=CC=81?= Date: Thu, 4 Mar 2021 01:01:53 +0100 Subject: [PATCH 7/9] Change return type of ofParameter::setName method Changing to a name that already exists in some parent is not allowed, setName returns false, and name is not changed. --- libs/openFrameworks/types/ofParameter.cpp | 13 +++-- libs/openFrameworks/types/ofParameter.h | 31 ++++++------ .../openFrameworks/types/ofParameterGroup.cpp | 49 +++++++++++++------ 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/libs/openFrameworks/types/ofParameter.cpp b/libs/openFrameworks/types/ofParameter.cpp index 99198bda553..74b0e41343c 100644 --- a/libs/openFrameworks/types/ofParameter.cpp +++ b/libs/openFrameworks/types/ofParameter.cpp @@ -79,12 +79,15 @@ ofParameter::ofParameter(const string& name) } -void ofParameter::setName(const string & name){ - std::string oldName = getEscapedName(); +bool ofParameter::setName(const string & name){ + std::string oldName = getName(); + if(escape(name) == escape(oldName)) return false; + if(!ofParameterGroup::changeChildName(this, obj->parents, escape(oldName), escape(name))){ + setName(oldName); + return false; + } obj->name = name; - - ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); - + return true; } diff --git a/libs/openFrameworks/types/ofParameter.h b/libs/openFrameworks/types/ofParameter.h index 281be39a411..e3b090b914c 100644 --- a/libs/openFrameworks/types/ofParameter.h +++ b/libs/openFrameworks/types/ofParameter.h @@ -24,7 +24,7 @@ class ofAbstractParameter{ public: virtual ~ofAbstractParameter(){} virtual std::string getName() const = 0; - virtual void setName(const std::string & name) = 0; + virtual bool setName(const std::string & name) = 0; virtual std::string toString() const = 0; virtual void fromString(const std::string & str) = 0; @@ -228,7 +228,7 @@ class ofParameterGroup: public ofAbstractParameter { friend std::ostream& operator<<(std::ostream& os, const ofParameterGroup& group); std::string getName() const; - void setName(const std::string& name); + bool setName(const std::string& name); std::string getEscapedName() const; std::string toString() const; void fromString(const std::string& name); @@ -273,7 +273,7 @@ class ofParameterGroup: public ofAbstractParameter { :serializable(true){} void notifyParameterChanged(ofAbstractParameter & param); - void updateParameterName(const std::string oldName, const std::string newName); + bool updateParameterName(const std::string oldName, const std::string newName); void notifyParameterNameChanged(ofAbstractParameter & param); std::map parametersIndex; @@ -292,7 +292,7 @@ class ofParameterGroup: public ofAbstractParameter { static void checkAndRemoveExpiredParents(std::vector> & parents); - static void changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName); + static bool changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName); template friend class ofParameter; @@ -517,7 +517,7 @@ class ofParameter: public ofAbstractParameter{ const ParameterType * operator->() const; operator const ParameterType & () const; - void setName(const std::string & name); + bool setName(const std::string & name); std::string getName() const; ParameterType getMin() const; @@ -836,12 +836,15 @@ inline ofParameter::operator const ParameterType & () const{ } template -void ofParameter::setName(const std::string & name){ - std::string oldName = getEscapedName(); +bool ofParameter::setName(const std::string & name){ + std::string oldName = getName(); + if(escape(name) == escape(oldName)) return false; + if(!ofParameterGroup::changeChildName(this, obj->parents, escape(oldName), escape(name))){ + setName(oldName); + return false; + } obj->name = name; - - ofParameterGroup::changeChildName(this, obj->parents, oldName, getEscapedName()); - + return true; } template ofEvent& ofParameter::nameChangedEvent() @@ -1026,7 +1029,7 @@ class ofParameter: public ofAbstractParameter{ ofParameter& set(const std::string & name); - void setName(const std::string & name); + bool setName(const std::string & name); std::string getName() const; std::string toString() const; @@ -1151,7 +1154,7 @@ class ofReadOnlyParameter: public ofAbstractParameter{ protected: virtual ofEvent& nameChangedEvent(); - void setName(const std::string & name); + bool setName(const std::string & name); void enableEvents(); void disableEvents(); void setSerializable(bool s); @@ -1313,8 +1316,8 @@ inline std::unique_ptr ofReadOnlyParameter -inline void ofReadOnlyParameter::setName(const std::string & name){ - parameter.setName(name); +inline bool ofReadOnlyParameter::setName(const std::string & name){ + return parameter.setName(name); } template diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index bb51b325174..6444314a987 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -343,11 +343,15 @@ string ofParameterGroup::getName() const{ return obj->name; } -void ofParameterGroup::setName(const string & name){ - std::string oldName = getEscapedName(); +bool ofParameterGroup::setName(const string & name){ + std::string oldName = getName(); obj->name = name; - - changeChildName(this, obj->parents, oldName, getEscapedName()); + + if(!ofParameterGroup::changeChildName(this, obj->parents, escape(oldName), getEscapedName())){ + setName(oldName); + return false; + } + return true; } string ofParameterGroup::getEscapedName() const{ @@ -445,11 +449,16 @@ void ofParameterGroup::Value::notifyParameterNameChanged(ofAbstractParameter & p }),parents.end()); } -void ofParameterGroup::Value::updateParameterName(const std::string oldName, const std::string newName){ - if(oldName != newName){ - parametersIndex[newName] = parametersIndex[oldName]; - parametersIndex.erase(oldName); - } +bool ofParameterGroup::Value::updateParameterName(const std::string oldName, const std::string newName){ + if(parametersIndex.find(newName) != parametersIndex.end()){ + return false; + } + if(oldName != newName){ + parametersIndex[newName] = parametersIndex[oldName]; + parametersIndex.erase(oldName); + return true; + } + return false; } const ofParameterGroup ofParameterGroup::getFirstParent() const{ @@ -551,19 +560,28 @@ void ofParameterGroup::checkAndRemoveExpiredParents(std::vector> & parents, const std::string& oldName, std::string newName) +bool ofParameterGroup::changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName) { - if(!child) return; + if(!child) return false; // name has not change, no need to notify anything - if(oldName == newName) return; + if(oldName == newName) return false; checkAndRemoveExpiredParents(parents); - for(auto & parent: parents){ - auto p = parent.lock(); + for(auto parent = parents.begin(); parent != parents.end(); ++parent){ + auto p = parent->lock(); if(p){ - p->updateParameterName(oldName, newName); + if(!p->updateParameterName(oldName, newName)){ + // Undo the name change in the paremeters that we already did + for(auto reverseParent = parent-1; reverseParent != parents.begin()-1; --reverseParent){ + auto rp = reverseParent->lock(); + if(rp){ + rp->updateParameterName(newName, oldName); + } + } + return false; + } } } @@ -577,6 +595,7 @@ void ofParameterGroup::changeChildName(ofAbstractParameter* child, std::vectornotifyParameterNameChanged(*child); } } + return true; } ofEvent& ofParameterGroup::nameChangedEvent() From d452ff2b149503ce00d896ff4fbc890cdf818457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduard=20Frigola=20Bague=CC=81?= Date: Thu, 4 Mar 2021 01:03:25 +0100 Subject: [PATCH 8/9] Add testing for changing parameter name --- tests/types/parameters/src/main.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/types/parameters/src/main.cpp b/tests/types/parameters/src/main.cpp index 2ffbd427df0..6d89482bf87 100644 --- a/tests/types/parameters/src/main.cpp +++ b/tests/types/parameters/src/main.cpp @@ -8,11 +8,20 @@ class ofApp: public ofxUnitTestsApp{ ofParameter p2{"p>2", 0, 0, 1000}; ofParameter p3{"p>3", 0, 0, 1000}; ofParameter p4{"p>4", 0, 0, 1000}; + ofParameter p5{"p>5", 0, 0, 1000}; ofParameterGroup group{ "group", p1, p2, p3, p4 }; + ofParameterGroup group2{ + "group2", + p4, p5 + }; group.remove(p1); + + ofxTest(!p4.setName("p>5"), "Group 2 already contains a parameter named p>5"); //Issue #6576 + ofxTest(p4.getName() != "p>5", "Name shouldn't be changed to p>5"); //Issue #6576 + ofxTest(!group.contains("p>5"), "Group shouldn't contain any parameter named p>5"); //Issue #6576 ofxTest(!group.contains("p>1"), "Group shouldn't contain p1 after remove"); ofxTestEq(group.get("p>2").getName(), "p>2", "p2 name " + group.get("p>2").getName() + " should be p>2, probably index map is corrupt"); //Issue #6016 ofxTestEq(group.get("p>3").getName(), "p>3", "p3 name " + group.get("p>3").getName() + " should be p>3, probably index map is corrupt"); //Issue #6016 From b9382a9f81d783b0cb866fc772f131b7fc73b918 Mon Sep 17 00:00:00 2001 From: Eduard Frigola Date: Fri, 2 Jul 2021 19:45:56 +0200 Subject: [PATCH 9/9] Fix assigning same name in parameterGroup caused infinite recursion --- libs/openFrameworks/types/ofParameterGroup.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index 6444314a987..3f2477566df 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -345,12 +345,12 @@ string ofParameterGroup::getName() const{ bool ofParameterGroup::setName(const string & name){ std::string oldName = getName(); - obj->name = name; - - if(!ofParameterGroup::changeChildName(this, obj->parents, escape(oldName), getEscapedName())){ + if (escape(name) == escape(oldName)) return false; + if (!ofParameterGroup::changeChildName(this, obj->parents, escape(oldName), escape(name))) { setName(oldName); return false; } + obj->name = name; return true; }