| schweingruber |
| Amarok |
EngineController::increaseVolume( int ticks ) and EngineController::decreaseVolume( int ticks ) take an argument that specifies the amount by which to increase/decrease the volume.
But it gets called with 0 as argument when pressing the shortcut key, so the volume was not changed at all.
To make the keyboard shortcuts work, create new slots that don't take arguments and call the previous ones with the default value.
Press the increase or decrease volume shortcut keys (+ and - by default), the volume is actually changed now, before they had no effect.
| Lint Skipped |
| Unit Tests Skipped |
PS: I think the argument is 0 because QAction::triggered() has a bool checked argument that's probably passed along.
But Qt's new signal/slots syntax doesn't support default arguments anyway, according to the docs.
Because I couldn't get it to compile successfully, I got this error:
In lambda function passing 'const EngineController' as 'this' argument discards qualifiers
Ping?
Btw, I mainly followed what was done for the stop() action in commit 1b81541e, where a new regularStop() slot was created for similar reasons AIUI.
Further suggestions how to fix it differently would be welcome too, of course.
I'm always easy to confuse about pointers and const, but isn't ec a const pointer to a const object? So calling ec->increaseVolume() as a non-const function) wouldn't really work outside a lambda either.
What I'm not sure about is why it was done that way and why it does work with the new style connects.
Yes, exactly. Adding a line ec->increaseVolume(); at that place gives the same compiler error:
/home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20191004T143525~5ed62f9a08/src/MainWindow.cpp: In member function 'void MainWindow::createActions()':
/home/abuild/rpmbuild/BUILD/amarok-2.9.70git.20191004T143525~5ed62f9a08/src/MainWindow.cpp:904:24: error: passing 'const EngineController' as 'this' argument discards qualifiers [-fpermissive]
ec->increaseVolume();
^What I'm not sure about is why it was done that way and why it does work with the new style connects.
Apparently Qt's connect stuff "circumvents" the constness. It's not as if the function is actually being called here.
I can't say why ec was declared const, but it was already the case in the KDE4 version (and it was no problem because it is only used to connect things to, with the old-style connect this particular issue with default arguments didn't exist either).
Shall I just remove the const from ec then and connect to a lambda instead?
Ok, as it was accepted, and there were no further comments since my last question whether I should change it, I'm just going to commit it now.