-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 halve/double controls for beatjump size #4269
Conversation
If we consider the missing controls a bug (I tend to say yes) I would rebase onto 2.3 |
eaa5c57
to
7a0e794
Compare
Also, controller mappings now can use the controls directly and don't need to implement the halve/double logic themselves anymore. |
IMHO this really streches the definition of a bug. It's a feature. |
Alright, any comments on the code? |
@@ -166,6 +166,18 @@ LoopingControl::LoopingControl(const QString& group, | |||
this, &LoopingControl::slotBeatJump, Qt::DirectConnection); | |||
m_pCOBeatJumpSize = new ControlObject(ConfigKey(group, "beatjump_size"), | |||
true, false, false, 4.0); | |||
|
|||
m_pCOBeatJumpSizeHalve = new ControlPushButton(ConfigKey(group, "beatjump_size_halve")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The delete call is missing if you wish to match the existing code.
A better solution would be using a unique_ptr to manage the live-time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now, I added the delete call and a TODO for unique_ptr.
I could change all controls here to unique_ptr, though, how would I deal with the controls in lists like QList<BeatLoopingControl*> m_beatLoops;
? They'd remain regular pointers, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QList does not support unique_ptr we may change to std::vector to work around this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR. I have left some comments.
7a0e794
to
2db57cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
Allows to change the beatjump size with keyboards.
beatjump_size_halve
beatjump_size_double
quick fix for a forum request.
https://mixxx.discourse.group/t/maintening-a-key-pressed-doesnt-have-any-effect/22977