Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement notification loop safeguard in ofParameter<void> #7122

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eduardfrigola
Copy link
Contributor

As discussed in #6602 this PR makes ofParameter<void> behave the same way as the rest of parameters does.
As it is a template specialisation, the code proposed in this pull request is just coping the same functionality as is already implemented for the general ofParameter template.

inline void ofParameter<ParameterType>::eventsSetValue(const ParameterType & v){
// If the object is notifying its parents, just set the value without triggering an event.
if(obj->bInNotify)
{
noEventsSetValue(v);
}
else
{
// Mark the object as in its notification loop.
obj->bInNotify = true;
// Set the value.
obj->value = v;
// Notify any local subscribers.
ofNotifyEvent(obj->changedE,obj->value,this);
// 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<ofParameterGroup::Value> & 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->notifyParameterChanged(*this);
}
}
}
obj->bInNotify = false;
}
}

This pull request does not address the fact that, implementing the same functionality twice, makes the code more prone to errors, and makes you responsible on checking differences in behaviour between the template and the specialisation when some changes are made. For that matter I feel a separate PR could be more clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant