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

Default value (using right click) for "Moog filter" #12479

Closed
nvdl opened this issue Dec 29, 2023 · 6 comments · Fixed by #12480
Closed

Default value (using right click) for "Moog filter" #12479

nvdl opened this issue Dec 29, 2023 · 6 comments · Fixed by #12480

Comments

@nvdl
Copy link

nvdl commented Dec 29, 2023

Bug Description

Mixxx 2.3.6
Git Version: 2.3.6
Date: Tuesday, August 15, 2023 7:47:16 AM CEST
Platform: Linux x86_64

I don't know if it is a bug or not.

Right clicking on the knob of "Moog filter" resets the knob to the far left.
Shouldn't it be centered where it has the minimal effect on the sound?

Version

2.3.6

OS

Debian 11

@nvdl nvdl added the bug label Dec 29, 2023
@Swiftb0y
Copy link
Member

Yup, though the problem is not effect specific, rather issue is that right clicking the super knob of an effect unit always resets it to 0 rather than the default. In 2.4, rightclicking the filter knob with an effect (such as the moog-filter) resets it to the correct value.

@Swiftb0y
Copy link
Member

Ah, my mistake... the difference is that the super1 control behaves correctly but meta doesn't...

Found the culprit.

m_pControlMetaParameter = std::make_unique<ControlPotmeter>(
ConfigKey(m_group, "meta"), 0.0, 1.0);
// QObject::connect cannot connect to slots with optional parameters using function
// pointer syntax if the slot has more parameters than the signal, so use a lambda
// to hack around this limitation.
connect(m_pControlMetaParameter.get(),
&ControlObject::valueChanged,
this,
[=, this](double value) { slotEffectMetaParameter(value); });
m_pControlMetaParameter->set(0.0);
m_pControlMetaParameter->setDefaultValue(0.0);
m_pControlLoaded->forceSet(0.0);
}

The setDefaultValue must not hardcode a value and instead make it dependent on the loaded effect. Not sure when if/when I'll be able to provide a fix. Someone else feel free to pick this up.

@nvdl
Copy link
Author

nvdl commented Dec 29, 2023

I can help with it but it looks like I have to go through some coding guidelines for the code etc.

What solution do you suggest here?
Set both lines (131 and 132) to a mid value? What numeric range are the knobs using?

@Swiftb0y
Copy link
Member

No worries, I think I got it. just waiting for the compile so I can test right now. In the meantime could you confirm whether #14257 fixes the issue for you? You should be able to download a deb from the bottom of https://github.com/mixxxdj/mixxx/actions/runs/7360936261 once it finished building.

@nvdl
Copy link
Author

nvdl commented Dec 30, 2023

Works as expected. Cheers!

@Swiftb0y
Copy link
Member

Thanks for also confirming. There won't be another 2.3 release so its only fixed in 2.4 newer.

@daschuer daschuer added this to the 2.4.0 milestone Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants