Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make decks' xfader assignment persistent #12074

Merged
merged 1 commit into from Oct 16, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 7, 2023

Closes #10122

I agree with Be's comment:

I am leaning towards simply making the crossfader orientation switches persistent in all cases. This is how every 4 deck mixer and controller with crossfader assignment switches works. I think if a user wants to change these, they probably want that change to persist.

@ronso0 ronso0 force-pushed the xfader-orientation-persists branch from a3a7ebb to 787e069 Compare October 8, 2023 21:49
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you.
Can this become a 2.4 fix?

@ronso0
Copy link
Member Author

ronso0 commented Oct 9, 2023

Personally I don't care.
It's a 3yo feature request, which in some settings may be considered a bug, or rather inconvenience.
For users who always want/need certain samplers to play on the left/right channel bus (VC setup with 2-channel mixer) it's am annoyance.
As Be stated correctly IMO, users who change the xfader assignment for the main decks do this either in skins (and the skin & skin config is persistent across restarts so the xfader toggles are visible) or via controller switches (which also persist).
And we didn't document that the xfader switches are reset on each start.
So, even though it's a behaviour change I don't consider it a regression or significantly break use cases.

If I didn't overlook any aspects may indeed target 2.4

@Swiftb0y
Copy link
Member

Swiftb0y commented Oct 9, 2023

don't see any reason this couldn't go into 2.4.

One thought I had during the curse of the recent couple PRs that made more CO's persistent was that we should probably have a button somewhere in the preferences that reset all CO's to their defaults.
Imagine this: A user messed with some controller or with the CO tools that changed the audio routing or just some characteristics ("orientation, "volume", "gain", etc.) Their Skin doesn't over the same amount of controls and just assumed some of these CO's to be at their default (simplified skins might make this assumption for example). The result could be that the user would end up with Mixxx in a state they can't explain and also not easily diagnose or change. Previously, a restart would've fixed the issue since the CO would've been reset from that. Now the only way for them to fix their install would be to delete the file where the CO values are saved (likely by just blatantly nuking their settings directory as a whole). The entire process is quite unpleasant so there should be a more user friendly and easier to discover way.

Just a thought, this is likely an incredibly unlikely case and very much contrived. Still something I thought of (I think I remember being a situation like this in my early days where I started messing with mixxx internals).

@ronso0
Copy link
Member Author

ronso0 commented Oct 9, 2023

FWIW simplified skins should set inaccessible controls in the manifest, like Shade does with the samplers' effect routing switches:

<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler1]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler2]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler3]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler4]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler5]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler6]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler7]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Sampler8]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Microphone]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Microphone2]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Auxiliary1]_enable">0</attribute>
<attribute persist="false" config_key="[EffectRack1_EffectUnit1],group_[Auxiliary2]_enable">0</attribute>

We can't do anything about this if custom skins are used 🤷‍♂️

Though, a Total Reset™️ button would be handy indeed.

@ronso0 ronso0 changed the base branch from main to 2.4 October 15, 2023 23:07
@ronso0 ronso0 force-pushed the xfader-orientation-persists branch from 787e069 to 2fc87d4 Compare October 15, 2023 23:09
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thank you

@daschuer daschuer merged commit 2ce5793 into mixxxdj:2.4 Oct 16, 2023
13 checks passed
@ronso0 ronso0 deleted the xfader-orientation-persists branch October 16, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crossfader orientation isn't saved/restored
3 participants