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.

Sign up for GitHub

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

Option to keep deck playing on track load #10944

Merged
merged 7 commits into from
Oct 19, 2022
Merged

Option to keep deck playing on track load #10944

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
87c5582
Option to keep deck playing on track load (closes #10548)
mbacarella Oct 6, 2022
1c79403
Catch up to main (resolve merge conflict)
mbacarella Oct 6, 2022
68ecf10
Tabstops: remove old item from tabstop, add comboBoxLoadWhenDeckPlaying
mbacarella Oct 6, 2022
fbff36e
Incorporate code review feedback
mbacarella Oct 11, 2022
7b85652
Make AllowTrackLoadToPlayingDeck a const ConfigKey
mbacarella Oct 17, 2022
59287c8
Run git-clang-format
mbacarella Oct 17, 2022
ed7001e
Actually try using the pre-commit.patch this time
mbacarella Oct 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/mixer/playermanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,26 @@ void PlayerManager::slotLoadLocationToPlayer(
emit loadLocationToPlayer(location, group, play);
}

void PlayerManager::slotLoadLocationToPlayerMaybePlay(const QString& location, const QString& group) {
bool play = false;
LoadWhenDeckPlaying loadWhenDeckPlaying =
m_pConfig->exists(kConfigKeyLoadWhenDeckPlaying)
? static_cast<LoadWhenDeckPlaying>(m_pConfig->getValueString(kConfigKeyLoadWhenDeckPlaying).toInt())
: kDefaultLoadWhenDeckPlaying;
switch (loadWhenDeckPlaying) {
case LoadWhenDeckPlaying::AllowButStopDeck:
case LoadWhenDeckPlaying::Reject:
break;
case LoadWhenDeckPlaying::Allow:
if (ControlObject::get(ConfigKey(group, "play")) > 0.0) {
// deck is currently playing, so immediately play new track
play = true;
}
break;
}
slotLoadLocationToPlayer(location, group, play);
}

void PlayerManager::slotLoadToDeck(const QString& location, int deck) {
slotLoadLocationToPlayer(location, groupForDeck(deck - 1));
}
Expand Down
5 changes: 2 additions & 3 deletions src/mixer/playermanager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,8 @@ class PlayerManager : public QObject, public PlayerManagerInterface {
// Slots for loading tracks into a Player, which is either a Sampler or a Deck
void slotLoadTrackToPlayer(TrackPointer pTrack, const QString& group, bool play = false);
void slotLoadLocationToPlayer(const QString& location, const QString& group, bool play = false);
void slotLoadLocationToPlayerStopped(const QString& location, const QString& group) {
slotLoadLocationToPlayer(location, group, false);
};
void slotLoadLocationToPlayerMaybePlay(const QString& location, const QString& group);

void slotCloneDeck(const QString& source_group, const QString& target_group);

// Slots for loading tracks to decks
Expand Down
53 changes: 31 additions & 22 deletions src/preferences/dialog/dlgprefdeck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,6 @@ DlgPrefDeck::DlgPrefDeck(QWidget* parent,
comboBoxTimeFormat->setCurrentIndex(
comboBoxTimeFormat->findData(time_format));

// Override Playing Track on Track Load
// The check box reflects the opposite of the config value
m_bDisallowTrackLoadToPlayingDeck = !m_pConfig->getValue(
ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"), false);
checkBoxDisallowLoadToPlayingDeck->setChecked(m_bDisallowTrackLoadToPlayingDeck);
connect(checkBoxDisallowLoadToPlayingDeck,
&QCheckBox::toggled,
this,
&DlgPrefDeck::slotDisallowTrackLoadToPlayingDeckCheckbox);

comboBoxLoadPoint->addItem(tr("Intro start"), static_cast<int>(SeekOnLoadMode::IntroStart));
comboBoxLoadPoint->addItem(tr("Main cue"), static_cast<int>(SeekOnLoadMode::MainCue));
comboBoxLoadPoint->addItem(tr("First sound (skip silence)"), static_cast<int>(SeekOnLoadMode::FirstSound));
Expand All @@ -178,6 +168,27 @@ DlgPrefDeck::DlgPrefDeck(QWidget* parent,
this,
&DlgPrefDeck::slotSetTrackLoadMode);

comboBoxLoadWhenDeckPlaying->addItem(tr("Allow"), static_cast<int>(LoadWhenDeckPlaying::Allow));
comboBoxLoadWhenDeckPlaying->addItem(tr("Allow, but stop deck"), static_cast<int>(LoadWhenDeckPlaying::AllowButStopDeck));
comboBoxLoadWhenDeckPlaying->addItem(tr("Reject"), static_cast<int>(LoadWhenDeckPlaying::Reject));
int loadWhenDeckPlaying;
if (m_pConfig->exists(kConfigKeyLoadWhenDeckPlaying)) {
loadWhenDeckPlaying = m_pConfig->getValueString(kConfigKeyLoadWhenDeckPlaying).toInt();
Copy link
Member

@daschuer daschuer Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need to use the default as well, if the value does not exists?
It would be grate to have a solution that works forward and backward with old an new versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't follow. If it doesn't exist, won't it then get the default from the else-else?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, right.
It is just the issue that "m_pConfig->exists" is a redundant lookup of the value. It is better to use getValue() with a fallback value like purposed above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well yes but I'm trying to be mindful of not losing behavior previously configured by the user in a previous version. If the user disabled (or enabled) playing track protection I want to import that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize this is confusing because, in a different part of the code, I didn't use getValue with default when I should have, but I think in this case it makes sense to probe for existence of the new config variable first and fall back to loading the older config variable.

} else {
// upgrade from older versions
if (m_pConfig->getValue(ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"), false)) {
loadWhenDeckPlaying = static_cast<int>(LoadWhenDeckPlaying::Allow);
} else {
loadWhenDeckPlaying = static_cast<int>(kDefaultLoadWhenDeckPlaying);
}
}
comboBoxLoadWhenDeckPlaying->setCurrentIndex(comboBoxLoadWhenDeckPlaying->findData(loadWhenDeckPlaying));
m_loadWhenDeckPlaying = static_cast<LoadWhenDeckPlaying>(loadWhenDeckPlaying);
connect(comboBoxLoadWhenDeckPlaying,
QOverload<int>::of(&QComboBox::currentIndexChanged),
this,
&DlgPrefDeck::slotSetLoadWhenDeckPlaying);

// This option was introduced in Mixxx 2.3 with the intro & outro cues.
// If the user has set main cue points with the intention of starting tracks
// from those points, enable this option. With Denon and Numark CueModes,
Expand Down Expand Up @@ -432,9 +443,6 @@ void DlgPrefDeck::slotUpdate() {

slotSetTrackTimeDisplay(m_pControlTrackTimeDisplay->get());

checkBoxDisallowLoadToPlayingDeck->setChecked(!m_pConfig->getValue(
ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"), false));

checkBoxCloneDeckOnLoadDoubleTap->setChecked(m_pConfig->getValue(
ConfigKey("[Controls]", "CloneDeckOnLoadDoubleTap"), true));

Expand Down Expand Up @@ -511,14 +519,15 @@ void DlgPrefDeck::slotResetToDefaults() {
// 8% Rate Range
ComboBoxRateRange->setCurrentIndex(ComboBoxRateRange->findData(kDefaultRateRangePercent));

// Don't load tracks into playing decks.
checkBoxDisallowLoadToPlayingDeck->setChecked(true);

// Clone decks by double-tapping Load button.
checkBoxCloneDeckOnLoadDoubleTap->setChecked(kDefaultCloneDeckOnLoad);

// Mixxx cue mode
ComboBoxCueMode->setCurrentIndex(0);

// What to do if someone loads into a playing deck
comboBoxLoadWhenDeckPlaying->setCurrentIndex(static_cast<int>(kDefaultLoadWhenDeckPlaying));

// Load at intro start
comboBoxLoadPoint->setCurrentIndex(
comboBoxLoadPoint->findData(static_cast<int>(SeekOnLoadMode::IntroStart)));
Expand Down Expand Up @@ -594,10 +603,6 @@ void DlgPrefDeck::slotKeyUnlockModeSelected(QAbstractButton* pressedButton) {
}
}

void DlgPrefDeck::slotDisallowTrackLoadToPlayingDeckCheckbox(bool checked) {
m_bDisallowTrackLoadToPlayingDeck = checked;
}

void DlgPrefDeck::slotCueModeCombobox(int index) {
m_cueMode = static_cast<CueMode>(ComboBoxCueMode->itemData(index).toInt());
}
Expand Down Expand Up @@ -668,6 +673,11 @@ void DlgPrefDeck::slotSetTrackLoadMode(int comboboxIndex) {
comboBoxLoadPoint->itemData(comboboxIndex).toInt());
}

void DlgPrefDeck::slotSetLoadWhenDeckPlaying(int comboboxIndex) {
m_loadWhenDeckPlaying = static_cast<LoadWhenDeckPlaying>(
comboBoxLoadWhenDeckPlaying->itemData(comboboxIndex).toInt());
}

void DlgPrefDeck::slotApply() {
m_pConfig->set(ConfigKey("[Controls]", "SetIntroStartAtMainCue"),
ConfigValue(m_bSetIntroStartAtMainCue));
Expand All @@ -687,8 +697,7 @@ void DlgPrefDeck::slotApply() {
}
m_pConfig->setValue(ConfigKey("[Controls]", "CueDefault"), static_cast<int>(m_cueMode));

m_pConfig->setValue(ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"),
!m_bDisallowTrackLoadToPlayingDeck);
m_pConfig->setValue(kConfigKeyLoadWhenDeckPlaying, static_cast<int>(m_loadWhenDeckPlaying));

m_pConfig->setValue(ConfigKey("[Controls]", "CueRecall"), static_cast<int>(m_seekOnLoadMode));
m_pConfig->setValue(ConfigKey("[Controls]", "CloneDeckOnLoadDoubleTap"),
Expand Down
15 changes: 13 additions & 2 deletions src/preferences/dialog/dlgprefdeck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,17 @@ enum class KeyunlockMode {
KeepLockedKey
};

enum class LoadWhenDeckPlaying {
Reject,
Allow,
AllowButStopDeck
};

namespace {
const ConfigKey kConfigKeyLoadWhenDeckPlaying = ConfigKey("[Controls]", "LoadWhenDeckPlaying");
constexpr LoadWhenDeckPlaying kDefaultLoadWhenDeckPlaying = LoadWhenDeckPlaying::Reject;
}

class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg {
Q_OBJECT
public:
Expand All @@ -68,9 +79,9 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg {
void slotRatePermFineSpinbox(double);
void slotSetTrackTimeDisplay(QAbstractButton*);
void slotSetTrackTimeDisplay(double);
void slotDisallowTrackLoadToPlayingDeckCheckbox(bool);
void slotCueModeCombobox(int);
void slotSetTrackLoadMode(int comboboxIndex);
void slotSetLoadWhenDeckPlaying(int comboboxIndex);
void slotCloneDeckOnLoadDoubleTapCheckbox(bool);
void slotRateRampingModeLinearButton(bool);
void slotRateRampSensitivitySlider(int);
Expand Down Expand Up @@ -116,7 +127,6 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg {
CueMode m_cueMode;

bool m_bSetIntroStartAtMainCue;
bool m_bDisallowTrackLoadToPlayingDeck;
bool m_bCloneDeckOnLoadDoubleTap;

int m_iRateRangePercent;
Expand All @@ -127,6 +137,7 @@ class DlgPrefDeck : public DlgPreferencePage, public Ui::DlgPrefDeckDlg {
KeylockMode m_keylockMode;
KeyunlockMode m_keyunlockMode;
SeekOnLoadMode m_seekOnLoadMode;
LoadWhenDeckPlaying m_loadWhenDeckPlaying;

RateControl::RampMode m_bRateRamping;
int m_iRateRampSensitivity;
Expand Down
14 changes: 5 additions & 9 deletions src/preferences/dialog/dlgprefdeckdlg.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@
<number>10</number>
</property>
<item row="5" column="0">
<widget class="QLabel" name="labelPlayingTrackProtection">
<widget class="QLabel" name="labelLoadWhenDeckPlaying">
<property name="text">
<string>Playing track protection</string>
<string>Loading a track, when deck is playing</string>
</property>
<property name="buddy">
<cstring>checkBoxDisallowLoadToPlayingDeck</cstring>
<cstring>comboBoxLoadWhenDeckPlaying</cstring>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why you changed this (changing translation texts requires new translation for every language).

Also, I thought the new option would a) load a track into a playing deck and b) play that right away.
Even though b) requires a), a) and b) are unrelated (I might want to load tracks into playing decks, but I don not want auto-play)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually deleting the checkbox entirely and replacing it with a combobox that offers more functionality:

  1. load track into playing deck and play that right away
  2. load track into playing deck but stop
  3. don't allow loading into playing deck

See the discussion at the end of this issue? #10548 (comment)

Copy link
Member

@ronso0 ronso0 Oct 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Of course, I should have taken a closer look, not just skimming through...

</property>
</widget>
</item>
Expand Down Expand Up @@ -145,11 +145,7 @@ CUP mode:
</widget>
</item>
<item row="5" column="1" colspan="2">
<widget class="QCheckBox" name="checkBoxDisallowLoadToPlayingDeck">
<property name="text">
<string>Do not load tracks into playing decks</string>
</property>
</widget>
<widget class="QComboBox" name="comboBoxLoadWhenDeckPlaying"/>
</item>
<item row="2" column="1" colspan="2">
<layout class="QHBoxLayout" name="horizontalLayout">
Expand Down Expand Up @@ -661,7 +657,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
<tabstop>radioButtonElapsedAndRemaining</tabstop>
<tabstop>comboBoxTimeFormat</tabstop>
<tabstop>comboBoxLoadPoint</tabstop>
<tabstop>checkBoxDisallowLoadToPlayingDeck</tabstop>
<tabstop>comboBoxLoadWhenDeckPlaying</tabstop>
<tabstop>checkBoxCloneDeckOnLoadDoubleTap</tabstop>
<tabstop>ComboBoxRateRange</tabstop>
<tabstop>checkBoxInvertSpeedSlider</tabstop>
Expand Down
14 changes: 7 additions & 7 deletions src/skin/legacy/legacyskinparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ QWidget* LegacySkinParser::parseOverview(const QDomElement& node) {
connect(overviewWidget,
&WOverview::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(overviewWidget, &WOverview::cloneDeck,
m_pPlayerManager, &PlayerManager::slotCloneDeck);

Expand Down Expand Up @@ -1031,7 +1031,7 @@ QWidget* LegacySkinParser::parseVisual(const QDomElement& node) {
connect(viewer,
&WWaveformViewer::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(viewer, &WWaveformViewer::cloneDeck,
m_pPlayerManager, &PlayerManager::slotCloneDeck);

Expand Down Expand Up @@ -1061,7 +1061,7 @@ QWidget* LegacySkinParser::parseText(const QDomElement& node) {
connect(pTrackText,
&WTrackText::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(pTrackText, &WTrackText::cloneDeck, m_pPlayerManager, &PlayerManager::slotCloneDeck);

TrackPointer pTrack = pPlayer->getLoadedTrack();
Expand Down Expand Up @@ -1098,7 +1098,7 @@ QWidget* LegacySkinParser::parseTrackProperty(const QDomElement& node) {
connect(pTrackProperty,
&WTrackProperty::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(pTrackProperty,
&WTrackProperty::cloneDeck,
m_pPlayerManager,
Expand Down Expand Up @@ -1141,7 +1141,7 @@ QWidget* LegacySkinParser::parseTrackWidgetGroup(const QDomElement& node) {
connect(pGroup,
&WTrackWidgetGroup::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(pGroup,
&WTrackWidgetGroup::cloneDeck,
m_pPlayerManager,
Expand Down Expand Up @@ -1317,7 +1317,7 @@ QWidget* LegacySkinParser::parseSpinny(const QDomElement& node) {
connect(pSpinny,
&WSpinny::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(pSpinny, &WSpinny::cloneDeck, m_pPlayerManager, &PlayerManager::slotCloneDeck);

ControlObject* showCoverControl = controlFromConfigNode(node.toElement(), "ShowCoverControl");
Expand Down Expand Up @@ -1434,7 +1434,7 @@ QWidget* LegacySkinParser::parseCoverArt(const QDomElement& node) {
connect(pCoverArt,
&WCoverArt::trackDropped,
m_pPlayerManager,
&PlayerManager::slotLoadLocationToPlayerStopped);
&PlayerManager::slotLoadLocationToPlayerMaybePlay);
connect(pCoverArt, &WCoverArt::cloneDeck,
m_pPlayerManager, &PlayerManager::slotCloneDeck);
}
Expand Down
23 changes: 19 additions & 4 deletions src/util/dnd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "library/parserm3u.h"
#include "library/parserpls.h"
#include "mixer/playermanager.h"
#include "preferences/dialog/dlgprefdeck.h"
#include "sources/soundsourceproxy.h"
#include "track/track.h"

Expand Down Expand Up @@ -80,10 +81,24 @@ bool allowLoadToPlayer(
return true;
}

return pConfig->getValueString(
ConfigKey("[Controls]",
"AllowTrackLoadToPlayingDeck"))
.toInt();
bool allowLoadTrackIntoPlayingDeck = false;
if (pConfig->exists(kConfigKeyLoadWhenDeckPlaying)) {
int loadWhenDeckPlaying =
pConfig->getValueString(kConfigKeyLoadWhenDeckPlaying).toInt();
switch (static_cast<LoadWhenDeckPlaying>(loadWhenDeckPlaying)) {
case LoadWhenDeckPlaying::Allow:
case LoadWhenDeckPlaying::AllowButStopDeck:
allowLoadTrackIntoPlayingDeck = true;
break;
case LoadWhenDeckPlaying::Reject:
break;
}
} else {
// support older version of this flag
allowLoadTrackIntoPlayingDeck =
pConfig->getValue<bool>(ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"));
Copy link
Member

Choose a reason for hiding this comment

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

Can you also introduce a constant for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
return allowLoadTrackIntoPlayingDeck;
}

} // anonymous namespace
Expand Down
22 changes: 19 additions & 3 deletions src/widget/wtrackmenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "moc_wtrackmenu.cpp"
#include "preferences/colorpalettesettings.h"
#include "preferences/configobject.h"
#include "preferences/dialog/dlgprefdeck.h"
#include "sources/soundsourceproxy.h"
#include "track/track.h"
#include "util/defs.h"
Expand Down Expand Up @@ -773,9 +774,24 @@ void WTrackMenu::updateMenus() {
QString deckGroup = PlayerManager::groupForDeck(i - 1);
bool deckPlaying = ControlObject::get(
ConfigKey(deckGroup, "play")) > 0.0;
bool loadTrackIntoPlayingDeck = m_pConfig->getValue<bool>(
ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"));
bool deckEnabled = (!deckPlaying || loadTrackIntoPlayingDeck) && singleTrackSelected;
bool allowLoadTrackIntoPlayingDeck = false;
if (m_pConfig->exists(kConfigKeyLoadWhenDeckPlaying)) {
int loadWhenDeckPlaying =
m_pConfig->getValueString(kConfigKeyLoadWhenDeckPlaying).toInt();
switch (static_cast<LoadWhenDeckPlaying>(loadWhenDeckPlaying)) {
case LoadWhenDeckPlaying::Allow:
case LoadWhenDeckPlaying::AllowButStopDeck:
allowLoadTrackIntoPlayingDeck = true;
break;
case LoadWhenDeckPlaying::Reject:
break;
}
} else {
// support older version of this flag
allowLoadTrackIntoPlayingDeck = m_pConfig->getValue<bool>(
ConfigKey("[Controls]", "AllowTrackLoadToPlayingDeck"));
}
bool deckEnabled = (!deckPlaying || allowLoadTrackIntoPlayingDeck) && singleTrackSelected;
QAction* pAction = new QAction(tr("Deck %1").arg(i), this);
pAction->setEnabled(deckEnabled);
m_pDeckMenu->addAction(pAction);
Expand Down
Loading