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

Playlist feature: add 'Shuffle playlist' sidebar action #12498

Merged
merged 2 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 2 additions & 3 deletions src/library/playlisttablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ void PlaylistTableModel::shuffleTracks(const QModelIndexList& shuffle, const QMo
// this is used to exclude the already loaded track at pos #1 if used from running Auto-DJ
excludePos = exclude.sibling(exclude.row(), positionColumn).data().toInt();
}
int numOfTracks = rowCount();
if (shuffle.count() > 1) {
// if there is more then one track selected, shuffle selection only
foreach (QModelIndex shuffleIndex, shuffle) {
Expand All @@ -300,8 +301,7 @@ void PlaylistTableModel::shuffleTracks(const QModelIndexList& shuffle, const QMo
}
}
} else {
// if there is only one track selected, shuffle all tracks
int numOfTracks = rowCount();
// if there is no track or only one selected, shuffle all tracks
for (int i = 0; i < numOfTracks; i++) {
int oldPosition = index(i, positionColumn).data().toInt();
if (oldPosition != excludePos) {
Expand All @@ -310,7 +310,6 @@ void PlaylistTableModel::shuffleTracks(const QModelIndexList& shuffle, const QMo
}
}
// Set up list of all IDs
int numOfTracks = rowCount();
for (int i = 0; i < numOfTracks; i++) {
int position = index(i, positionColumn).data().toInt();
TrackId trackId(index(i, idColumn).data());
Expand Down
17 changes: 9 additions & 8 deletions src/library/trackset/baseplaylistfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -678,17 +678,18 @@ TreeItemModel* BasePlaylistFeature::sidebarModel() const {
return m_pSidebarModel;
}

void BasePlaylistFeature::bindLibraryWidget(WLibrary* libraryWidget,
KeyboardEventFilter* keyboard) {
Q_UNUSED(keyboard);
WLibraryTextBrowser* edit = new WLibraryTextBrowser(libraryWidget);
edit->setHtml(getRootViewHtml());
edit->setOpenLinks(false);
connect(edit,
void BasePlaylistFeature::bindLibraryWidget(WLibrary* pLibraryWidget,
KeyboardEventFilter* pKeyboard) {
Q_UNUSED(pKeyboard);
WLibraryTextBrowser* pEdit = new WLibraryTextBrowser(pLibraryWidget);
pEdit->setHtml(getRootViewHtml());
pEdit->setOpenLinks(false);
connect(pEdit,
&WLibraryTextBrowser::anchorClicked,
this,
&BasePlaylistFeature::htmlLinkClicked);
libraryWidget->registerView(m_rootViewName, edit);
m_pLibraryWidget = QPointer(pLibraryWidget);
m_pLibraryWidget->registerView(m_rootViewName, pEdit);
}

void BasePlaylistFeature::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) {
Expand Down
1 change: 1 addition & 0 deletions src/library/trackset/baseplaylistfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature {
QModelIndex m_lastClickedIndex;
QModelIndex m_lastRightClickedIndex;
QPointer<WLibrarySidebar> m_pSidebarWidget;
QPointer<WLibrary> m_pLibraryWidget;

QAction* m_pCreatePlaylistAction;
QAction* m_pDeletePlaylistAction;
Expand Down
55 changes: 55 additions & 0 deletions src/library/trackset/playlistfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
#include "sources/soundsourceproxy.h"
#include "util/db/dbconnection.h"
#include "util/duration.h"
#include "widget/wlibrary.h"
#include "widget/wlibrarysidebar.h"
#include "widget/wtracktableview.h"

namespace {

Expand Down Expand Up @@ -43,6 +45,12 @@ PlaylistFeature::PlaylistFeature(Library* pLibrary, UserSettingsPointer pConfig)
std::unique_ptr<TreeItem> pRootItem = TreeItem::newRoot(this);
m_pSidebarModel->setRootItem(std::move(pRootItem));
constructChildModel(kInvalidPlaylistId);

m_pShufflePlaylistAction = new QAction(tr("Shuffle Playlist"), this);
connect(m_pShufflePlaylistAction,
&QAction::triggered,
this,
&PlaylistFeature::slotShufflePlaylist);
}

QVariant PlaylistFeature::title() {
Expand Down Expand Up @@ -73,6 +81,10 @@ void PlaylistFeature::onRightClickChild(
QMenu menu(m_pSidebarWidget);
menu.addAction(m_pCreatePlaylistAction);
menu.addSeparator();
// TODO If playlist is selected and has more than one track selected
// show "Shuffle selected tracks", else show "Shuffle playlist"?
menu.addAction(m_pShufflePlaylistAction);
menu.addSeparator();
menu.addAction(m_pRenamePlaylistAction);
menu.addAction(m_pDuplicatePlaylistAction);
menu.addAction(m_pDeletePlaylistAction);
Expand Down Expand Up @@ -222,6 +234,49 @@ QString PlaylistFeature::fetchPlaylistLabel(int playlistId) {
return QString();
}

void PlaylistFeature::slotShufflePlaylist() {
int playlistId = playlistIdFromIndex(m_lastRightClickedIndex);
if (playlistId == kInvalidPlaylistId) {
return;
}

if (m_playlistDao.isPlaylistLocked(playlistId)) {
qDebug() << "Can't shuffle locked playlist" << playlistId
<< m_playlistDao.getPlaylistName(playlistId);
return;
}

// Shuffle all tracks
// If the playlist is loaded/visible shuffle only selected tracks
QModelIndexList selection;
if (isChildIndexSelectedInSidebar(m_lastRightClickedIndex) &&
m_pPlaylistTableModel->getPlaylist() == playlistId) {
if (m_pLibraryWidget) {
WTrackTableView* view = dynamic_cast<WTrackTableView*>(
m_pLibraryWidget->getActiveView());
if (view != nullptr) {
selection = view->selectionModel()->selectedIndexes();
}
}
m_pPlaylistTableModel->shuffleTracks(selection, QModelIndex());
} else {
// Create a temp model so we don't need to select the playlist
// in the persistent model in order to shuffle it
QScopedPointer<PlaylistTableModel> pPlaylistTableModel(
Copy link
Member

Choose a reason for hiding this comment

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

Did you consider to use unique_ptr and make_unique to get around the literal new()? You may also update other occurrences.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tbh I didn't think about it in this case. Ime my PRs sit for too long when I optimize too much en passant.
However, there are 9 occurences of this pattern in the playlist features and I can of course change them to use unique_ptr

Copy link
Member

Choose a reason for hiding this comment

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

Ups, I did not notice how that there so many occurrences. Thank you very much for fixing them all.

new PlaylistTableModel(this,
m_pLibrary->trackCollectionManager(),
"mixxx.db.model.playlist_shuffle"));
pPlaylistTableModel->selectPlaylist(playlistId);
pPlaylistTableModel->setSort(
pPlaylistTableModel->fieldIndex(
ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION),
Qt::AscendingOrder);
pPlaylistTableModel->select();

pPlaylistTableModel->shuffleTracks(selection, QModelIndex());
}
}

/// Purpose: When inserting or removing playlists,
/// we require the sidebar model not to reset.
/// This method queries the database and does dynamic insertion
Expand Down
3 changes: 3 additions & 0 deletions src/library/trackset/playlistfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class PlaylistFeature : public BasePlaylistFeature {
void slotPlaylistTableChanged(int playlistId) override;
void slotPlaylistContentOrLockChanged(const QSet<int>& playlistIds) override;
void slotPlaylistTableRenamed(int playlistId, const QString& newName) override;
void slotShufflePlaylist();

protected:
QString fetchPlaylistLabel(int playlistId) override;
Expand All @@ -44,4 +45,6 @@ class PlaylistFeature : public BasePlaylistFeature {

private:
QString getRootViewHtml() const override;

QAction* m_pShufflePlaylistAction;
};
10 changes: 5 additions & 5 deletions src/library/trackset/setlogfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ QVariant SetlogFeature::title() {
}

void SetlogFeature::bindLibraryWidget(
WLibrary* libraryWidget, KeyboardEventFilter* keyboard) {
BasePlaylistFeature::bindLibraryWidget(libraryWidget, keyboard);
WLibrary* pLibraryWidget, KeyboardEventFilter* pKeyboard) {
BasePlaylistFeature::bindLibraryWidget(pLibraryWidget, pKeyboard);
connect(&PlayerInfo::instance(),
&PlayerInfo::currentPlayingTrackChanged,
this,
&SetlogFeature::slotPlayingTrackChanged);
m_libraryWidget = QPointer(libraryWidget);
m_pLibraryWidget = QPointer(pLibraryWidget);
}

void SetlogFeature::deleteAllUnlockedPlaylistsWithFewerTracks() {
Expand Down Expand Up @@ -600,9 +600,9 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) {
// View needs a refresh

bool hasActiveView = false;
if (m_libraryWidget) {
if (m_pLibraryWidget) {
WTrackTableView* view = dynamic_cast<WTrackTableView*>(
m_libraryWidget->getActiveView());
m_pLibraryWidget->getActiveView());
if (view != nullptr) {
// We have a active view on the history. The user may have some
// important active selection. For example putting track into crates
Expand Down
2 changes: 0 additions & 2 deletions src/library/trackset/setlogfeature.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ class SetlogFeature : public BasePlaylistFeature {

int m_currentPlaylistId;
int m_yearNodeId;

QPointer<WLibrary> m_libraryWidget;
Library* m_pLibrary;
UserSettingsPointer m_pConfig;
};