Adopt naming schema from other controls in Mixxx for effect parameters #207
Conversation
|
Possible future plans: parameterN_value_max |
|
Some of these controls are unused. |
|
matching skins are here https://github.com/mixxxdj/developer_skins/tree/pull_207 |
| const unsigned int iParameterNumber) { | ||
| return QString("[EffectRack%1_EffectUnit%2_Effect%3_Parameter%4]") | ||
| const unsigned int iSlotNumber) { | ||
| return QString("[EffectRack%1_EffectUnit%2_Effect%3]") |
There was a problem hiding this comment.
Please keep the entire group for the parameter. There is a lot of stuff in here and it will get crowded very quickly. It deserves its own namespace.
There was a problem hiding this comment.
Hm, actually I don't mind either way.
There was a problem hiding this comment.
You implementation would be new for Mixxx. Elsewhere we have the the naming
[Channel1],play
[Channel1],play_indicator
[Channel1],rate
[Channel1],rate_step_up
I have just made it fit.
Undocumented is fine. I also don't mind if they are removed. |
I'm talking about the min/max/max_limit/min_limit here. Though I think script authors may be interested in min/max at least if they wish to set raw values. |
…ept with the inter control solution. If we later have a usecase that a min or max value should be tweakable for a effect, this effect might handle it intern by a new dedicated parameter.
|
Ready! Now we have new ControllEffectKnob which manages the setting curve. I have also try to fix the bitchrusher but a logarithmic scale for bit dep does not help. |
|
Ok, finally the logarithmic effect parameter are working. |
| @@ -209,7 +209,12 @@ void EffectParameter::onChainParameterChanged(double chainParameter) { | |||
| } | |||
| max = m_maximum.toDouble(); | |||
| min = m_minimum.toDouble(); | |||
| setValue(min + chainParameter * (max - min)); | |||
| if (m_parameter.controlHint() == EffectManifestParameter::CONTROL_KNOB_LOGARITHMIC) { | |||
There was a problem hiding this comment.
Ah, can't we call setParameter(chainParameter) now instead of this log vs. linear logic?
There was a problem hiding this comment.
Er, nevermind I misread where we were.
There was a problem hiding this comment.
Ah, now I see. To get rid of this logic the superknob parameter linking should flow from EffectChainSlot -> EffectSlot -> EffectParameterSlot -> value CO instead of from EffectChainSlot -> EffectChain -> Effect -> EffectParameter::onChainParameterChanged(...).
There was a problem hiding this comment.
Yes, this is is definitely a candidate for refactoring. I do not get your point exactly.
Since this is merged, I will file a launchpad bug to continue discussing.
|
Nice! This looks good -- thanks @daschuer. |
Adopt naming schema from other controls in Mixxx for effect parameters
README: document how to test new blog posts locally
With this PR, we are more in line with the rest of Mixxx.
Now we have: