From 10f39d7e26f51a6c156a9c1721d6649c338e6c65 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 22:40:43 +0200 Subject: [PATCH 01/11] History feature: selecting YEAR node disables search & cover art --- src/library/trackset/baseplaylistfeature.cpp | 4 +-- src/library/trackset/setlogfeature.cpp | 36 ++++++++++++++++---- src/library/trackset/setlogfeature.h | 1 + 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 46da3f86197..23c8d667195 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -201,9 +201,7 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { //qDebug() << "BasePlaylistFeature::activateChild()" << index; int playlistId = playlistIdFromIndex(index); if (playlistId == kInvalidPlaylistId) { - // This happens if user clicks on group nodes. - // Doesn't apply to YEAR nodes in the history feature, they are linked to - // a dummy playlist. + // may happen during initialization return; } m_lastClickedIndex = index; diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 54f1c4d5a9f..bcc7b8ee690 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -639,7 +639,6 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { if (newIndex.isValid()) { emit featureSelect(this, newIndex); activateChild(newIndex); - return; } else if (rootWasSelected) { // calling featureSelect with invalid index will select the root item emit featureSelect(this, newIndex); @@ -672,6 +671,27 @@ void SetlogFeature::activate() { activatePlaylist(m_playlistId); } +void SetlogFeature::activateChild(const QModelIndex& index) { + // qDebug() << "SetlogFeature::activateChild()" << index; + int playlistId = playlistIdFromIndex(index); + if (playlistId == kInvalidPlaylistId) { + // may happen during initialization + return; + } + m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); + emit saveModelState(); + m_pPlaylistTableModel->setTableModel(playlistId); + emit showTrackModel(m_pPlaylistTableModel); + if (playlistId == m_placeholderId) { + // Disable search and cover art for YEAR items + emit disableSearch(); + emit enableCoverArtDisplay(false); + } else { + emit enableCoverArtDisplay(true); + } +} + void SetlogFeature::activatePlaylist(int playlistId) { // qDebug() << "SetlogFeature::activatePlaylist()" << playlistId; if (playlistId == kInvalidPlaylistId) { @@ -684,8 +704,8 @@ void SetlogFeature::activatePlaylist(int playlistId) { emit saveModelState(); m_pPlaylistTableModel->setTableModel(playlistId); emit showTrackModel(m_pPlaylistTableModel); - emit enableCoverArtDisplay(true); - // Update sidebar selection only if this is a child, incl. current playlist. + // Update sidebar selection only if this is a child, incl. current playlist + // and YEAR nodes. // indexFromPlaylistId() can't be used because, in case the root item was // selected, that would switch to the 'current' child. if (m_lastClickedIndex != m_pSidebarModel->getRootIndex()) { @@ -694,10 +714,14 @@ void SetlogFeature::activatePlaylist(int playlistId) { // redundant // activateChild(index); - // TODO(ronso0) Disable search for YEAR items - // emit disableSearch(); - // emit enableCoverArtDisplay(false); + if (playlistId == m_placeholderId) { + // Disable search and cover art for YEAR items + emit disableSearch(); + emit enableCoverArtDisplay(false); + return; + } } + emit enableCoverArtDisplay(true); } QString SetlogFeature::getRootViewHtml() const { diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 9d6c622920b..92b3d0c14ec 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -32,6 +32,7 @@ class SetlogFeature : public BasePlaylistFeature { void slotDeletePlaylist() override; void slotGetNewPlaylist(); void activate() override; + void activateChild(const QModelIndex& index) override; protected: QModelIndex constructChildModel(int selectedId); From 820f05c9288124730d0f172f563c60562daed61e Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 23:03:27 +0200 Subject: [PATCH 02/11] History feature: improve playlist variable names --- src/library/trackset/setlogfeature.cpp | 52 +++++++++++++------------- src/library/trackset/setlogfeature.h | 4 +- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index bcc7b8ee690..ec954b2eccc 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -39,8 +39,8 @@ SetlogFeature::SetlogFeature( /*keep deleted tracks*/ true), QStringLiteral("SETLOGHOME"), QStringLiteral("history")), - m_playlistId(kInvalidPlaylistId), - m_placeholderId(kInvalidPlaylistId), + m_currentPlaylistId(kInvalidPlaylistId), + m_yearNodeId(kInvalidPlaylistId), m_pLibrary(pLibrary), m_pConfig(pConfig) { // remove unneeded entries @@ -48,13 +48,11 @@ SetlogFeature::SetlogFeature( // Create empty placeholder playlist for YEAR items QString placeholderName = "historyPlaceholder"; - m_placeholderId = m_playlistDao.createUniquePlaylist(&placeholderName, + m_yearNodeId = m_playlistDao.createUniquePlaylist(&placeholderName, PlaylistDAO::PLHT_UNKNOWN); - VERIFY_OR_DEBUG_ASSERT(m_placeholderId != kInvalidPlaylistId) { - qWarning() << "Failed to create empty History placeholder playlist!"; - } + DEBUG_ASSERT(m_yearNodeId != kInvalidPlaylistId); // just to be safe - m_playlistDao.setPlaylistLocked(m_placeholderId, true); + m_playlistDao.setPlaylistLocked(m_yearNodeId, true); //construct child model m_pSidebarModel->setRootItem(TreeItem::newRoot(this)); @@ -131,10 +129,10 @@ void SetlogFeature::slotDeletePlaylist() { return; } int playlistId = playlistIdFromIndex(m_lastRightClickedIndex); - if (playlistId == m_playlistId) { + if (playlistId == m_currentPlaylistId) { // the current setlog must not be deleted return; - } else if (playlistId == m_placeholderId) { + } else if (playlistId == m_yearNodeId) { // this is a YEAR node slotDeleteAllUnlockedChildPlaylists(); } else { @@ -172,7 +170,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex m_pLockPlaylistAction->setText(locked ? tr("Unlock") : tr("Lock")); QMenu menu(m_pSidebarWidget); - if (playlistId == m_placeholderId) { + if (playlistId == m_yearNodeId) { // this is a YEAR item menu.addAction(m_pLockAllChildPlaylists); menu.addAction(m_pUnlockAllChildPlaylists); @@ -190,7 +188,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex menu.addAction(m_pAddToAutoDJTopAction); menu.addSeparator(); menu.addAction(m_pRenamePlaylistAction); - if (playlistId != m_playlistId) { + if (playlistId != m_currentPlaylistId) { // Todays playlist should not be locked or deleted menu.addAction(m_pDeletePlaylistAction); menu.addAction(m_pLockPlaylistAction); @@ -199,10 +197,10 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex // The very first (oldest) setlog cannot be joint menu.addAction(m_pJoinWithPreviousAction); } - if (playlistId == m_playlistId) { + if (playlistId == m_currentPlaylistId) { // Todays playlists can change ! m_pStartNewPlaylist->setEnabled( - m_playlistDao.tracksInPlaylist(m_playlistId) > 0); + m_playlistDao.tracksInPlaylist(m_currentPlaylistId) > 0); menu.addAction(m_pStartNewPlaylist); } menu.addSeparator(); @@ -270,7 +268,7 @@ QModelIndex SetlogFeature::constructChildModel(int selectedId) { // create YEAR item the playlist will sorted into // store id of empty placeholder playlist auto pNewGroupItem = std::make_unique( - QString::number(yearCreated), m_placeholderId); + QString::number(yearCreated), m_yearNodeId); pGroupItem = pNewGroupItem.get(); groups.insert(yearCreated, pGroupItem); itemList.push_back(std::move(pNewGroupItem)); @@ -319,7 +317,7 @@ QString SetlogFeature::fetchPlaylistLabel(int playlistId) { } void SetlogFeature::decorateChild(TreeItem* item, int playlistId) { - if (playlistId == m_playlistId) { + if (playlistId == m_currentPlaylistId) { item->setIcon(QIcon(":/images/library/ic_library_history_current.svg")); } else if (m_playlistDao.isPlaylistLocked(playlistId)) { item->setIcon(QIcon(":/images/library/ic_library_locked.svg")); @@ -346,10 +344,10 @@ void SetlogFeature::slotGetNewPlaylist() { } //qDebug() << "Creating session history playlist name:" << set_log_name; - m_playlistId = m_playlistDao.createPlaylist( + m_currentPlaylistId = m_playlistDao.createPlaylist( set_log_name, PlaylistDAO::PLHT_SET_LOG); - if (m_playlistId == kInvalidPlaylistId) { + if (m_currentPlaylistId == kInvalidPlaylistId) { qDebug() << "Setlog playlist Creation Failed"; qDebug() << "An unknown error occurred while creating playlist: " << set_log_name; @@ -358,9 +356,9 @@ void SetlogFeature::slotGetNewPlaylist() { } // reload child model again because the 'added' signal fired by PlaylistDAO - // might have triggered slotPlaylistTableChanged() before m_playlistId was set, + // might have triggered slotPlaylistTableChanged() before m_currentPlaylistId was set, // which causes the wrong playlist being decorated as 'current' - slotPlaylistTableChanged(m_playlistId); + slotPlaylistTableChanged(m_currentPlaylistId); } void SetlogFeature::slotJoinWithPrevious() { @@ -385,7 +383,7 @@ void SetlogFeature::slotJoinWithPrevious() { if (previousPlaylistId >= 0) { m_pPlaylistTableModel->setTableModel(previousPlaylistId); - if (currentPlaylistId == m_playlistId) { + if (currentPlaylistId == m_currentPlaylistId) { // mark all the Tracks in the previous Playlist as played m_pPlaylistTableModel->select(); @@ -403,7 +401,7 @@ void SetlogFeature::slotJoinWithPrevious() { } // Change current setlog - m_playlistId = previousPlaylistId; + m_currentPlaylistId = previousPlaylistId; } qDebug() << "slotJoinWithPrevious() current:" << currentPlaylistId @@ -555,7 +553,7 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { return; } - if (m_pPlaylistTableModel->getPlaylist() == m_playlistId) { + if (m_pPlaylistTableModel->getPlaylist() == m_currentPlaylistId) { // View needs a refresh bool hasActiveView = false; @@ -580,7 +578,7 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { } else { // TODO(XXX): Care whether the append succeeded. m_playlistDao.appendTrackToPlaylist( - currentPlayingTrackId, m_playlistId); + currentPlayingTrackId, m_currentPlaylistId); } } @@ -599,7 +597,7 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { // a child index was selected (actual playlist or YEAR item) int lastClickedPlaylistId = m_pPlaylistTableModel->getPlaylist(); - if (lastClickedPlaylistId == m_placeholderId) { + if (lastClickedPlaylistId == m_yearNodeId) { // a YEAR item was selected selectedYearIndexRow = m_lastClickedIndex.row(); } else if (playlistId == lastClickedPlaylistId && @@ -668,7 +666,7 @@ void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newN void SetlogFeature::activate() { // The root item was clicked, so actuvate the current playlist. m_lastClickedIndex = m_pSidebarModel->getRootIndex(); - activatePlaylist(m_playlistId); + activatePlaylist(m_currentPlaylistId); } void SetlogFeature::activateChild(const QModelIndex& index) { @@ -683,7 +681,7 @@ void SetlogFeature::activateChild(const QModelIndex& index) { emit saveModelState(); m_pPlaylistTableModel->setTableModel(playlistId); emit showTrackModel(m_pPlaylistTableModel); - if (playlistId == m_placeholderId) { + if (playlistId == m_yearNodeId) { // Disable search and cover art for YEAR items emit disableSearch(); emit enableCoverArtDisplay(false); @@ -714,7 +712,7 @@ void SetlogFeature::activatePlaylist(int playlistId) { // redundant // activateChild(index); - if (playlistId == m_placeholderId) { + if (playlistId == m_yearNodeId) { // Disable search and cover art for YEAR items emit disableSearch(); emit enableCoverArtDisplay(false); diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 92b3d0c14ec..4368b33c013 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -58,8 +58,8 @@ class SetlogFeature : public BasePlaylistFeature { QAction* m_pUnlockAllChildPlaylists; QAction* m_pDeleteAllChildPlaylists; - int m_playlistId; - int m_placeholderId; + int m_currentPlaylistId; + int m_yearNodeId; QPointer m_libraryWidget; Library* m_pLibrary; From 160928c7523f04449e0bdf2d4937d71547edd930 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Sun, 21 May 2023 23:54:27 +0200 Subject: [PATCH 03/11] Playlists: keep previous sidebar selection (other feature or root item) --- src/library/trackset/playlistfeature.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/trackset/playlistfeature.cpp b/src/library/trackset/playlistfeature.cpp index 435442a31f4..a9142bb738d 100644 --- a/src/library/trackset/playlistfeature.cpp +++ b/src/library/trackset/playlistfeature.cpp @@ -296,7 +296,7 @@ void PlaylistFeature::slotPlaylistTableChanged(int playlistId) { clearChildModel(); QModelIndex newIndex = constructChildModel(selectedPlaylistId); - if (newIndex.isValid()) { + if (selectedPlaylistId != kInvalidPlaylistId && newIndex.isValid()) { // If a child index was selected and we got a new valid index select that. // Else (root item was selected or for some reason no index could be created) // there's nothing to do: either no child was selected earlier, or the root From ae493827345578cf3ebf12532ce1b4e810b786ec Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 00:11:18 +0200 Subject: [PATCH 04/11] History: fix debug output --- src/library/trackset/baseplaylistfeature.cpp | 5 +++-- src/library/trackset/setlogfeature.cpp | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 23c8d667195..2fd95eb9bda 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -213,11 +213,11 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { } void BasePlaylistFeature::activatePlaylist(int playlistId) { + // qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId << index; VERIFY_OR_DEBUG_ASSERT(playlistId != kInvalidPlaylistId) { return; } QModelIndex index = indexFromPlaylistId(playlistId); - //qDebug() << "BasePlaylistFeature::activatePlaylist()" << playlistId << index; VERIFY_OR_DEBUG_ASSERT(index.isValid()) { return; } @@ -718,7 +718,8 @@ void BasePlaylistFeature::htmlLinkClicked(const QUrl& link) { } void BasePlaylistFeature::updateChildModel(const QSet& playlistIds) { - // qDebug() << "BasePlaylistFeature::updateChildModel"; + // qDebug() << "BasePlaylistFeature::updateChildModel() for" + // << playlistIds.count() << "playlist(s)"; if (playlistIds.isEmpty()) { return; } diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index ec954b2eccc..b5b0695fb11 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -215,6 +215,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex /// Use a custom model in the history for grouping by year /// @param selectedId row which should be selected QModelIndex SetlogFeature::constructChildModel(int selectedId) { + // qDebug() << "SetlogFeature::constructChildModel() id:" << selectedId; // Setup the sidebar playlist model QSqlTableModel playlistTableModel(this, m_pLibrary->trackCollectionManager()->internalCollection()->database()); @@ -362,7 +363,7 @@ void SetlogFeature::slotGetNewPlaylist() { } void SetlogFeature::slotJoinWithPrevious() { - //qDebug() << "slotJoinWithPrevious() row:" << m_lastRightClickedIndex.data(); + // qDebug() << "SetlogFeature::slotJoinWithPrevious() row:" << m_lastRightClickedIndex.data(); if (m_lastRightClickedIndex.isValid()) { int currentPlaylistId = m_playlistDao.getPlaylistIdFromName( @@ -583,7 +584,7 @@ void SetlogFeature::slotPlayingTrackChanged(TrackPointer currentPlayingTrack) { } void SetlogFeature::slotPlaylistTableChanged(int playlistId) { - //qDebug() << "updateChildModel() playlistId:" << playlistId; + // qDebug() << "SetlogFeature::slotPlaylistTableChanged() id:" << playlistId; PlaylistDAO::HiddenType type = m_playlistDao.getHiddenType(playlistId); if (type != PlaylistDAO::PLHT_SET_LOG && type != PlaylistDAO::PLHT_UNKNOWN) { // deleted Playlist @@ -645,7 +646,8 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { } void SetlogFeature::slotPlaylistContentOrLockChanged(const QSet& playlistIds) { - // qDebug() << "slotPlaylistContentOrLockChanged() playlistId:" << playlistId; + // qDebug() << "slotPlaylistContentOrLockChanged() for" + // << playlistIds.count() << "playlist(s)"; QSet idsToBeUpdated; for (const auto playlistId : qAsConst(playlistIds)) { if (m_playlistDao.getHiddenType(playlistId) == PlaylistDAO::PLHT_SET_LOG) { From 18409a73dd183b887168d74d8b7afe182403a849 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 22:36:54 +0200 Subject: [PATCH 05/11] History (cleanup): remove redundant menu action setup steps --- src/library/trackset/setlogfeature.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index b5b0695fb11..a94ee6a8078 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -162,13 +162,6 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex return; } - bool locked = m_playlistDao.isPlaylistLocked(playlistId); - m_pDeletePlaylistAction->setEnabled(!locked); - m_pRenamePlaylistAction->setEnabled(!locked); - m_pJoinWithPreviousAction->setEnabled(!locked); - - m_pLockPlaylistAction->setText(locked ? tr("Unlock") : tr("Lock")); - QMenu menu(m_pSidebarWidget); if (playlistId == m_yearNodeId) { // this is a YEAR item @@ -206,6 +199,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex menu.addSeparator(); menu.addAction(m_pExportPlaylistAction); } + menu.exec(globalPos); } From 93881e802efb1b96407e32fc47cba8c939842a73 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 00:23:29 +0200 Subject: [PATCH 06/11] History: keep selection when merging playlists Note this is just a workaround for an inconsistency created by slotJoinWithPrevious. That slot uses m_pPlaylistTableModel to load a playlist while there is a different sidebar item selected. --- src/library/trackset/setlogfeature.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index a94ee6a8078..eba0366b358 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -591,7 +591,7 @@ void SetlogFeature::slotPlaylistTableChanged(int playlistId) { bool rootWasSelected = false; if (isChildIndexSelectedInSidebar(m_lastClickedIndex)) { // a child index was selected (actual playlist or YEAR item) - int lastClickedPlaylistId = m_pPlaylistTableModel->getPlaylist(); + int lastClickedPlaylistId = playlistIdFromIndex(m_lastClickedIndex); if (lastClickedPlaylistId == m_yearNodeId) { // a YEAR item was selected selectedYearIndexRow = m_lastClickedIndex.row(); @@ -660,8 +660,9 @@ void SetlogFeature::slotPlaylistTableRenamed(int playlistId, const QString& newN } void SetlogFeature::activate() { - // The root item was clicked, so actuvate the current playlist. + // The root item was clicked, so activate the current playlist. m_lastClickedIndex = m_pSidebarModel->getRootIndex(); + m_lastRightClickedIndex = QModelIndex(); activatePlaylist(m_currentPlaylistId); } @@ -704,9 +705,8 @@ void SetlogFeature::activatePlaylist(int playlistId) { // selected, that would switch to the 'current' child. if (m_lastClickedIndex != m_pSidebarModel->getRootIndex()) { m_lastClickedIndex = index; + m_lastRightClickedIndex = QModelIndex(); emit featureSelect(this, index); - // redundant - // activateChild(index); if (playlistId == m_yearNodeId) { // Disable search and cover art for YEAR items From 9eacc4dd80704349863b8ae557d21aac6ab1208b Mon Sep 17 00:00:00 2001 From: ronso0 Date: Thu, 25 May 2023 22:06:49 +0200 Subject: [PATCH 07/11] History slotJoinWithPrevious(): early return + debug assert --- src/library/trackset/setlogfeature.cpp | 90 ++++++++++++++------------ 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index eba0366b358..d00fc97ab15 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -358,55 +358,59 @@ void SetlogFeature::slotGetNewPlaylist() { void SetlogFeature::slotJoinWithPrevious() { // qDebug() << "SetlogFeature::slotJoinWithPrevious() row:" << m_lastRightClickedIndex.data(); + if (!m_lastRightClickedIndex.isValid()) { + return; + } + + int clickedPlaylistId = m_playlistDao.getPlaylistIdFromName( + m_lastRightClickedIndex.data().toString()); + if (clickedPlaylistId == kInvalidPlaylistId) { + return; + } + + bool locked = m_playlistDao.isPlaylistLocked(clickedPlaylistId); + if (locked) { + qDebug() << "Aborting playlist join because playlist" + << clickedPlaylistId << "is locked."; + return; + } - if (m_lastRightClickedIndex.isValid()) { - int currentPlaylistId = m_playlistDao.getPlaylistIdFromName( - m_lastRightClickedIndex.data().toString()); + // Add every track from right-clicked playlist to that with the next smaller ID + int previousPlaylistId = m_playlistDao.getPreviousPlaylist( + clickedPlaylistId, PlaylistDAO::PLHT_SET_LOG); + if (previousPlaylistId == kInvalidPlaylistId) { + qDebug() << "Aborting playlist join because playlist" + << clickedPlaylistId << "because there's no previous playlist."; + return; + } - if (currentPlaylistId >= 0) { - bool locked = m_playlistDao.isPlaylistLocked(currentPlaylistId); + m_pPlaylistTableModel->setTableModel(previousPlaylistId); - if (locked) { - qDebug() << "Skipping playlist deletion because playlist" - << currentPlaylistId << "is locked."; - return; - } + if (clickedPlaylistId == m_currentPlaylistId) { + // mark all the Tracks in the previous Playlist as played - // Add every track from right-clicked playlist to that with the next smaller ID - int previousPlaylistId = m_playlistDao.getPreviousPlaylist( - currentPlaylistId, PlaylistDAO::PLHT_SET_LOG); - if (previousPlaylistId >= 0) { - m_pPlaylistTableModel->setTableModel(previousPlaylistId); - - if (currentPlaylistId == m_currentPlaylistId) { - // mark all the Tracks in the previous Playlist as played - - m_pPlaylistTableModel->select(); - int rows = m_pPlaylistTableModel->rowCount(); - for (int i = 0; i < rows; ++i) { - QModelIndex index = m_pPlaylistTableModel->index(i, 0); - if (index.isValid()) { - TrackPointer track = - m_pPlaylistTableModel->getTrack(index); - // Do not update the play count, just set played status. - PlayCounter playCounter(track->getPlayCounter()); - playCounter.triggerLastPlayedNow(); - track->setPlayCounter(playCounter); - } - } - - // Change current setlog - m_currentPlaylistId = previousPlaylistId; - } - qDebug() << "slotJoinWithPrevious() current:" - << currentPlaylistId - << " previous:" << previousPlaylistId; - if (m_playlistDao.copyPlaylistTracks( - currentPlaylistId, previousPlaylistId)) { - m_playlistDao.deletePlaylist(currentPlaylistId); - } + m_pPlaylistTableModel->select(); + int rows = m_pPlaylistTableModel->rowCount(); + for (int i = 0; i < rows; ++i) { + QModelIndex index = m_pPlaylistTableModel->index(i, 0); + if (index.isValid()) { + TrackPointer track = m_pPlaylistTableModel->getTrack(index); + DEBUG_ASSERT(track != nullptr); + // Do not update the play count, just set played status. + PlayCounter playCounter(track->getPlayCounter()); + playCounter.triggerLastPlayedNow(); + track->setPlayCounter(playCounter); } } + + // Change current setlog + m_currentPlaylistId = previousPlaylistId; + } + qDebug() << "slotJoinWithPrevious() current:" + << clickedPlaylistId + << " previous:" << previousPlaylistId; + if (m_playlistDao.copyPlaylistTracks(clickedPlaylistId, previousPlaylistId)) { + m_playlistDao.deletePlaylist(clickedPlaylistId); } } From 2fc05e2beb9244a8456fa1089260799f64452f0f Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 00:32:49 +0200 Subject: [PATCH 08/11] History slotJoinWithPrevious(): use a temporary table model --- src/library/trackset/baseplaylistfeature.cpp | 2 +- src/library/trackset/setlogfeature.cpp | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 2fd95eb9bda..2dddf35abcb 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -468,7 +468,7 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlistFile, // Create a temporary PlaylistTableModel for the Playlist the entries shall be imported to. // This is used as a proxy object to write to the database. - // We cannot use m_pPlaylistTableModel since it might have another playlist selected which + // We cannot use m_pPlaylistTableModel since it might have another playlist selected which // is not the playlist that received the right-click. QScopedPointer pPlaylistTableModel( new PlaylistTableModel(this, diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index d00fc97ab15..5eae424e5d7 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -362,8 +362,7 @@ void SetlogFeature::slotJoinWithPrevious() { return; } - int clickedPlaylistId = m_playlistDao.getPlaylistIdFromName( - m_lastRightClickedIndex.data().toString()); + int clickedPlaylistId = playlistIdFromIndex(m_lastRightClickedIndex); if (clickedPlaylistId == kInvalidPlaylistId) { return; } @@ -384,17 +383,22 @@ void SetlogFeature::slotJoinWithPrevious() { return; } - m_pPlaylistTableModel->setTableModel(previousPlaylistId); + // Right-clicked playlist may not be loaded. Use a temporary model to + // keep sidebar selection and table view in sync + QScopedPointer pPlaylistTableModel( + new PlaylistTableModel(this, + m_pLibrary->trackCollectionManager(), + "mixxx.db.model.playlist_export")); + pPlaylistTableModel->setTableModel(previousPlaylistId); if (clickedPlaylistId == m_currentPlaylistId) { // mark all the Tracks in the previous Playlist as played - - m_pPlaylistTableModel->select(); - int rows = m_pPlaylistTableModel->rowCount(); + pPlaylistTableModel->select(); + int rows = pPlaylistTableModel->rowCount(); for (int i = 0; i < rows; ++i) { - QModelIndex index = m_pPlaylistTableModel->index(i, 0); + QModelIndex index = pPlaylistTableModel->index(i, 0); if (index.isValid()) { - TrackPointer track = m_pPlaylistTableModel->getTrack(index); + TrackPointer track = pPlaylistTableModel->getTrack(index); DEBUG_ASSERT(track != nullptr); // Do not update the play count, just set played status. PlayCounter playCounter(track->getPlayCounter()); From 38752b77241d85988fbb3ec0ae799206665ef6c2 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 01:26:17 +0200 Subject: [PATCH 09/11] playlist models: rename setTableModel > selectPlaylist --- src/library/autodj/autodjprocessor.cpp | 2 +- src/library/banshee/bansheefeature.cpp | 6 +++--- src/library/banshee/bansheeplaylistmodel.cpp | 4 ++-- src/library/banshee/bansheeplaylistmodel.h | 2 +- src/library/playlisttablemodel.cpp | 4 ++-- src/library/playlisttablemodel.h | 2 +- src/library/trackset/baseplaylistfeature.cpp | 10 +++++----- src/library/trackset/setlogfeature.cpp | 6 +++--- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index e55e9f6f9ab..57014e06412 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -122,7 +122,7 @@ AutoDJProcessor::AutoDJProcessor( m_transitionTime(kTransitionPreferenceDefault) { m_pAutoDJTableModel = new PlaylistTableModel(this, pTrackCollectionManager, "mixxx.db.model.autodj"); - m_pAutoDJTableModel->setTableModel(iAutoDJPlaylistId); + m_pAutoDJTableModel->selectPlaylist(iAutoDJPlaylistId); m_pAutoDJTableModel->select(); m_pShufflePlaylist = new ControlPushButton( diff --git a/src/library/banshee/bansheefeature.cpp b/src/library/banshee/bansheefeature.cpp index 860b0d555f6..df329cf19f3 100644 --- a/src/library/banshee/bansheefeature.cpp +++ b/src/library/banshee/bansheefeature.cpp @@ -110,7 +110,7 @@ void BansheeFeature::activate() { emit featureLoadingFinished(this); } - m_pBansheePlaylistModel->setTableModel(0); // Gets the master playlist + m_pBansheePlaylistModel->selectPlaylist(0); // Loads the master playlist emit showTrackModel(m_pBansheePlaylistModel); emit enableCoverArtDisplay(false); } @@ -120,7 +120,7 @@ void BansheeFeature::activateChild(const QModelIndex& index) { int playlistID = item->getData().toInt(); if (playlistID > 0) { qDebug() << "Activating " << item->getLabel(); - m_pBansheePlaylistModel->setTableModel(playlistID); + m_pBansheePlaylistModel->selectPlaylist(playlistID); emit showTrackModel(m_pBansheePlaylistModel); emit enableCoverArtDisplay(false); } @@ -141,7 +141,7 @@ void BansheeFeature::appendTrackIdsFromRightClickIndex(QList* trackIds, new BansheePlaylistModel(this, m_pLibrary->trackCollectionManager(), &m_connection); - pPlaylistModelToAdd->setTableModel(playlistID); + pPlaylistModelToAdd->selectPlaylist(playlistID); pPlaylistModelToAdd->select(); // Copy Tracks diff --git a/src/library/banshee/bansheeplaylistmodel.cpp b/src/library/banshee/bansheeplaylistmodel.cpp index 0115f1e63f1..8326d91fc16 100644 --- a/src/library/banshee/bansheeplaylistmodel.cpp +++ b/src/library/banshee/bansheeplaylistmodel.cpp @@ -86,8 +86,8 @@ void BansheePlaylistModel::dropTempTable() { } } -void BansheePlaylistModel::setTableModel(int playlistId) { - //qDebug() << "BansheePlaylistModel::setTableModel" << this << playlistId; +void BansheePlaylistModel::selectPlaylist(int playlistId) { + // qDebug() << "BansheePlaylistModel::selectPlaylist" << this << playlistId; if (m_playlistId == playlistId) { qDebug() << "Already focused on playlist " << playlistId; return; diff --git a/src/library/banshee/bansheeplaylistmodel.h b/src/library/banshee/bansheeplaylistmodel.h index 261a6733632..868a85cfb01 100644 --- a/src/library/banshee/bansheeplaylistmodel.h +++ b/src/library/banshee/bansheeplaylistmodel.h @@ -16,7 +16,7 @@ class BansheePlaylistModel final : public BaseSqlTableModel { BansheePlaylistModel(QObject* pParent, TrackCollectionManager* pTrackCollectionManager, BansheeDbConnection* pConnection); ~BansheePlaylistModel() final; - void setTableModel(int playlistId); + void selectPlaylist(int playlistId); TrackPointer getTrack(const QModelIndex& index) const final; TrackId getTrackId(const QModelIndex& index) const final; diff --git a/src/library/playlisttablemodel.cpp b/src/library/playlisttablemodel.cpp index db7f37a0edd..94ec2e1db46 100644 --- a/src/library/playlisttablemodel.cpp +++ b/src/library/playlisttablemodel.cpp @@ -121,8 +121,8 @@ void PlaylistTableModel::initSortColumnMapping() { } } -void PlaylistTableModel::setTableModel(int playlistId) { - //qDebug() << "PlaylistTableModel::setTableModel" << playlistId; +void PlaylistTableModel::selectPlaylist(int playlistId) { + // qDebug() << "PlaylistTableModel::selectPlaylist" << playlistId; if (m_iPlaylistId == playlistId) { qDebug() << "Already focused on playlist " << playlistId; return; diff --git a/src/library/playlisttablemodel.h b/src/library/playlisttablemodel.h index 6aa31ba06d9..bc68c9cf7c8 100644 --- a/src/library/playlisttablemodel.h +++ b/src/library/playlisttablemodel.h @@ -10,7 +10,7 @@ class PlaylistTableModel final : public TrackSetTableModel { PlaylistTableModel(QObject* parent, TrackCollectionManager* pTrackCollectionManager, const char* settingsNamespace, bool keepDeletedTracks = false); ~PlaylistTableModel() final = default; - void setTableModel(int playlistId = -1); + void selectPlaylist(int playlistId = -1 /* kInvalidPlaylistId */); int getPlaylist() const { return m_iPlaylistId; } diff --git a/src/library/trackset/baseplaylistfeature.cpp b/src/library/trackset/baseplaylistfeature.cpp index 2dddf35abcb..30ca7cc4a98 100644 --- a/src/library/trackset/baseplaylistfeature.cpp +++ b/src/library/trackset/baseplaylistfeature.cpp @@ -207,7 +207,7 @@ void BasePlaylistFeature::activateChild(const QModelIndex& index) { m_lastClickedIndex = index; m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); - m_pPlaylistTableModel->setTableModel(playlistId); + m_pPlaylistTableModel->selectPlaylist(playlistId); emit showTrackModel(m_pPlaylistTableModel); emit enableCoverArtDisplay(true); } @@ -224,7 +224,7 @@ void BasePlaylistFeature::activatePlaylist(int playlistId) { m_lastClickedIndex = index; m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); - m_pPlaylistTableModel->setTableModel(playlistId); + m_pPlaylistTableModel->selectPlaylist(playlistId); emit showTrackModel(m_pPlaylistTableModel); emit enableCoverArtDisplay(true); // Update selection @@ -474,7 +474,7 @@ void BasePlaylistFeature::slotImportPlaylistFile(const QString& playlistFile, new PlaylistTableModel(this, m_pLibrary->trackCollectionManager(), "mixxx.db.model.playlist_export")); - pPlaylistTableModel->setTableModel(playlistId); + pPlaylistTableModel->selectPlaylist(playlistId); pPlaylistTableModel->setSort( pPlaylistTableModel->fieldIndex( ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), @@ -583,7 +583,7 @@ void BasePlaylistFeature::slotExportPlaylist() { "mixxx.db.model.playlist_export")); emit saveModelState(); - pPlaylistTableModel->setTableModel(playlistId); + pPlaylistTableModel->selectPlaylist(playlistId); pPlaylistTableModel->setSort( pPlaylistTableModel->fieldIndex( ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), @@ -629,7 +629,7 @@ void BasePlaylistFeature::slotExportTrackFiles() { "mixxx.db.model.playlist_export")); emit saveModelState(); - pPlaylistTableModel->setTableModel(playlistId); + pPlaylistTableModel->selectPlaylist(playlistId); pPlaylistTableModel->setSort(pPlaylistTableModel->fieldIndex( ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION), Qt::AscendingOrder); diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index 5eae424e5d7..d432f06fbe5 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -389,7 +389,7 @@ void SetlogFeature::slotJoinWithPrevious() { new PlaylistTableModel(this, m_pLibrary->trackCollectionManager(), "mixxx.db.model.playlist_export")); - pPlaylistTableModel->setTableModel(previousPlaylistId); + pPlaylistTableModel->selectPlaylist(previousPlaylistId); if (clickedPlaylistId == m_currentPlaylistId) { // mark all the Tracks in the previous Playlist as played @@ -684,7 +684,7 @@ void SetlogFeature::activateChild(const QModelIndex& index) { m_lastClickedIndex = index; m_lastRightClickedIndex = QModelIndex(); emit saveModelState(); - m_pPlaylistTableModel->setTableModel(playlistId); + m_pPlaylistTableModel->selectPlaylist(playlistId); emit showTrackModel(m_pPlaylistTableModel); if (playlistId == m_yearNodeId) { // Disable search and cover art for YEAR items @@ -705,7 +705,7 @@ void SetlogFeature::activatePlaylist(int playlistId) { return; } emit saveModelState(); - m_pPlaylistTableModel->setTableModel(playlistId); + m_pPlaylistTableModel->selectPlaylist(playlistId); emit showTrackModel(m_pPlaylistTableModel); // Update sidebar selection only if this is a child, incl. current playlist // and YEAR nodes. From 612ffb7b4ba41625fd2977e8d50e9f73794a1cd5 Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 02:13:50 +0200 Subject: [PATCH 10/11] Track: add updatePlayedStatusKeepPlayCount(bool) --- src/library/trackset/setlogfeature.cpp | 8 +++----- src/track/track.cpp | 10 ++++++++++ src/track/track.h | 2 ++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index d432f06fbe5..a1d1541f468 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -398,12 +398,10 @@ void SetlogFeature::slotJoinWithPrevious() { for (int i = 0; i < rows; ++i) { QModelIndex index = pPlaylistTableModel->index(i, 0); if (index.isValid()) { - TrackPointer track = pPlaylistTableModel->getTrack(index); - DEBUG_ASSERT(track != nullptr); + TrackPointer pTrack = pPlaylistTableModel->getTrack(index); + DEBUG_ASSERT(pTrack != nullptr); // Do not update the play count, just set played status. - PlayCounter playCounter(track->getPlayCounter()); - playCounter.triggerLastPlayedNow(); - track->setPlayCounter(playCounter); + pTrack->updatePlayedStatusKeepPlayCount(true); } } diff --git a/src/track/track.cpp b/src/track/track.cpp index d73054b7d98..fbc9a2f9ac3 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -755,6 +755,16 @@ void Track::updatePlayCounter(bool bPlayed) { } } +void Track::updatePlayedStatusKeepPlayCount(bool bPlayed) { + auto locked = lockMutex(&m_qMutex); + PlayCounter playCounter(m_record.getPlayCounter()); + playCounter.setPlayedFlag(bPlayed); + if (compareAndSet(m_record.ptrPlayCounter(), playCounter)) { + markDirtyAndUnlock(&locked); + emit timesPlayedChanged(); + } +} + mixxx::RgbColor::optional_t Track::getColor() const { const auto locked = lockMutex(&m_qMutex); return m_record.getColor(); diff --git a/src/track/track.h b/src/track/track.h index e3a960446be..76554eee884 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -248,6 +248,8 @@ class Track : public QObject { } // Sets played status and increments or decrements the play count void updatePlayCounter(bool bPlayed = true); + // Sets played status but leaves play count untouched + void updatePlayedStatusKeepPlayCount(bool bPlayed); // Only required for the times_played property int getTimesPlayed() const { From a58f192479c2f0b29aecba1a0a7c5636ad52858c Mon Sep 17 00:00:00 2001 From: ronso0 Date: Fri, 26 May 2023 02:17:21 +0200 Subject: [PATCH 11/11] History: add "Mark all tracks as played" --- src/library/trackset/setlogfeature.cpp | 45 +++++++++++++++++++++++++- src/library/trackset/setlogfeature.h | 2 ++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/library/trackset/setlogfeature.cpp b/src/library/trackset/setlogfeature.cpp index a1d1541f468..83bc757cc70 100644 --- a/src/library/trackset/setlogfeature.cpp +++ b/src/library/trackset/setlogfeature.cpp @@ -64,6 +64,12 @@ SetlogFeature::SetlogFeature( this, &SetlogFeature::slotJoinWithPrevious); + m_pMarkTracksPlayedAction = new QAction(tr("Mark all tracks played)"), this); + connect(m_pMarkTracksPlayedAction, + &QAction::triggered, + this, + &SetlogFeature::slotMarkAllTracksPlayed); + m_pStartNewPlaylist = new QAction(tr("Finish current and start new"), this); connect(m_pStartNewPlaylist, &QAction::triggered, @@ -175,8 +181,8 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex m_pDeletePlaylistAction->setEnabled(!locked); m_pRenamePlaylistAction->setEnabled(!locked); m_pJoinWithPreviousAction->setEnabled(!locked); - m_pLockPlaylistAction->setText(locked ? tr("Unlock") : tr("Lock")); + menu.addAction(m_pAddToAutoDJAction); menu.addAction(m_pAddToAutoDJTopAction); menu.addSeparator(); @@ -185,6 +191,7 @@ void SetlogFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex // Todays playlist should not be locked or deleted menu.addAction(m_pDeletePlaylistAction); menu.addAction(m_pLockPlaylistAction); + menu.addAction(m_pMarkTracksPlayedAction); } if (index.sibling(index.row() + 1, index.column()).isValid()) { // The very first (oldest) setlog cannot be joint @@ -416,6 +423,42 @@ void SetlogFeature::slotJoinWithPrevious() { } } +void SetlogFeature::slotMarkAllTracksPlayed() { + // qDebug() << "SetlogFeature::slotMarkAllTracksPlayed()"; + if (!m_lastRightClickedIndex.isValid()) { + return; + } + + int clickedPlaylistId = playlistIdFromIndex(m_lastRightClickedIndex); + if (clickedPlaylistId == kInvalidPlaylistId) { + return; + } + + if (clickedPlaylistId == m_currentPlaylistId) { + return; + } + + // Right-clicked playlist may not be loaded. Use a temporary model to + // keep sidebar selection and table view in sync + QScopedPointer pPlaylistTableModel( + new PlaylistTableModel(this, + m_pLibrary->trackCollectionManager(), + "mixxx.db.model.playlist_export")); + pPlaylistTableModel->selectPlaylist(clickedPlaylistId); + // mark all the Tracks in the previous Playlist as played + pPlaylistTableModel->select(); + int rows = pPlaylistTableModel->rowCount(); + for (int i = 0; i < rows; ++i) { + QModelIndex index = pPlaylistTableModel->index(i, 0); + if (index.isValid()) { + TrackPointer pTrack = pPlaylistTableModel->getTrack(index); + DEBUG_ASSERT(pTrack != nullptr); + // Do not update the play count, just set played status. + pTrack->updatePlayedStatusKeepPlayCount(true); + } + } +} + void SetlogFeature::slotLockAllChildPlaylists() { lockOrUnlockAllChildPlaylists(true); } diff --git a/src/library/trackset/setlogfeature.h b/src/library/trackset/setlogfeature.h index 4368b33c013..c25babad3cf 100644 --- a/src/library/trackset/setlogfeature.h +++ b/src/library/trackset/setlogfeature.h @@ -27,6 +27,7 @@ class SetlogFeature : public BasePlaylistFeature { void onRightClick(const QPoint& globalPos) override; void onRightClickChild(const QPoint& globalPos, const QModelIndex& index) override; void slotJoinWithPrevious(); + void slotMarkAllTracksPlayed(); void slotLockAllChildPlaylists(); void slotUnlockAllChildPlaylists(); void slotDeletePlaylist() override; @@ -53,6 +54,7 @@ class SetlogFeature : public BasePlaylistFeature { std::list m_recentTracks; QAction* m_pJoinWithPreviousAction; + QAction* m_pMarkTracksPlayedAction; QAction* m_pStartNewPlaylist; QAction* m_pLockAllChildPlaylists; QAction* m_pUnlockAllChildPlaylists;