Skip to content

Conversation

@hiroMTB
Copy link
Contributor

@hiroMTB hiroMTB commented Dec 14, 2018

Right now we are able to initialize ofParameter like below in ofApp.h.

ofParameter<int>  fps{"FPS", 0, 0, 120};
ofParameter<bool> bFullscreen{"Fullscreen", false};
ofParameterGroup  grp{"general settings", fps, bFullscreen};

The first parameter (e.g. "FPS") is the name of parameter then initial value, min and max.
This is very useful because we can write in header file and reduce code in ofApp.cpp file.
I'm using this often in my project since I found this syntax in Entropy project(for example here),

It would be nicer if we could initialize "serializable" variable as well in same manner. With this PR we can write this way and reduce .setSerializable(false) in ofApp.cpp to avoid saving parameter to xml (or json).

ofParameter<int>  fps{"FPS", 0, 0, 120, false};
ofParameter<bool> bFullscreen{"Fullscreen", false, false};
ofParameterGroup  grp{"general settings", fps, bFullscreen};

*Same to ofReadOnlyParameter

@arturoc
Copy link
Member

arturoc commented Dec 14, 2018

The problem with this is that it adds a bool parameter to a constructor which already has a lot of parameters. bool parameters are really hard to understand since a true/false in the 4th position of the call could be anything

Perhaps we could add an enum with OF_SERIALIZABLE / OF_NOT_SERIALIZABLE values or something similar that replaces that parameter so your example would look something like;

ofParameter<int>  fps{"FPS", 0, 0, 120, OF_NOT_SERIALIZABLE};
ofParameter<bool> bFullscreen{"Fullscreen", false, OF_NOT_SERIALIZABLE};
ofParameterGroup  grp{"general settings", fps, bFullscreen};

which is much clearer

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Dec 14, 2018

Totally agree. I will work on it.
Shall we also consider initializing bInNotify as well?

@arturoc
Copy link
Member

arturoc commented Dec 14, 2018

what is binNotify?

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Dec 14, 2018

Or we could also have ofPrameterSettings as a struct? But it could be almost same with existing Value class

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Dec 14, 2018

bInNotify is another ofParameter variable to be set false in default. I guess switch on/off calling ofNotifyEvent when variable changed. Will check later when I'm back on my desk.

@arturoc
Copy link
Member

arturoc commented Dec 14, 2018

oh i see, perhaps we can leave that one out or perhaps we could have binary flags like:

enum ofParameterFlags{
    OF_PARAM_NOT_SERIALIZABLE = 1 << 0,
    OF_PARAM_DONT_NOTIFY          = 1 << 1,
};

then you can do:

ofParameter<bool> bFullscreen{"Fullscreen", false, OF_PARAM_NOT_SERIALIZABLE | OF_PARAM_DONT_NOTIFY};

@hiroMTB
Copy link
Contributor Author

hiroMTB commented Dec 14, 2018

Nice idea. It would be little mysterious for C++ beginners to use | operator but this is advanced feature so might not be a big problem.

I had a time to think again about future development of ofParameter. For example if we want to add "step" variable which is useful to change value only with specified step. So the resulted value will be always quantized to the certain distance. Let's say we have ofParameter with initial value of 0.0f and and we set 1.5f for step variable. And when I change the value to 2.9f it will automatically quantized to 3.0f.

This feature is common in most of GUI mechanism like in iOS.

To implement this feature, ofParameterFlags won't be enough since it can only hold boolean.

How do you think?

@eduardfrigola
Copy link
Contributor

Hi!

I like the idea to have more information inside ofParameter, and to be easy to modify it. I agree with @hiroMTB about having other types than boolean it's needed. Also @arturoc did use also a fourth argument in here: #5598 and that conflicts with it.

I think that going to a more descriptive approach will be simpler, and less confusing. For example what memo did with ofxMSAControlFreak
the only thing that has to be changed is that the "set" methods have to return the parameter itself, useful also when setting the parameter inside a group.

ofParameterGroup parameters;
ofParameter<float> floatParam;
// Proposed method
parameters->add(floatParam("Float", 0, 0, 1, false));

// New method
parameters->add(floatParam("Float", 0, 0, 1).setSerializable(false));

Also if setSerializable (and all other setters) returned the parameter itself (don't know if that is a cpp aberration), you could do:

parameters->add(floatParam("Float", 0, 0, 1).setSerializable(false).setNotify(false).setStep(1.5).setScale(ofParameterScale::Logarithmic));

And about future development of ofParameter... I don't know if it's the best place to discus it, but I would like to be able to, for example, declare a simple class, that inherits from ofAbstractParameterFlags or ofAbstractParameterInfo, with custom arguments, and can be stored inside the ofParameter itself. This way you can have the associated information about ofParameter inside the ofParameterGroup, and can benefit from passing only the parameterGroup for GUI related stuff. OfParameter will keeps simple and extendable.

Eduard.

@hiroMTB hiroMTB closed this Dec 21, 2023
@hiroMTB hiroMTB deleted the feature-ofParameter-useful-constructor branch December 21, 2023 22:32
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.

3 participants