Traktor S4MK3: Add alternative modes for a few controls.#15664
Traktor S4MK3: Add alternative modes for a few controls.#15664ywwg wants to merge 1 commit intomixxxdj:mainfrom
Conversation
|
Thank you for this! The relative tempo mode is indeed helpful, for me when I want to match another track's BPM which is > +- 8% but I don't want to change the rate range. Though there are a few issue with the current implementation:
How can we implement a 'relative mode' toggle? |
(see below)
what would you want them to do here?
I can clamp that for sure. And an upper bound just for safety seems useful too.
I'll think about this -- currently I think it's ok to keep it as a controller setting and not something accessible from the controller hardware. |
|
I guess shift+jog can work? But I think "shift+master long press" is available: https://github.com/mixxxdj/mixxx/blob/main/res/controllers/Traktor-Kontrol-S4-MK3.js#L1989-L1998 |
Actually I was missing the center indictaor, but that's d be kind of useless anyway because, without the (scripted) center zone, the center can actually not be reached. Will test the latest update soonish. |
|
I suppose we could light up the light when the pitch is zero, regardless of where the slider is. (That would show how far off from centered the slider is) |
|
Yeah, thing is, what I tried to explain: with the high-res faders I never managed to hit zero -- that's why I added the center zone #14735, and with relative mode we currently don't have this center zone. Is there a way to engage relative mode on request? And auto-quit?
(1) I suspect the crucial/tricky part would be to distinguish rate changes done by the engine (sync locked) and the physial fader Kind of related to #14985 |
is the goal here "I want to reset the pitch to zero"? I think that could be accomplished by moving the physical fader to the center, disabling relative mode and reengaging relative mode. Personally I would just right-click on the screen to do this rather than try to do it with the controller. If you can say more about the use-case for this it would help me understand. I don't quite understand what would be happening in a DJ set that you want to do. |
|
Right. First of all, I'm not requesting to expand the scope here. Relative mode is nice, even if it can only be toggled in the settings. My use case is beat matching to external sources. I usually try to match by ear or use a BPM tap tool. What (in my mind) would feel natural is moving the fader to the upper/lower limit, Shift-move it back to (somewhere near) the center and continue in relative mode. And later on return to absolute mode as described. |
acolombier
left a comment
There was a problem hiding this comment.
These new features are nice! 👍
Perhaps it would be worth documenting them as well.
| // For alternativeMoveMode | ||
| beatAlternative: 5, |
There was a problem hiding this comment.
Is this redundant? I am wondering if it would be best to handle the alternative status as part of the beat mode, rather than duplicating the mode entirely. The issue being, any new feature looking into the mode (e.g feedback on the active mode on screen) would require to handle both of these modes, while they are actually the same, just swapping the shifted/unshifted behaviour
(See below suggestion inside the main switch)
There was a problem hiding this comment.
I am being a little sneaky here -- using a component pattern to change behavior instead of logic switches. That makes it easier to debug and switch between behaviors because the behavior is self-contained. But doing it halfway like this does result in a lot of visible duplication. It gets shorter if the behavior (a beatjump size adjust) is assigned to a specific action in one line (unpressed + unshifted).
Note that all three branches of logic change with the Alt version -- so it would get pretty messy indeed to interleave the mode conditional with the shift conditional with the encoder-press conditional and then with the direction conditional.
I can mash them together but I think it's going to look pretty bad all spread out -- I think it would be better to create little behavior functions to clean it up.
| } | ||
| this.secondDeckModes = null; | ||
| this.selectedHotcue = null; | ||
| this.relativeTempoMode = RelativeTempoMode; |
There was a problem hiding this comment.
Is this now redundant with Shift+ long press Master? Or would we want to have a DefaultRelativeTempoMode option?
There was a problem hiding this comment.
Yes there's a config setting to make relative the default mode (line 207), and this loads the default.
| engine.setValue(this.deck.group, `hotcue_${this.deck.selectedHotcue}_color`, Object.keys(LedColorMap)[currentColorIdx]); | ||
| break; | ||
| } | ||
| case moveModes.beatAlternative: |
There was a problem hiding this comment.
Aligned with my comment above, creating a new mode feels suboptimal. Here you could factorise the beat mode case (currently handled in default) as such, so you don't need to duplicate the branch.
(Note that I am suggesting AlternativeBeatMoveMode instead of AlternativeMoveMode since this last one is confusing, as none of the other move mode's behaviour are changing)
default:
if (!this.shifted) {
if (!this.deck.leftEncoderPress.pressed && !AlternativeBeatMoveMode) {
script.triggerControl(this.group, right ? "beatjump_forward" : "beatjump_backward");
} else if (this.deck.leftEncoderPress.pressed && AlternativeBeatMoveMode) {
script.triggerControl(this.group, right ? "pitch_up_small" : "pitch_down_small");
} else {
let beatjumpSize = engine.getValue(this.group, "beatjump_size");
engine.setValue(this.group, "beatjump_size", beatjumpSize * (right ? 2 : 1/2));
}
}
else if (AlternativeBeatMoveMode) {
script.triggerControl(this.group, right ? "beatjump_forward" : "beatjump_backward");
} else {
script.triggerControl(this.group, right ? "pitch_up_small" : "pitch_down_small");
}
break;
}There was a problem hiding this comment.
I find this really hard to read and follow the logic, especially with all the inversions ("NOT shifted and NOT pressed..."), but I agree the duplicated version is not great either. (I am not even sure what set of conditionals trigger the bare else! uhhhh "encoderPressed NOT Alternativebeatmodemode"?)
There was a problem hiding this comment.
I'll play with it and see what I can come up with, getting rid of the new mode
|
I think this is a decent balance between ease of reading and duplication. I can now read through the branches and know what will happen with each operation. The one-liners for action help, good suggestion. |
c000bb4 to
270372e
Compare
acolombier
left a comment
There was a problem hiding this comment.
Code's looking good, I'll try to take that for another test during the week end, except if @ronso0 wants to press merge before?
Feel free to prepare the manual PR as well
|
Yes, I may have some time with my S4 before the weekend so could do a test -- no guarantee though |
|
manual: mixxxdj/manual#819 |
acolombier
left a comment
There was a problem hiding this comment.
Tested and works great! I actually like the relative tempo mode and will likely use this feature often!
I just pushed a small fixup to use the tempo led to display feedback on the current active tempo mode: f94e82c
|
if it's ok by you then you are good to merge the two PRs 😎 |
|
Are you happy with getting the above fixup in? Happy to push it to your branch and merge if that's fine by you! |
Relative Tempo Fader mode: Tempo fader only responds to changes, not absolute position. Use shift to change position without adjusting rate. Relative Tempo Fader, Shifted mode: Only respond to changes when shift is held. Prevents accidental bumps when DJing in poor tactile environments (wearing fursuit paws) Jog Wheel Adjustment factor: Adjust sensitivity of jog wheel when making sync adjustments. Alternative Mode mode: for the move encoder, change assignments: unshifted to change beatjump size, shift to do a beatjump, push and change to adjust pitch. Signed-off-by: Owen Williams <owilliams@mixxx.org>
001fe38 to
763666f
Compare
|
squashed and pushed |
acolombier
left a comment
There was a problem hiding this comment.
I just had the opportunity to take this mapping for a spin and unfortunately, I encounter a few bug, I couldn't spot yet, which are:
- Key reset doesn't work and seem to jump back to a very low key instead (0?)
- when jumping between relative and absolute, the tempo led seem to get stuck in takeover low color
I'll see if I can find them and suggest a fixup if time allows it.
|
how are you doing key reset? |
|
Shift+left encoder press. However, this seems to set the key to |
|
|
||
| const TempoFaderSoftTakeoverColorLow = LedColors[engine.getSetting("tempoFaderSoftTakeoverColorLow")] || LedColors.white; | ||
| const TempoFaderSoftTakeoverColorHigh = LedColors[engine.getSetting("tempoFaderSoftTakeoverColorHigh")] || LedColors.green; | ||
| const TempoFaderRelativeMove = LedColors[engine.getSetting("tempoFaderRelativeMove")] || LedColors.blue; |
There was a problem hiding this comment.
The var name should contain 'color' IMO
| const TempoFaderRelativeMove = LedColors[engine.getSetting("tempoFaderRelativeMove")] || LedColors.blue; | |
| const TempoFaderRelativeModeColor = LedColors[engine.getSetting("tempoFaderRelativeModeColor")] || LedColors.blue; |
|
FWIW I didn't really get used to relative tempo mode. (tested it once during a session, ported to my mapping) Some time ago I had Wdyt? |
acolombier
left a comment
There was a problem hiding this comment.
I re-tested this mapping to try and find this issue with key reset, but couldn't reproduce and had everything working again.
Happy to merge as is and fix forward!
I think this ultra slider is a good idea, and I would support merging it, but personally the relative mode is much more useful to me so I will use that instead. for me it's not about going beyond the bounds of the slider (and it's trivial to shift+move if I hit the edge), it's totally avoiding soft-takeover, which I do not personally like. I would say it makes sense to move all of these modes into mixxx instead of individually implementing them in controllers. |
|
sorry again for the lack of work on this, I am still waiting for my new laptop and I cannot do any mixxx work until it gets here. |
|
yay new laptop |
Relative Tempo Fader mode: Tempo fader only responds to changes, not absolute position. Use shift to change position without adjusting rate. (great for sync lock mode)
Relative Tempo Fader, Shifted mode: Only respond to changes when shift is held. Prevents accidental bumps when DJing in poor tactile environments (wearing fursuit paws)
Jog Wheel Adjustment factor: Adjust sensitivity of jog wheel when making sync adjustments. (current default is very sensitive)
Alternative Move mode: for the move encoder, change assignments: unshifted to change beatjump size, shift to do a beatjump, push and change to adjust pitch. (another case where I don't want to accidentally bump the control and jump beats)