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

Fix eq apply-to-all in preferences. #437

Merged
merged 3 commits into from
Dec 28, 2014
Merged

Fix eq apply-to-all in preferences. #437

merged 3 commits into from
Dec 28, 2014

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Dec 26, 2014

  • slotLoadEffectChainInSlot was deleted, so the emit did nothing. Create
    a function to fix that behavior.
  • Don't instant-apply eq and quickeffect selections -- wait for the user to hit apply.
    (Otherwise, what's the point of an apply button?)

fixes lp:1404525

* slotLoadEffectChainInSlot was deleted, so the emit did nothing.  Create
  a function to fix that behavior.
* Don't instant-apply eq and quickeffect selections -- wait for the user to hit apply.
  (Otherwise, what's the point of an apply button?)

fixes lp:1404525
@@ -331,6 +331,34 @@ EqualizerRack::EqualizerRack(EffectsManager* pEffectsManager,
EqualizerRack::formatGroupString(iRackNumber)) {
}

bool EqualizerRack::loadEffectToGroup(const QString& group,
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 is copied from the QuickEffectRack -- it might make sense to make this a generic function that all racks have.

Copy link
Member

Choose a reason for hiding this comment

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

It was at one point and I removed it :) it doesn't make sense for non PerGroupRacks. I think they were slightly different but if they are actually the same now then it could be added to PerGroupRack.

Copy link
Member Author

Choose a reason for hiding this comment

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

we can leave it as dupe code for now

@daschuer
Copy link
Member

Just tested it and there is still an issue with the "Use the same .." check box.
If you change and apply an EQ and then uncheck the box, only he deck 1 combobox has the new EQ.

@ywwg
Copy link
Member Author

ywwg commented Dec 26, 2014

that's actually on purpose. It remembers what the individual choices were in case you want to go back and forth. But when the settings are applied to all, they are

@daschuer
Copy link
Member

That is confusing for me. Why is Deck 1 changed, but the others not? What is special to Deck 1?

Does the "Use the same .." checkbox ignore the individual settings from cfg file? I would prefer a consistent dependency; checkbox checked, all cfg values equal.

@ywwg
Copy link
Member Author

ywwg commented Dec 27, 2014

ok fixed

EffectSlotPointer pEffectSlot = pChainSlot->getEffectSlot(0);
if (pEffectSlot) {
pEffectSlot->onChainSuperParameterChanged(
pChainSlot->getSuperParameter(), true);
Copy link
Member

Choose a reason for hiding this comment

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

That must not be done for EQs since we have no superknob on the EQ Racks.
This will override the Effect defaults of the effects with a superknob binding.
The force parameter is a quick effect only thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@daschuer
Copy link
Member

LGTM, Thank you!

ywwg added a commit that referenced this pull request Dec 28, 2014
Fix eq apply-to-all in preferences.
@ywwg ywwg merged commit de87e07 into mixxxdj:master Dec 28, 2014
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.

None yet

3 participants