search

mixxxdj / mixxx

Mixxx is Free DJ software that gives you everything you need to perform live mixes.
http://mixxx.org
Other
4.52k stars 1.28k forks source link

JogWheelBasic component doesn't take into account deck switching #11867

Open miwig opened 1 year ago

miwig commented 1 year ago

Bug Description

As seen here the JogWheelBasic component doesn't take into account the selected deck/group when scratching.

I could try a PR if you want. I assume the fix would be to add special handling here similar to how it's done for EffectAssignmentButton.

Version

2.3.5

OS

Arch Linux btw

Swiftb0y commented 1 year ago

Hey there. Thank you for filing the issue. This is indeed an oversight. I'd be very happy if you could file a PR for that. I see two possible solutions:

  1. The special handling as you suggested
  2. instead of relying on the .deck, we instead extract the deck number from the .group on each call.

The second solution is more robust, but likely very much lower-performant... Not sure what solution is the most optimal one. What do you think?

miwig commented 1 year ago

instead of relying on the .deck, we instead extract the deck number from the .group on each call.

Couldn't JogWheelBasic override connect to extract the deck number from the group?

Swiftb0y commented 1 year ago

That... could work... Not sure how robust that is though.

Another option would to do the regex extraction on each call, but speed it up by memoizing the call each time.

miwig commented 1 year ago

That... could work... Not sure how robust that is though.

It seems more robust than the special case solution to me. The docs also suggest that overriding connect is allowed, but give a different reason for doing it.

Swiftb0y commented 1 year ago

Right, overwriting connect is completely reasonable and encouraged. I'm just not 100% confident that consumers will always call connect after changing the group. On the other hand, they should, otherwise that would probably result in bugs in all other components as well. So yeah, overwriting connect and extracting the deck from the group is probably the best solution. Can you open a PR with that?

miwig commented 1 year ago

I can probably get around to it this week, yeah.