MC7000: Fix 'inverted shift' bug in the controller mapping #4755
Conversation
|
Awesome finding! Happy to see you are working to improve the mapping. (I upgraded last year to SC6000M+X1850 so no longer working with the MC7000). |
fwcd
force-pushed
the
fix-mc7000-shift-inversion-bug
branch
from
ee31155
to
e7d48f3
Compare
fwcd
force-pushed
the
fix-mc7000-shift-inversion-bug
branch
from
e7d48f3
to
08c0641
Compare
|
@fwcd |
Please always consider opening a pull request to include these important changes in Mixxx. Changes posted somewhere in the forums will get lost. The current, official version of the files here in the repo is always the reference. I didn't check the differences between the independently developed versions. You who own this controller are also the only ones who are able to verify if they work as expected. It would be great if you could coordinate with each other to finally get all those improvements and fixes into the official version! Edit: I suppose to do this incrementally, bug fixes separately from new features. New features and changes in behavior must also be documented in the manual. |
|
To be fair, there does seem to be a PR updating the mapping (#4729). The problem is that I'm currently the only one reviewing mappings and there is a giant backlog of other mappings that also need reviewing. Many of these just get lost and thus end up taking ages to get reviewed. |
|
Yes, PR #4729 is identical to the version I uploaded to the forum. And I will happily update the manual once the changes have been merged. |
This should make the mapping more robust against e.g. the 'shift inversion' bug that happened when switching decks while holding shift since the shift state should at least converge to the correct state after pressing it again.
This is needed to handle the deck switching correctly (since we only want to transfer shift state if the deck was actually switched, not if the same deck's button has been pressed, since the latter would yield us the same shift inversion bug again).
fwcd
force-pushed
the
fix-mc7000-shift-inversion-bug
branch
from
08c0641
to
ffdaa4e
Compare
|
I have updated the branch with the suggestions and rebased it on the new 2.3 branch, so it should be ready. |
Based on #4757
The controller mapping for the MC7000 has a bug where the controller keeps the wrong shift state while switching decks. For example, consider the following flow:
Since the MC7000 will emit only a press event on deck 1 and a release event on deck 3, the mapping's shift array (
MC7000.shift) will incorrectly end up in the state where both decks are assumed to be currently shift-held.This PR fixes the issue by transferring the shift state when switching decks. For this, the deck switching buttons (1, 3 and 2, 4) are mapped in the MIDI XML file and associated with a new function
MC7000.switchDeck.To cleanly track transitions between the top and the bottom deck on each side (which we need, since holding shift while pressing the active deck's button again would result in the same bug), we also declare a new property
MC7000.topDeckActive, indexed by the side of the controller we are dealing with (0 for left, 1 for right). Also, the active decks on the controller are now synced withtopDeckActive, so as a side-effect this means that decks 1 and 2 are automatically switched to whenever the controller mapping is initialized (is that fine?).In addition to the changes described above, we also update the implementation of
MC7000.shiftButtonand read the shift state directly withvalue > 0rather than simply negating the existing state. This makes the shift implementation less brittle against bugs like these in the future, since the shift state will always converge to the correct state once the user presses the button again.cc @toszlanyi