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 6d217c7eb34..a8d3fd5ea89 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 de217ec9659..0e8921deb47 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; } diff --git a/libs/openFrameworks/types/ofParameter.cpp b/libs/openFrameworks/types/ofParameter.cpp index 513e5ef330c..459475a74a2 100644 --- a/libs/openFrameworks/types/ofParameter.cpp +++ b/libs/openFrameworks/types/ofParameter.cpp @@ -80,8 +80,21 @@ ofParameter::ofParameter(const string& name) } -void ofParameter::setName(const string & name){ +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; + return true; +} + + +ofEvent& ofParameter::nameChangedEvent() +{ + return obj->nameChangedEvent; } string ofParameter::getName() const{ @@ -110,11 +123,8 @@ void ofParameter::trigger(){ // 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(), - [](const std::weak_ptr & p){ return p.expired(); }), - obj->parents.end()); + // Erase each invalid parent + 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 @@ -135,10 +145,7 @@ 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(), - [](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 64044391a01..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; @@ -67,11 +67,16 @@ class ofAbstractParameter{ virtual bool isReferenceTo(const ofAbstractParameter& other) const; + virtual ofEvent& nameChangedEvent() = 0; + 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; + + + }; @@ -223,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); @@ -245,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; @@ -265,18 +273,27 @@ class ofParameterGroup: public ofAbstractParameter { :serializable(true){} void notifyParameterChanged(ofAbstractParameter & param); - + bool 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) :obj(obj){} + + static void checkAndRemoveExpiredParents(std::vector> & parents); + + static bool changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName); + template friend class ofParameter; @@ -500,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; @@ -580,9 +597,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{ @@ -590,6 +609,8 @@ class ofParameter: public ofAbstractParameter{ } } + virtual ofEvent& nameChangedEvent(); + size_t getNumListeners() const; const void* getInternalObject() const; @@ -631,6 +652,7 @@ class ofParameter: public ofAbstractParameter{ ParameterType value; ParameterType min, max; ofEvent changedE; + ofEvent nameChangedEvent; bool bInNotify; bool serializable; std::vector> parents; @@ -745,10 +767,7 @@ 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(), - [](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 @@ -817,8 +836,20 @@ inline ofParameter::operator const ParameterType & () const{ } template -void ofParameter::setName(const std::string & name){ +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; + return true; +} +template +ofEvent& ofParameter::nameChangedEvent() +{ + return obj->nameChangedEvent; } template @@ -998,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; @@ -1048,6 +1079,9 @@ class ofParameter: public ofAbstractParameter{ const void* getInternalObject() const{ return obj.get(); } + + virtual ofEvent& nameChangedEvent(); + protected: private: @@ -1062,6 +1096,7 @@ class ofParameter: public ofAbstractParameter{ std::string name; ofEvent changedE; + ofEvent nameChangedEvent; bool serializable; std::vector> parents; }; @@ -1118,7 +1153,8 @@ class ofReadOnlyParameter: public ofAbstractParameter{ std::string valueType() const; protected: - void setName(const std::string & name); + virtual ofEvent& nameChangedEvent(); + bool setName(const std::string & name); void enableEvents(); void disableEvents(); void setSerializable(bool s); @@ -1280,8 +1316,13 @@ 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 +inline ofEvent& ofReadOnlyParameter::nameChangedEvent(){ + return parameter.nameChangedEvent(); } template diff --git a/libs/openFrameworks/types/ofParameterGroup.cpp b/libs/openFrameworks/types/ofParameterGroup.cpp index a39932322b9..e644cb532ca 100644 --- a/libs/openFrameworks/types/ofParameterGroup.cpp +++ b/libs/openFrameworks/types/ofParameterGroup.cpp @@ -352,8 +352,15 @@ string ofParameterGroup::getName() const{ return obj->name; } -void ofParameterGroup::setName(const string & name){ +bool ofParameterGroup::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; + return true; } string ofParameterGroup::getEscapedName() const{ @@ -441,6 +448,28 @@ void ofParameterGroup::Value::notifyParameterChanged(ofAbstractParameter & param }),parents.end()); } +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()); +} + +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{ auto first = std::find_if(obj->parents.begin(),obj->parents.end(),[](const weak_ptr & p){return p.lock()!=nullptr;}); if(first!=obj->parents.end()){ @@ -531,3 +560,58 @@ 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()); +} + + +bool ofParameterGroup::changeChildName(ofAbstractParameter* child, std::vector> & parents, const std::string& oldName, std::string newName) +{ + if(!child) return false; + + // name has not change, no need to notify anything + if(oldName == newName) return false; + + checkAndRemoveExpiredParents(parents); + + for(auto parent = parents.begin(); parent != parents.end(); ++parent){ + auto p = parent->lock(); + if(p){ + 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; + } + } + } + + 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); + } + } + return true; +} + +ofEvent& ofParameterGroup::nameChangedEvent() +{ + return obj->nameChangedEvent; +} +ofEvent& ofParameterGroup::childNameChangedEvent() +{ + return obj->childNameChangedEvent; +} 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