AutoDJ: Add Random Track Control to AutoDJ. #3076

PawBud · 2020-09-04T16:16:37Z

This PR aims to implement the wishlist Feature Request, Adding Control to Add Random Track to AutoDJ.
Discussions:-https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/Add.20Control.20to.20Add.20Random.20Track.20to.20AutoDJ
Feature Request:-https://bugs.launchpad.net/mixxx/+bug/1887845

src/controllers/controlpickermenu.cpp

daschuer · 2020-09-04T21:27:26Z

This PR does currently not build:

/usr/bin/ld: libmixxx-lib.a(dlgautodj.cpp.o): in function `DlgAutoDJ::addRandomTrackButton(bool)':
dlgautodj.cpp:(.text+0x20): multiple definition of `DlgAutoDJ::addRandomTrackButton(bool)'; libmixxx-lib.a(mocs_compilation.cpp.o):mocs_compilation.cpp:(.text+0x5370): first defined here
/usr/bin/ld: libmixxx-lib.a(dlgautodj.cpp.o): in function `DlgAutoDJ::addRandomTrackButton(bool)':
dlgautodj.cpp:(.text+0x28): undefined reference to `AutoDJProcessor::addRandomTrack()'
/usr/bin/ld: libmixxx-lib.a(mocs_compilation.cpp.o): in function `AutoDJProcessor::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)':
mocs_compilation.cpp:(.text+0x1e4ce): undefined reference to `AutoDJProcessor::addRandomTrack()'
/usr/bin/ld: libmixxx-lib.a(autodjprocessor.cpp.o): in function `AutoDJProcessor::controlAddRandomTrack(double)':
autodjprocessor.cpp:(.text+0xab): undefined reference to `AutoDJProcessor::addRandomTrack()'

daschuer

We like to have all commits build. So please rebase and sqash the commits after they are building again.

src/library/autodj/autodjprocessor.cpp

src/library/autodj/dlgautodj.cpp

PawBud · 2020-09-05T07:29:34Z

This PR does currently not build:

/usr/bin/ld: libmixxx-lib.a(dlgautodj.cpp.o): in function `DlgAutoDJ::addRandomTrackButton(bool)':
dlgautodj.cpp:(.text+0x20): multiple definition of `DlgAutoDJ::addRandomTrackButton(bool)'; libmixxx-lib.a(mocs_compilation.cpp.o):mocs_compilation.cpp:(.text+0x5370): first defined here
/usr/bin/ld: libmixxx-lib.a(dlgautodj.cpp.o): in function `DlgAutoDJ::addRandomTrackButton(bool)':
dlgautodj.cpp:(.text+0x28): undefined reference to `AutoDJProcessor::addRandomTrack()'
/usr/bin/ld: libmixxx-lib.a(mocs_compilation.cpp.o): in function `AutoDJProcessor::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)':
mocs_compilation.cpp:(.text+0x1e4ce): undefined reference to `AutoDJProcessor::addRandomTrack()'
/usr/bin/ld: libmixxx-lib.a(autodjprocessor.cpp.o): in function `AutoDJProcessor::controlAddRandomTrack(double)':
autodjprocessor.cpp:(.text+0xab): undefined reference to `AutoDJProcessor::addRandomTrack()'

So Sorry about this, It was pretty reckless of me to not notice that, I'll rebase the branch as you suggested.

PawBud · 2020-09-06T05:17:36Z

Ok so, I guess I can't figure this one out

How do I resolve the 2 errors?

daschuer · 2020-09-06T08:39:54Z

You screenshots are not that handy.
It would be better to just past the error message and ask explicit what you want to know.

daschuer · 2020-09-06T08:41:16Z

If you address my inline comments, I think everything schould work.

PawBud · 2020-09-07T05:09:53Z

Your screenshots are not that handy.
It would be better to just past the error message and ask explicit what you want to know.

/usr/bin/ld: libmixxx-lib.a(mocs_compilation.cpp.o): in function `AutoDJProcessor::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)':
/home/pawbuddy/Desktop/OpenSource/mixxx/cmake_build/mixxx-lib_autogen/M6YM4VV6JB/moc_autodjprocessor.cpp:640: undefined reference to `AutoDJProcessor::addRandomTrack()'
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/mixxx-test.dir/build.make:1546: mixxx-test] Error 1
make[1]: *** [CMakeFiles/Makefile2:1126: CMakeFiles/mixxx-test.dir/all] Error 2
make: *** [Makefile:163: all] Error 2

I couldn't make anything out of this error, I mean it says that it's undefined but I don't really know where are the changes I am supposed to make? I even tried to make the changes u requested first, re-built my cmake_build dir and re-compiled it but the error persisted

daschuer · 2020-09-07T05:48:58Z

You need to remove
slot:
addRandomTrack()
From
AutoDJProcessor class definition.
Currently Qt's moc is trying to link to it, but it is not there.

This commit introduces the Random Track Control to AutoDJ. It replaces add_random with add_random_track in the whole codebase, and changing the function names and QT class members to maintain consisistency.

PawBud · 2020-09-08T04:34:49Z

@daschuer What more needs to be done?

daschuer · 2020-09-08T19:02:24Z

Looks and works good. Once you have removed the draft state I can merge.

Would you mind to also add the new control here:
https://github.com/mixxxdj/mixxx/wiki/Mixxxcontrols

Thank you very much.

PawBud · 2020-09-09T05:23:12Z

https://github.com/mixxxdj/mixxx/wiki/Mixxxcontrols#autodj over here, right?
I think the range will be binary and as for the What it does I can just add Adds a random track to the Auto DJ queue and for on screen feedback I can add Auto DJ Add Random Track ?

ronso0 · 2020-09-09T12:03:38Z

yes, the range is binary, the description is also okay.
on-screen feedback is "Track is added to AutoDJ queue"

Holzhaus · 2020-09-09T12:31:24Z

Please add a changelog entry to CHANGELOG.md.

PawBud · 2020-09-09T14:16:33Z

Hope I added it at the right place :)

PawBud · 2020-09-09T14:19:36Z

yes, the range is binary, the description is also okay.
on-screen feedback is "Track is added to AutoDJ queue"

Sorry about that mistake, should I increment all the leading numbers by 1?

CHANGELOG.md

PawBud · 2020-09-09T16:12:55Z

Done @Holzhaus

ronso0 · 2020-09-09T19:48:35Z

Sorry about that mistake, should I increment all the leading numbers by 1?

Please don't. Those are just links to footnotes. I set it to 217 introduced in 2.3

PawBud · 2020-09-10T11:34:14Z

Please don't. Those are just links to footnotes. I set it to 217 introduced in 2.3

Yea, sorry about that, I was confused with adding the footnote. Moreover, Is there anything left to do in this PR?

ronso0 · 2020-09-10T11:42:32Z

Don't be sorry, I just wanted to save you from annoying, unnecessary work :)
Later on, we could restore the footwork links (or show the footnote on hover like with PR links?) and concatenate the version hints.

I did not look into the code. Someone else needs to review & merge.

PawBud · 2020-09-10T11:47:44Z

Later on, we could restore the footwork links (or show the footnote on hover like with PR links?) and concatenate the version hints.

Yes! I was thinking of discussing things regarding the footwork in the wiki, Would love to work on it.

I did not look into the code. Someone else needs to review & merge.

Sure

Holzhaus · 2020-09-10T12:13:22Z

Later on, we could restore the footwork links (or show the footnote on hover like with PR links?) and concatenate the version hints.

We should eventually move this from the wiki to the manual, then we get versioning for free.

Be-ing · 2020-10-26T19:37:40Z

@Holzhaus anything left to do here? Ready to merge?

Holzhaus · 2020-10-26T19:47:29Z

The pre-commit hooks fail.

Holzhaus · 2020-10-26T19:54:42Z

Looks like the control documentation accidently ended up in the manual even though it hasn't been merged yet. I'll remove it: mixxxdj/manual#255 and you can open a new PR that adds it back (with .. versionadded:: 2.4.0) when this is ready to merge.

Be-ing · 2020-10-26T19:55:37Z

What about this is not ready to merge?

Holzhaus · 2020-10-26T19:59:09Z

The pre-commit hooks fail.

PawBud · 2020-10-30T11:02:41Z

The pre-commit hooks fail.
@Holzhaus, How can I be of help?

Holzhaus · 2020-10-30T11:11:39Z

Click on the "Details" for the Travis Check, look at the output for the "pre-commit-pr" job and apply the diff from there.

To avoid future issues, I suggest you install our pre-commit hooks locally: https://github.com/mixxxdj/mixxx/wiki/Using-Git#set-up-automatic-code-checking

PawBud · 2020-10-30T11:35:09Z

Click on the "Details" for the Travis Check, look at the output for the "pre-commit-pr" job and apply the diff from there.

So about this

Will this not introduce an inconsistency in our codebase?

Holzhaus · 2020-10-30T13:09:34Z

It you installed the pre-commit hook, you won't be able to commit that patch as-is. The hook will detect the long lines and break it according to our code style.

PawBud · 2020-10-30T13:35:31Z

It you installed the pre-commit hook, you won't be able to commit that patch as-is. The hook will detect the long lines and break it according to our code style.

ohh ok, Sure then, I'll commit the changes now, btw yes I did install it, thanks :)

PawBud · 2020-10-31T08:24:27Z

@Holzhaus Is this ok?

daschuer · 2020-11-03T08:47:06Z

Thank you for your first PR :-)

Before merge, you need to sign:
https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6M
please comment here when done.

PawBud · 2020-11-03T09:50:29Z

@daschuer uhmm, so the thing is, this is my second pr, my first one was #3043 (comment), I had already signed the aforementioned form. Do I need to re-sign it?

daschuer · 2020-11-03T10:11:25Z

Oh I am very sorry. I have missed to add you name to the Mixxx about box.
I will do now.
Nothing else to do for you.

Holzhaus · 2020-11-03T11:03:33Z

I'll take care of it this time, but please don't merge CO changes before a corresponding PR for the manual that documents the changes is ready to merge.

