Traktor Kontrol S3 script improvements, vanilla-like FX behavior, control initialization, better scratching, and more #11199
Conversation
It's trivial to change this back if you prefer the calmer orange colors for decks 1 and 2, but it seems like a good idea to stay as close to stock as possible.
I feel like by default it should just behave like everything else and releasing the jog wheel should immediately resume playback.
For the S3 controller mapping.
I don't see why anyone would want to use this with scratch2 mode.
Instead of the velocity from this manual velocity calculation.
Instead of the deck number.
Mixxx already does this internally.
This makes it so Mixxx's initial state matches the S3.
This makes sure no values can be lost.
This behaves as intended by Native Instruments. Only the input parts have been implemented, the LEDs still need to be done.
This makes more sense to me than keeping whatever was set last time.
This should now be pretty much done.
And don't highlight the unpressed ones. This doesn't look as cool but it makes it more obvious which ones are active.
This is the same as `loaded_chain_preset`, except that it preserves the value when changing presets. Bikeshedding over the name is very welcome.
With lexically scoped const/let bindings. The `var TraktorS3 = ...` needs to stay a var by design.
This would make it impossible to set the quick effect super knob to exactly 0.5. This is important for some effects like the Moog filter.
I liked the idea, but it quickly becomes confusing once you start assigning effects to channels.
This is the original S3 behavior and it's probably less confusing.
As the result of mixxxdj#10859 being merged. Everything shifted up one spot.
|
There's nothing that really requires it. It's just that without the option it doesn't behave the way you'd expect it to. And without #4651 some settings are not correctly restored from the device's state. |
|
On a side note, I noticed the other day that some of the LED state isn't synchronized when changing between layers. I noticed that the play/pause button blinking didn't match with the new channel just after switching, and there may be other controls that also don't match. I have not yet had the time to look into this. |
|
are those regressions, or is that the same as the current state? As long as the experience on the controller is strictly better with this PR, I'd like to get it in and keep iterating |
|
I haven't tested the current version but I also didn't touch that part of the code (other than transforming the prototypes to ES6 classes), so it shouldn't be a regression. |
|
yeah then I'd be ok pushing this in (along with the manual changes) and then try to get the other issues cleaned up for 2.4. Can you create Issues for the things you've noticed? that way they get tracked |
|
Yeah they're on my personal todo list. I'll get to them (and perhaps open PRs) after I finish a couple other things. |
| TraktorS3.incomingData([2, ...Array.from(report2Values.map(x => ~x))]); | ||
| TraktorS3.incomingData([2, ...Array.from(report2Values)]); |
There was a problem hiding this comment.
| TraktorS3.incomingData([2, ...Array.from(report2Values.map(x => ~x))]); | |
| TraktorS3.incomingData([2, ...Array.from(report2Values)]); | |
| TraktorS3.incomingData(new Uint8Array([2, ...Uint8Array.from(report2Values.map(x => ~x))])); | |
| TraktorS3.incomingData(new Uint8Array([2, ...Uint8Array.from(report2Values)])); |
When incomingData is called from C++, the argument is an array of type Uint8Array. The same type should be used here too.
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
|
Oh yeah, there's one other thing I was wondering about with this mapping. This line from the previous version of the script creates a link from the |
| // just tell the packet parser the current values so it won't ignore the | ||
| // next input. | ||
| const report1Values = new Uint8Array(controller.getInputReport(1)); | ||
| TraktorS3.incomingData([1, ...Array.from(report1Values)]); |
There was a problem hiding this comment.
| TraktorS3.incomingData([1, ...Array.from(report1Values)]); | |
| TraktorS3.incomingData(new Uint8Array([1, ...Uint8Array.from(report1Values)])); |
This should also be Uint8Array
There was a problem hiding this comment.
Fixed. The HID parser previously accepted both regular arrays and Uint8Array but after #4894 it only accepts Uint8Arrays.
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
93fa58a to
29bfd1d
Compare
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
|
thanks for working on the docs! |
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
|
As far as I can see the changes to the FX behavior block merging this PR since a while. Would it make sense to move th FX changes to a follow-up PR and merge the other improvements into 2.4? |
|
Without the option from #11198 this would still work. It could just lead to some slightly confusing behavior if you've ever used Pioneer, Denon, NI, or other DJ gear with channel FX selectors before. |
|
@ywwg What is needed to get this merged? |
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
As suggested in mixxxdj#11199 (comment). The function does accept both typed arrays and plain arrays.
|
Merge this without the super-knob default PR, because this is independent: #11198 (comment) |
Zullip thread
I've been using this for the last couple months and it's been a joy to use. This PR does depend on #11198 and #4651. The former exposes an option that should be enabled for the new vanilla FX behavior to work as intended, and latter is needed to be able to initialize the quick effect/filter knobs. Otherwise the effects are initialized after the control script has finished initializing and they'll be reset to their default values. This PR makes the following changes to the Traktor Kontrol S3 script:
0.4998778998778999to control object instead of 0.5. This would break certain features that explicitly check for 0.5 values, like the filters.TraktorS3object have been removed (lots of variables previously used leaked variable bindings), and a custom bind function has been replaced withfn.bind().The manual still needs to be updated to mention the new FX behavior. Should I just create a PR in the manual repo?