Update Mixtrack (Pro) 3 mappings for Mixxx 2.1#1180
Update Mixtrack (Pro) 3 mappings for Mixxx 2.1#1180daschuer merged 83 commits intomixxxdj:masterfrom
Conversation
| // NMTP3 is a "Model A" controller for scratching, it centers on 0. | ||
| // See http://www.mixxx.org/wiki/doku.php/midi_scripting#scratching | ||
| // and see "Jogger" object constructor | ||
| this.beatJumpSize = 1; |
There was a problem hiding this comment.
In the future, this will be available as a ControlObject that will also be shown on skins so controller scripts won't have to keep track of this. I might implement that for Mixxx 2.2. There is a Launchpad ticket tracking it.
There was a problem hiding this comment.
That would be awesome. I will not be committing code that will modify this value until the CO will be there as this controller has no way of displaying it.
There was a problem hiding this comment.
You may find a beatjump size of 4 beats to be more helpful.
There was a problem hiding this comment.
FYI, I just tried to install Mixxx 2.1 on Windows 10 and the installer failed. Also, I got 3 different warnings asking to confirm that I wanted to install the package; the first one saying that Windows did not recommend installing it, likely because package was not signed.
There was a problem hiding this comment.
In Windows 10 you have to click "More Info" and then tell it you really mean to install it.
|
Will the new effects interface be used across all skins? There is currently a lot of code duplication as a result of having to support skins that have only one effect per rack. Documented here: // Effects mode
// 1 for multi-effect mode (3 effects controlled in each EffectUnit, best used with Deere Skin)
// 2 for single effect mode (1 effect controlled in each EffectUnit, best used with skins other than Deere)
var FXmode = 1; // multi-effect mode by default |
Yes, that's part of the point. Controller mappings having different options for different skins is a really ugly situation. |
|
Awesome, thank you. 👍 |
| } | ||
|
|
||
| for (i = engine.getValue("[Master]", "num_decks"); i >= 1; i--) { | ||
| for (var i = engine.getValue("[Master]", "num_decks"); i >= 1; i--) { |
There was a problem hiding this comment.
FYI, in the old version of JS that Mixxx still uses (and in modern JS using var), variables are scoped to functions, not blocks as you might expect. You don't need to redeclare i for each loop. Just something to be aware of if you encounter puzzling bugs with one loop messing up the behavior of the next.
There was a problem hiding this comment.
I'm aware that JS lacks block scoping when using var, and that variable declarations get hoisted, but I think pretty much everyone except Crockford will advocate for including var in for loops.
As an aside, the mapping in its current state is not functional since I committed stuff without having the controller on hand. I will go back and test things commit by commit though!
There was a problem hiding this comment.
Okay, just pointing it out because many people who contribute mapping scripts aren't that familiar with JS.
|
@djsteph Just came across this: https://docs.google.com/spreadsheets/d/1YbLS7hBprP2uvEKizO-b0WiaRmpvml0r5NfBp4GmBVc/edit#gid=0 I'm guessing it was most useful during development but I think I'd like to use it as well. It will make a fantastic quality check for the mapping. |
|
You are welcome to use it...I think anyone can edit the file
…On Fri, Feb 10, 2017 at 4:23 PM, Radu Suciu ***@***.***> wrote:
@djsteph <https://github.com/djsteph> Just came across this:
https://docs.google.com/spreadsheets/d/1YbLS7hBprP2uvEKizO-
b0WiaRmpvml0r5NfBp4GmBVc/edit#gid=0
I'm guessing it was most useful during development but I think I'd like to
use it as well. It will make a fantastic quality check for the mapping.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1180 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AQDKyQgEOAPE4mdCerrXSVUkS_pG51EBks5rbNVcgaJpZM4L81Rl>
.
|
|
Trying to debug things and getting a billion messages of this nature: Note that none of the buttons on the controller are flashing, but the messages keep pouring in. Is this normal? |
|
Yes, that is normal. I added outgoing MIDI debugging messages in #1004. Is that an issue? Would you like an option to only see incoming MIDI? |
3925289 to
38ed3f2
Compare
|
Well, it kind of makes it difficult to debug by logging things with Perhaps I'm doing debugging wrong? These are the options I am using: Thank you for your help! |
|
|
| contract = 0; | ||
| } | ||
|
|
||
| if (shifted && value === ON) { |
There was a problem hiding this comment.
Just curious, you are not keeping the maximize library on the browse button push? In this code sniplet, I don't see any value set for LibraryGroup and Library command nor do I see the actual change in the code to remove the variable
There was a problem hiding this comment.
That was an oversight - thanks for noticing. Will fix.
8cadac0 to
84edb44
Compare
- Pressing sampler button if no sample is currently loaded will load sample from library - Shift + sampler button if sample is loaded will eject appropriate sample Fix checking if track is loaded. Make sure sample is stopped before ejecting
| * the same tempo, and decks that also have quantize | ||
| * enabled will always have their beats lined up. | ||
| * If the Sync Loack was previously activated, it just desactivate it, | ||
| * If Sync Lock was previously activated, it just desactivate it, |
| } else { | ||
| // play sampler with Sync | ||
| engine.setValue(group, "cue_gotoandplay", 1); | ||
| engine.setValue(group, "beatsync", 1); |
There was a problem hiding this comment.
You removed the comment about playing with sync. Did you mean to remove the syncing?
There was a problem hiding this comment.
No, I just thought the code is pretty clear in this case. Shift + Sampler will play the sample without sync so it's pretty easy to switch between the two.
Earlier I thought there'd be an issue with mapping Shift + Sampler to stop a sample, but there isn't one since stopping is only done on playing samples and playing without sync is only done on non-playing samples.
| <MixxxControllerPreset mixxxVersion="2.1.0+" schemaVersion="1"> | ||
| <info> | ||
| <name>Numark Mixtrack (Pro) 3</name> | ||
| <author>Stéphane Morin</author> |
There was a problem hiding this comment.
Feel free to mention yourself here (Keep Stéphane's name too).
Be-ing
left a comment
There was a problem hiding this comment.
Wow that was a lot of code changed! Thanks for the cleanup. I found a handful of other places in the diff where the code could be cleaned up a little bit plus a few UX suggestions. Looks almost ready to merge.
| // fastSeekEnabled: enable fast seek with Jog Wheel with Wheel Off and Shift ON | ||
| // Shift can be locked or not | ||
| var fastSeekEnabled = true; | ||
|
|
||
| //activate PFL of deck on track load | ||
| var smartPFL = true; | ||
|
|
||
| // use beatlooproll instead of beatloop | ||
| var beatlooprollActivate = false; |
There was a problem hiding this comment.
This option could be removed by following the behavior of the new loopauto_toggle CO in #1187: normal loops for loops > 1 beat; rolling loops for loops <= 1 beat. That matches how the loop sizes are mapped to the pads in this. Normal loops without shift; rolling loops with shift.
There was a problem hiding this comment.
I don't think those behaviours are the same, this option makes it so that all the loops set with the auto loop buttons are rolling. Also, the Shift behaviour isn't to toggle loopRoll, but rather to allow for the toggling of fractional beat loops (mapping 4 buttons to 8 loop sizes).
I understand your suggestion but I'm not quite sure if I want all of my <= 1 beat loops to be rolling. Will consider it, especially if others want the shifted behaviour you describe.
For now, I will remove the option since I can't imagine anyone wanting all of their loops to be rolling, especially without a way to toggle. Will be revisiting this when #1187 is merged.
There was a problem hiding this comment.
What is the use of loops <= 1 beat that aren't rolling? Play with it, I think you'll find my suggestion makes sense.
There was a problem hiding this comment.
Say instead of rolling into a drop on the same track, you bring in another track instead, and you want to leave the other track where it is instead of having it slip past when you stop the loop.
I suppose there are other ways around that though..
| }); | ||
| // permanent timer | ||
| this.flashTimer = engine.beginTimer(num_ms_on + num_ms_off, function() { | ||
| myself.flashOnceOn(false); |
There was a problem hiding this comment.
You can get rid of this hacky closure and use this directly since #1196 was merged.
There was a problem hiding this comment.
is this:
engine.beginTimer(num_ms_on + num_ms_off, 'this.flashOnceOn(false)');preferred over:
engine.beginTimer(num_ms_on + num_ms_off, this.flashOnceOn.bind(this, false));I get that I don't need to bind for this but I also have an argument I'd like to curry. Most examples I've seen use the string method but I feel like the latter is better practice.
There was a problem hiding this comment.
IMO the ability to pass strings instead of functions is an ugly hack that shouldn't have been implemented in the first place. I think this form would be best:
engine.beginTimer(num_ms_on + num_ms_off, function () {
this.flashOnceOn(false);
});
| if (flashCount > 1) { | ||
| // flashcount>0 , means temporary flash, first flash already done, | ||
| // so we don't need this part if flashcount=1 | ||
| // temporary timer. The end of this timer stops the permanent flashing | ||
|
|
||
| this.flashTimer2 = engine.beginTimer(flashCount * (num_ms_on + | ||
| num_ms_off) - num_ms_off, function() { | ||
| this.flashTimer2 = engine.beginTimer(flashCount * (num_ms_on + num_ms_off) - num_ms_off, function() { | ||
| myself.Stopflash(relight); |
| myself.flashOnceOff(relight); | ||
| }, true); | ||
| this.flashOnceTimer = engine.beginTimer(this.num_ms_on - scriptpause, function() { | ||
| myself.flashOnceOff(relight); |
| }, true); | ||
| if (this.ButtonTimer === 0) { // first press | ||
| this.ButtonTimer = engine.beginTimer(this.DoublePressTimeOut, function() { | ||
| myself.ButtonDecide(); |
There was a problem hiding this comment.
Similar to my question above.
engine.beginTimer(this.DoublePressTimeOut, 'this.ButtonDecide()');vs.
engine.beginTimer(this.DoublePressTimeOut, this.ButtonDecide);here I assume that bind isn't necessary at all since we're not going to pass any parameters.
| } else { | ||
| // Else play/pause | ||
| toggleValue("[Channel" + decknum + "]", "play"); | ||
| // else play/pause |
There was a problem hiding this comment.
Likewise, no explanation needed.
| if (!deck.TrackIsLoaded()) { | ||
| // If a track is not loaded, load the selected track (if any) and play | ||
| engine.setValue("[Channel" + decknum + "]", "LoadSelectedTrackAndPlay", true); | ||
| // if a track is not loaded, load the selected track (if any) and play |
There was a problem hiding this comment.
I think this code is self explanatory and doesn't need a comment.
| } else if (deck.shiftKey && value === DOWN) { | ||
| // load next effect and make sure the unit is enabled | ||
| engine.setValue(effectGroup, "next_effect", true); | ||
| engine.setValue("[EffectRack1_EffectUnit" + decknum + "]", "group_[Channel" + decknum + "]_enable", true); |
There was a problem hiding this comment.
I still think this line should be removed. It may have made sense with the old mapping, but I don't think it makes sense to couple it to switching one effect in the chain.
| for (i = 1; i <= 4; i++) { | ||
| var deck = NumarkMixtrack3.deckFromGroup(group); | ||
| // beat knobs sends 1 or 127 as value. If value = 127, turn is counterclockwise | ||
| var increase = (value !== 127); |
There was a problem hiding this comment.
I think var increase = (value === 1); would be clearer.
| var decknum = script.deckFromGroup(group); | ||
| var deck = NumarkMixtrack3.decks["D" + decknum]; | ||
| if (deck.loaded && TrackEndWarning) { | ||
| var timeremaining = duration * (1 - value); |
There was a problem hiding this comment.
inconsistent capitalization of variable name
| }, true); | ||
|
|
||
| this.ButtonLongPressTimer = engine.beginTimer( | ||
| this.LongPressThreshold, this.ButtonAssertLongPress, true |
There was a problem hiding this comment.
Oh, the ButtonAssertLongPress function refers to this, so maybe it wouldn't have worked. Anyway, looks good now.
There was a problem hiding this comment.
Yeah, just figured there's little point in wrapping it with a function if no arguments need to be passed.
| "hotCue3": 0x1d, | ||
| "hotCue4": 0x1e, | ||
| "Cue": 0x03, | ||
| "cue": 0x03, |
There was a problem hiding this comment.
FYI, if you forget something in a commit and haven't added any new commits yet, git commit --amend is an easy way to overwrite the incomplete commit. It's no big deal you didn't do that here, but something to keep in mind if you are in that situation again.
|
Alright this looks good to me! 👍 Thank you! I'm glad one of the best selling controllers on the market right now will have a good mapping for the new effects UI for the 2.1 release. :) After #1187 is merged you may want to reconsider how loops are mapped, but I've been careful to make sure that old mappings will still work with the new looping interface. |
|
If you'd like to play with those before they're merged, you can checkout PRs locally and build them. I put some tips on the wiki for using git worktree to play with experimental skins and mappings. |
Conflict with InstantFX
120 goes into red way too easily. 82 was juuuust right.
|
Good to go from my end with these minor fixes. Effect units definitely need to be reworked though, will try and see if I can get a build going as you described. Thanks for review and hand-holding! |
|
Could someone merge this? I do not know if you are on the mailing list, but I just wrote a proposal for registering MIDI inputs from JS. Feedback would be appreciated. |
|
@daschuer can you merge this? |
|
Yo! Thank your for the work. |
|
@radusuciu, now that #1187 and #940 have been merged, could you update this for the new looping and beatjumping Controls? I suggest remapping the autoloop pad layer similar to the Pioneer DDJ-SB2. Users have been reporting that mapping pads that way is a little strange at first but great after getting used to it. Unlike the DDJ-SB2, because there are other parts of the controller that could be used for beatjumping, I think it would make more sense to use beatjump_1_backward/forward as the shift action for loop halve/double. You could use shift + pitch bend buttons to halve/double beatjump_size and shift + Beats encoder for beatjump_backward/forward. Also, when I played with a DDJ-SB2, I found it was more helpful to have quick access to changing the focused effect than toggling the effect enable switches. I suggest switching these around in the Mixtrack Pro 3 mapping, so pressing the effect buttons changes the focused effect and Tap + effect button toggles the effect enable switches. |
|
Yep, I will update it to make use of the new controls and will consider your mapping suggestions. I've also done a bit of playing with your components lib so can maybe bring in |
|
The EffectUnit object from the Components library is made for controllers with 4 effects knobs, so adapting it for the Mixtrack Pro 3 would probably be just as much (or maybe more) work than writing the code from scratch. However, using the Components and ComponentContainer objects to write an EffectUnit object specific to this controller would make sense if you'd like to rewrite the code you already have. I don't recommend copying how the effects buttons work with the DDJ-SB2 in #1243. I used short versus long button presses to distinguish between switching focus and enabling effects, which is a bit clumsy because the script has to wait for the button to be released before switching the focus. There also isn't a way to show which effects are enabled on the controller. These don't need to be issues with the Mixtrack Pro 3 because the Tap button can be used to switch layers. |
This is a WIP, not quite ready for review since I haven't had time to actually test it. Opening pull request a bit early just to move discussion from #1014. First time contributor so please let me know if I'm going against the grain!
To-do:
focused_effect