In this instance, we already have documentation because @PawBud added it to the wiki page before we migrated the control documentation to the wiki (reverted in mixxxdj/manual#255). I'll make a new PR that reverts that PR.

…trol This reverts commit 78d3a4a and bumps the versionadded to 2.4.0, since mixxxdj/mixxx#3076 has now been merged.

This reverts commit 78d3a4a and bumps the versionadded to 2.4.0, since mixxxdj/mixxx#3076 has now been merged.

Holzhaus · 2020-11-03T11:13:20Z

Have a look at: mixxxdj/manual#277

PawBud commented Sep 4, 2020

View reviewed changes

src/controllers/controlpickermenu.cpp Outdated Show resolved Hide resolved

PawBud marked this pull request as draft September 4, 2020 16:19

daschuer requested changes Sep 4, 2020

View reviewed changes

src/library/autodj/autodjprocessor.cpp Outdated Show resolved Hide resolved

src/library/autodj/dlgautodj.cpp Outdated Show resolved Hide resolved

src/library/autodj/dlgautodj.cpp Outdated Show resolved Hide resolved

PawBud added 2 commits September 7, 2020 11:43

AutoDJ: Add Random Track Control to AutoDJ.

3dccf14

This commit introduces the Random Track Control to AutoDJ. It replaces add_random with add_random_track in the whole codebase, and changing the function names and QT class members to maintain consisistency.

ControlPickerMenu: Removed grammatical error.

6a75a0c

PawBud marked this pull request as ready for review September 9, 2020 05:21

Holzhaus requested changes Sep 9, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Changelog: Added the change in the controllers column.

89ada92

Be-ing changed the base branch from master to main October 23, 2020 23:10

Holzhaus mentioned this pull request Oct 26, 2020

source/chapters/advanced_topics: Remove [AutoDJ],add_random_track control mixxxdj/manual#255

Merged

Travis CI: Fixed the Pre-commit-pr hooks error

f7aa473

github-actions bot added library skins labels Oct 30, 2020

daschuer approved these changes Nov 3, 2020

View reviewed changes

daschuer merged commit 21be90f into mixxxdj:main Nov 3, 2020

Holzhaus added a commit to Holzhaus/manual that referenced this pull request Nov 3, 2020

source/chapters/advanced_topics: Remove [AutoDJ],add_random_track con…

4c15715

…trol This reverts commit 78d3a4a and bumps the versionadded to 2.4.0, since mixxxdj/mixxx#3076 has now been merged.

Holzhaus added a commit to Holzhaus/manual that referenced this pull request Nov 3, 2020

source/chapters/advanced_topics: Add [AutoDJ],add_random_track control

535f334

This reverts commit 78d3a4a and bumps the versionadded to 2.4.0, since mixxxdj/mixxx#3076 has now been merged.

Holzhaus added a commit to Holzhaus/manual that referenced this pull request Nov 3, 2020

source/chapters/advanced_topics: Add [AutoDJ],add_random_track control

fddcf56

This reverts commit 78d3a4a and bumps the versionadded to 2.4.0, since mixxxdj/mixxx#3076 has now been merged.

Holzhaus mentioned this pull request Nov 3, 2020

source/chapters/advanced_topics: Add [AutoDJ],add_random_track control mixxxdj/manual#277

Merged

daschuer added a commit to daschuer/mixxx that referenced this pull request Aug 18, 2021

remove redundant mixxxdj#3076 entry

0ee41f3

mixxxbot mentioned this pull request Aug 23, 2022

Feature Request - Add Control to Add Random Track to AutoDJ #10050

Closed

AutoDJ: Add Random Track Control to AutoDJ. #3076

AutoDJ: Add Random Track Control to AutoDJ. #3076

Conversation

PawBud commented Sep 4, 2020

daschuer commented Sep 4, 2020

daschuer left a comment

Choose a reason for hiding this comment

PawBud commented Sep 5, 2020

PawBud commented Sep 6, 2020

daschuer commented Sep 6, 2020

daschuer commented Sep 6, 2020

PawBud commented Sep 7, 2020

daschuer commented Sep 7, 2020

PawBud commented Sep 8, 2020

daschuer commented Sep 8, 2020

PawBud commented Sep 9, 2020 • edited Loading

ronso0 commented Sep 9, 2020

Holzhaus commented Sep 9, 2020

PawBud commented Sep 9, 2020 • edited Loading

PawBud commented Sep 9, 2020

PawBud commented Sep 9, 2020

ronso0 commented Sep 9, 2020

PawBud commented Sep 10, 2020

ronso0 commented Sep 10, 2020

PawBud commented Sep 10, 2020

Holzhaus commented Sep 10, 2020

Be-ing commented Oct 26, 2020

Holzhaus commented Oct 26, 2020

Holzhaus commented Oct 26, 2020

Be-ing commented Oct 26, 2020

Holzhaus commented Oct 26, 2020

PawBud commented Oct 30, 2020

Holzhaus commented Oct 30, 2020

PawBud commented Oct 30, 2020 • edited Loading

Holzhaus commented Oct 30, 2020

PawBud commented Oct 30, 2020 • edited Loading

PawBud commented Oct 31, 2020

daschuer commented Nov 3, 2020

PawBud commented Nov 3, 2020 • edited Loading

daschuer commented Nov 3, 2020

Holzhaus commented Nov 3, 2020

Holzhaus commented Nov 3, 2020

PawBud commented Sep 9, 2020 •

edited

Loading

PawBud commented Sep 9, 2020 •

edited

Loading

PawBud commented Oct 30, 2020 •

edited

Loading

PawBud commented Oct 30, 2020 •

edited

Loading

PawBud commented Nov 3, 2020 •

edited

Loading