-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add useful ofParameter constructor #6195
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
Add useful ofParameter constructor #6195
Conversation
…ble via initializer list
…erializable via initializer list
|
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 |
|
Totally agree. I will work on it. |
|
what is binNotify? |
|
Or we could also have ofPrameterSettings as a struct? But it could be almost same with existing Value class |
|
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. |
|
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}; |
|
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? |
|
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 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. |
Right now we are able to initialize ofParameter like below in ofApp.h.
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).
*Same to ofReadOnlyParameter