Numark Mixtrack Pro mapping fix and upgrade v.2.5 - 2.6 - #15629
Numark Mixtrack Pro mapping fix and upgrade v.2.5 - 2.6#15629vespadj wants to merge 310 commits into
Conversation
…ck-qml-proxy feat: refactor player proxy and create separated track proxy
…rdware-setting fix: prevent unnecessary need restart popup
…rm-on-qml feat: add more renderers on QML
…veform-zoom-exp QML: Improve waveform zoom exp
Use FindWrapOpenGL.cmake. It allows X11-less system. Set link_target to OpenGL::OpenGL, GLVND-based. If not found, use OpenGL:GL. Furthermore, adding a __X11__ definition so that the screensaver that requires Xlib is now optional. Signed-off-by: Nicolas PARLANT <nicolas.parlant@parhuet.fr>
X11-less - Use FindWrapOpenGL
add 'LoadTrackFromPreviewDeck' control
Because X11/XKBlib.h is a part of libX11 Signed-off-by: Nicolas PARLANT <nicolas.parlant@parhuet.fr>
Don't try localeFromXkbSymbol w/o __X11__ defined
…ch-2.6-to-main
Merge changes from `2.6` into `main`
…pping_validation_test and controllerscriptenginelegacy_test executable without crash
…ch-2.6-to-main
…ch-2.6-to-main
Merge changes from `2.6` into `main`
…ch-2.6-to-main
Merge changes from `2.6` into `main`
…rProxy Create JavascriptPlayerProxy
Merge changes from `2.6` into `main`
Don't use asKeyValueRange() when building with Qt 6.2
|
|
This PR is marked as stale because it has been open 90 days with no activity. |
|
Okay, this looks good. Please rebase onto 2.6 and try to adjust commit messages to reflect the changes ("reviews 3" is not helpful when looking at the commit history later on). Maybe even squash related commits? But I'll leave that up to you as it might be cumbersome. |
ebf1d30 to
3c6db1a
Compare
|
I done. I'm not sure about the git push command, please give me instructions if it's necessary, for this PR and the relative manual (link above). |
|
Thanks. You force-pushed but changing the base here to 2.6 (now 309 commits) indicates you didn't rebase onto 2.6 I've found some nits in the last commit(s), minor requests only, and will leave a review soon. |
ronso0
left a comment
There was a problem hiding this comment.
another review run..
Btw did you close the previous review convos yourslef or did this happen automatically after you force-pushed?
For the record: convos are closed by reviewers. Else we don't know which have been actually resolved and need to go through all again..
| NumarkMixTrackPro.isPflOn = [0, 0]; | ||
| engine.makeConnection("[Channel1]", "pfl", (value) => { | ||
| NumarkMixTrackPro.isPflOn[0] = value; | ||
| console.log("NumarkMixTrackPro.isPflOn[0]", NumarkMixTrackPro.isPflOn[0]); |
There was a problem hiding this comment.
debug leftover?
if yes, please remove
| NumarkMixTrackPro.LedTimer = function(id, led, count, state) { | ||
| this.id = id; | ||
| this.led = led; | ||
| this.count = count; | ||
| this.state = state; | ||
| }; |
There was a problem hiding this comment.
not used, deleted in next commit.
| NumarkMixTrackPro.setStutterBeat = function(deck, value) { | ||
| const secondsBlink = 30; | ||
| const secondsToEnd = | ||
| engine.getParameter(`[Channel${deck}]`, "duration") * | ||
| (1 - engine.getParameter(`[Channel${deck}]`, "playposition")); | ||
|
|
||
| if ( | ||
| secondsToEnd < secondsBlink && | ||
| secondsToEnd > 1 && | ||
| engine.getParameter(`[Channel${deck}]`, "play") | ||
| ) { | ||
| // The song is going to end | ||
| NumarkMixTrackPro.setLED(NumarkMixTrackPro.leds[deck].Cue, value); | ||
| } | ||
|
|
||
| NumarkMixTrackPro.setLED(NumarkMixTrackPro.leds[deck].stutter, value); | ||
| }; |
There was a problem hiding this comment.
I suggest to connect to end_of_track
That way users can define the threshold in Preferences -> Waveforms
And the LED currently doesn't blink, or does it?
There was a problem hiding this comment.
OK, done. Stutter LED blink at each beat and Cue blink to at the end of the song.
There was a problem hiding this comment.
used makeConnection in the proper way with group. Deleted two callback functions, and renamed the rest.
| NumarkMixTrackPro.Stutter1Beat = function(value) { | ||
| NumarkMixTrackPro.setStutterBeat(1, value); | ||
| }; | ||
|
|
||
| NumarkMixTrackPro.Stutter2Beat = function(value) { | ||
| NumarkMixTrackPro.setStutterBeat(2, value); | ||
| }; |
There was a problem hiding this comment.
These wrappers are not necessary if you use a lambda (or whatever that is called in js) for the stuterbeat makeConnections like you do for other connections, then just call setStutterBeat (rename to flashStutterLED?)
There was a problem hiding this comment.
oh, kinda already covered by Nicko's review #15629 (comment)
There was a problem hiding this comment.
So will you resolve this conversation?
There was a problem hiding this comment.
Niko's review, yes.
his was about the wrappers, I'm saying we only need NumarkMixTrackPro.setStutterBeat(deck, value) and call it from the lambdas
| variable="numarkMixTrackPro_brakeEnabled" | ||
| type="boolean" | ||
| default="false" | ||
| label='Play/Pause performs Brake and Soft Start if "SCRATCH LED" is on. [Require Mixxx v.2.6+]'> |
There was a problem hiding this comment.
The tooltip mentioning 2.6+ is however still in place in the xml
| NumarkMixTrackPro.Channel1Clip = function(value) { | ||
| NumarkMixTrackPro.clipLED(value, NumarkMixTrackPro.leds[1].sync); | ||
| }; | ||
|
|
||
| NumarkMixTrackPro.Channel2Clip = function(value) { | ||
| NumarkMixTrackPro.clipLED(value, NumarkMixTrackPro.leds[2].sync); | ||
| }; |
| // cargar el tema con el pitch en 0 | ||
| engine.softTakeover(group, "rate", false); | ||
| engine.setValue(group, "rate", 0); | ||
| engine.setParameter(group, "rate", 0.5); |
There was a problem hiding this comment.
as per Niko's review:
don't do this in the mapping, it's a user setting that can be set in Preferences -> Decks -> Speed / Key options
| if (engine.getValue(group, "play")) { | ||
| engine.setValue(group, "play", 0); | ||
| } else { | ||
| engine.setValue(group, "play", 1); | ||
| } |
There was a problem hiding this comment.
can be simply
script.toggleControl(group, "play");
| if (engine.getParameter(group, "quantize")) { | ||
| engine.setParameter(group, "quantize", 0); | ||
| } else { | ||
| engine.setParameter(group, "quantize", 1); | ||
| } |
There was a problem hiding this comment.
script.toggleControl(group, "quantize");
This PR enhances the Numark Mixtrack Pro controller mapping with several improvements and modernizations:
sync_enabled.deleteKeytoshiftKey, and removed old [Flanger] references.All changes have been tested and are ready for integration.