AutoDJ crossfader reset option #13156
Conversation
| @@ -39,6 +39,9 @@ DlgPrefAutoDJ::DlgPrefAutoDJ(QWidget* pParent, | |||
| // Auto DJ random enqueue | |||
| RandomQueueCheckBox->setChecked(m_pConfig->getValue( | |||
| ConfigKey("[Auto DJ]", "EnableRandomQueue"), false)); | |||
| // Auto DJ XFaderCenterResetState | |||
| XFaderCenterResetStateCheckBox->setChecked(m_pConfig->getValue( | |||
| ConfigKey("[Auto DJ]", "XFaderCenterResetState"), true)); | |||
There was a problem hiding this comment.
Let's name it
CenterXFaderWhenDisabling
and the default should be false
| m_pConfig->setValue(ConfigKey("[Auto DJ]", "XFaderCenterResetState"), | ||
| enable); |
There was a problem hiding this comment.
This basically circumvents slotApply() and applies the selection right away.
All this "buffering" to the config is not good pratice as it unnnecessarily increases complexity and pollutes the config file with temp values. This page hasn't been touched for years. So essentially, if you want start working on (and learning about) the preferences, the AutoDJ page is not a good example.
IMO we should not built upon this, but instead seize the opportunity to refactor this page.
We can either
-
- stick with these checkbox / spinbox slots and store the temp values in member variables
- use the temp values in slotApply()
- (preferred IMO) simply read all checkboxes and spinboxes in slotApply() and write to the config
Do you want to work on this refactoring?
There was a problem hiding this comment.
I indeed tried to imitate the given code as much as possible. With a lot of trial and error.
I saw the not cleaned buff-occurences in the cfg-file and searched for a method avoiding those extra lines,
I struggled myself through the code :-)
I don't have a clue how old certain files are, but I can imagine there were different people adding their own part (like I did) puzzling out how it could be done, finding a way and making it all more complex.
I compaired other dialog settings files, and the structure is each time different, there is no consistency (not even about checkbox in front or after the text), making it hard for newbies.
I think it started with making a spinbox depending on a checkbox and ended with a lot of buffers and rules.
I am honoured with your question and I would like to improve this part. I just can't give a timeline, my students are studying these days, next week I start taking exams, so the spare time willl be less.
There was a problem hiding this comment.
Sure, I understand, no need to excuse, I think that's how most contributors start.
If you decide to do the refactoring, please start over with a new branch. Or reorder commits and force-puhs, as you like.
| <string>Reset the Crossfader back to center after disabling AutoDJ</string> | ||
| </property> | ||
| </widget> | ||
| </item> |
There was a problem hiding this comment.
I think we definitely have to add a note here explaining the consequences (main gain drop), see #10683
AutoDJ, Added Settings to reset XFader to center after disabling Auto Dj