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

Effects refactoring: loaded chain #4431

Merged
merged 10 commits into from
Oct 18, 2021
Merged

Effects refactoring: loaded chain #4431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8d8074f
Rename [EffectRack1_EffectUnit1],loaded_preset to [EffectRack1_Effect…
daschuer Oct 15, 2021
2f986c4
Make next/prev_chain variables matching the corresponding CO names
daschuer Oct 15, 2021
77cac9d
Ignore 0 in EffectChain::slotControlChainSelector to match the behavi…
daschuer Oct 15, 2021
2b96252
Simplify control flow in loadChainPreset
daschuer Oct 15, 2021
a3902f4
Remove redundant setting of m_pControlLoadedChainPreset
daschuer Oct 15, 2021
e3322ce
Added a TODO for the unused [EffectRack1_EffectUnit1],loaded
daschuer Oct 15, 2021
e5fda81
Remove pointless check if group is empty.
daschuer Oct 15, 2021
f175f9e
Rename num_presetsavalable to num_chain_presets for a consistent nami…
daschuer Oct 15, 2021
3883a28
rename loaded_chain to loaded_chain_preset
daschuer Oct 18, 2021
9cb902e
Remove unused [EffectRackn_EffectUnitn],loaded CO
daschuer Oct 18, 2021
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
2 changes: 1 addition & 1 deletion src/effects/chains/quickeffectchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ QuickEffectChain::QuickEffectChain(const QString& group,
&EffectChainPresetManager::effectChainPresetListUpdated,
this,
&QuickEffectChain::slotPresetListUpdated);
m_pControlNumPresetsAvailable->forceSet(m_pChainPresetManager->numQuickEffectPresets());
m_pControlNumChainPresets->forceSet(m_pChainPresetManager->numQuickEffectPresets());
connect(m_pChainPresetManager.data(),
&EffectChainPresetManager::quickEffectChainPresetListUpdated,
this,
Expand Down
52 changes: 20 additions & 32 deletions src/effects/effectchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,15 @@ EffectChain::EffectChain(const QString& group,
ConfigKey(m_group, "num_effectslots"));
m_pControlNumEffectSlots->setReadOnly();

m_pControlNumPresetsAvailable = std::make_unique<ControlObject>(
ConfigKey(m_group, "num_presetsavailable"));
m_pControlNumPresetsAvailable->set(m_pChainPresetManager->numPresets());
m_pControlNumPresetsAvailable->setReadOnly();
m_pControlNumChainPresets = std::make_unique<ControlObject>(
ConfigKey(m_group, "num_chain_presets"));
m_pControlNumChainPresets->set(m_pChainPresetManager->numPresets());
m_pControlNumChainPresets->setReadOnly();
connect(m_pChainPresetManager.data(),
&EffectChainPresetManager::effectChainPresetListUpdated,
this,
&EffectChain::slotPresetListUpdated);

m_pControlChainLoaded =
std::make_unique<ControlObject>(ConfigKey(m_group, "loaded"));
m_pControlChainLoaded->setReadOnly();
if (group != QString()) {
m_pControlChainLoaded->forceSet(1.0);
}

m_pControlChainEnabled =
std::make_unique<ControlPushButton>(ConfigKey(m_group, "enabled"));
m_pControlChainEnabled->setButtonMode(ControlPushButton::POWERWINDOW);
Expand Down Expand Up @@ -101,24 +94,25 @@ EffectChain::EffectChain(const QString& group,
this,
&EffectChain::sendParameterUpdate);

m_pControlLoadedPreset = std::make_unique<ControlObject>(
ConfigKey(m_group, "loaded_preset"), false);
m_pControlLoadedPreset->connectValueChangeRequest(
this, &EffectChain::slotControlLoadedChainPresetRequest);
m_pControlLoadedChainPreset = std::make_unique<ControlObject>(
ConfigKey(m_group, "loaded_chain_preset"), false);
m_pControlLoadedChainPreset->connectValueChangeRequest(
this,
&EffectChain::slotControlLoadedChainPresetRequest);

m_pControlChainNextPreset = std::make_unique<ControlPushButton>(
m_pControlNextChainPreset = std::make_unique<ControlPushButton>(
ConfigKey(m_group, "next_chain"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ConfigKey(m_group, "next_chain"));
ConfigKey(m_group, "next_chain_preset"));

and add aliases

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to clutter the CO interface with aliases for such a minor issue. I think it is obvious already what the CO does even without the alias. We can add the alias at any time later.

Copy link
Contributor

Choose a reason for hiding this comment

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

You have already changed one. Now change the others or you leave it in an inconsistent state.

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR is against the mixxxdj/effects_refactoring. If you think the renaming + alias is rectified, you can create your own PR. Maybe another team member is convinced that an this is a good idea. I like to move on the other issues.

connect(m_pControlChainNextPreset.get(),
connect(m_pControlNextChainPreset.get(),
&ControlObject::valueChanged,
this,
&EffectChain::slotControlChainNextPreset);
&EffectChain::slotControlNextChainPreset);

m_pControlChainPrevPreset = std::make_unique<ControlPushButton>(
m_pControlPrevChainPreset = std::make_unique<ControlPushButton>(
ConfigKey(m_group, "prev_chain"));
connect(m_pControlChainPrevPreset.get(),
connect(m_pControlPrevChainPreset.get(),
&ControlObject::valueChanged,
this,
&EffectChain::slotControlChainPrevPreset);
&EffectChain::slotControlPrevChainPreset);

// Ignoring no-ops is important since this is for +/- tickers.
m_pControlChainSelector = std::make_unique<ControlEncoder>(
Expand Down Expand Up @@ -205,11 +199,6 @@ void EffectChain::loadChainPreset(EffectChainPresetPointer pPreset) {
int effectSlotIndex = 0;
for (const auto& pEffectPreset : pPreset->effectPresets()) {
EffectSlotPointer pEffectSlot = m_effectSlots.at(effectSlotIndex);
if (pEffectPreset->isEmpty()) {
pEffectSlot->loadEffectFromPreset(nullptr);
effectSlotIndex++;
continue;
}
pEffectSlot->loadEffectFromPreset(pEffectPreset);
effectSlotIndex++;
}
Expand All @@ -221,7 +210,6 @@ void EffectChain::loadChainPreset(EffectChainPresetPointer pPreset) {
emit presetNameChanged(m_presetName);

setControlLoadedPresetIndex(presetIndex());
m_pControlLoadedPreset->setAndConfirm(presetIndex() + 1);
}

void EffectChain::sendParameterUpdate() {
Expand Down Expand Up @@ -332,7 +320,7 @@ void EffectChain::slotControlChainSelector(double value) {
int index = presetIndex();
if (value > 0) {
index++;
} else {
} else if (value < 0) {
index--;
}
Be-ing marked this conversation as resolved.
Show resolved Hide resolved
loadChainPreset(presetAtIndex(index));
Expand All @@ -350,16 +338,16 @@ void EffectChain::slotControlLoadedChainPresetRequest(double value) {

void EffectChain::setControlLoadedPresetIndex(uint index) {
// add 1 to make the ControlObject 1-indexed like other ControlObjects
m_pControlLoadedPreset->setAndConfirm(index + 1);
m_pControlLoadedChainPreset->setAndConfirm(index + 1);
}

void EffectChain::slotControlChainNextPreset(double value) {
void EffectChain::slotControlNextChainPreset(double value) {
if (value > 0) {
loadChainPreset(presetAtIndex(presetIndex() + 1));
}
}

void EffectChain::slotControlChainPrevPreset(double value) {
void EffectChain::slotControlPrevChainPreset(double value) {
if (value > 0) {
loadChainPreset(presetAtIndex(presetIndex() - 1));
}
Expand All @@ -376,7 +364,7 @@ void EffectChain::slotChannelStatusChanged(

void EffectChain::slotPresetListUpdated() {
setControlLoadedPresetIndex(presetIndex());
m_pControlNumPresetsAvailable->forceSet(numPresets());
m_pControlNumChainPresets->forceSet(numPresets());
}

void EffectChain::enableForInputChannel(const ChannelHandleAndGroup& handleGroup) {
Expand Down
13 changes: 6 additions & 7 deletions src/effects/effectchain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class EffectChain : public QObject {
EffectsMessengerPointer m_pMessenger;
std::unique_ptr<ControlObject> m_pControlChainMix;
std::unique_ptr<ControlObject> m_pControlChainSuperParameter;
std::unique_ptr<ControlObject> m_pControlNumPresetsAvailable;
std::unique_ptr<ControlObject> m_pControlNumChainPresets;
QList<EffectSlotPointer> m_effectSlots;

protected slots:
Expand All @@ -117,8 +117,8 @@ class EffectChain : public QObject {
private slots:
void slotControlLoadedChainPresetRequest(double value);
void slotControlChainSelector(double value);
void slotControlChainNextPreset(double value);
void slotControlChainPrevPreset(double value);
void slotControlNextChainPreset(double value);
void slotControlPrevChainPreset(double value);
void slotChannelStatusChanged(double value, const ChannelHandleAndGroup& handleGroup);

private:
Expand All @@ -133,13 +133,12 @@ class EffectChain : public QObject {

std::unique_ptr<ControlPushButton> m_pControlClear;
std::unique_ptr<ControlObject> m_pControlNumEffectSlots;
std::unique_ptr<ControlObject> m_pControlChainLoaded;
std::unique_ptr<ControlPushButton> m_pControlChainEnabled;
std::unique_ptr<ControlPushButton> m_pControlChainMixMode;
std::unique_ptr<ControlObject> m_pControlLoadedPreset;
std::unique_ptr<ControlObject> m_pControlLoadedChainPreset;
std::unique_ptr<ControlEncoder> m_pControlChainSelector;
std::unique_ptr<ControlPushButton> m_pControlChainNextPreset;
std::unique_ptr<ControlPushButton> m_pControlChainPrevPreset;
std::unique_ptr<ControlPushButton> m_pControlNextChainPreset;
std::unique_ptr<ControlPushButton> m_pControlPrevChainPreset;

void setControlLoadedPresetIndex(uint index);

Expand Down
2 changes: 1 addition & 1 deletion src/effects/effectslot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ EffectParameterSlotBasePointer EffectSlot::getEffectParameterSlot(
}

void EffectSlot::loadEffectFromPreset(const EffectPresetPointer pPreset) {
if (!pPreset) {
if (!pPreset || pPreset->isEmpty()) {
loadEffectInner(nullptr, nullptr, true);
return;
}
Expand Down