[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: [amarok] /: Playlist{Actions, Controller}: refactor code, fix queueing
From: Matěj_Laitl <matej () laitl ! cz>
Date: 2013-05-16 15:51:46
Message-ID: 20130516155146.E2A3FA605B () git ! kde ! org
[Download RAW message or body]
Git commit 6a3f1c67f761803479f87a3080415057e61c49be by Matěj Laitl.
Committed on 16/05/2013 at 16:46.
Pushed by laitl into branch 'master'.
Playlist{Actions,Controller}: refactor code, fix queueing
The actual fix for bug was to remove the isPlayable() check in
TrackNavigator, however this fixes more potential problems (like the
queueing being affected by the current playlist filter) and cleans up
the code.
BUGFIXES:
* Fix `amarok --queue` which didn't actually queue the tracks.
BUG: 317385
FIXED-IN: 2.8
M +1 -0 ChangeLog
M +13 -11 src/playlist/PlaylistActions.cpp
M +12 -2 src/playlist/PlaylistActions.h
M +44 -64 src/playlist/PlaylistController.cpp
M +1 -13 src/playlist/navigators/TrackNavigator.cpp
M +0 -1 src/playlist/navigators/TrackNavigator.h
http://commits.kde.org/amarok/6a3f1c67f761803479f87a3080415057e61c49be
diff --git a/ChangeLog b/ChangeLog
index c21adee..1576c1b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -52,6 +52,7 @@ VERSION 2.8-Beta 1
not to fool users.
BUGFIXES:
+ * Fix `amarok --queue` which didn't actually queue the tracks. (BR 317385)
* Fix suboptimal initial MusicBrainz tag dialog size by remembering it. (BR \
269454)
* Fix file-browser becoming empty when a file was moved to trash. (BR 317944)
* Fix `amarok --play file.mp3` option didn't actually start playback.
diff --git a/src/playlist/PlaylistActions.cpp b/src/playlist/PlaylistActions.cpp
index b36794d..1decce7 100644
--- a/src/playlist/PlaylistActions.cpp
+++ b/src/playlist/PlaylistActions.cpp
@@ -185,7 +185,7 @@ Playlist::Actions::play()
}
void
-Playlist::Actions::play( const QModelIndex& index )
+Playlist::Actions::play( const QModelIndex &index )
{
DEBUG_BLOCK
@@ -388,22 +388,24 @@ Playlist::Actions::dequeue( quint64 id )
}
void
-Playlist::Actions::queue( QList<int> rows )
+Playlist::Actions::queue( const QList<int> &rows )
{
- DEBUG_BLOCK
-
+ QList<quint64> ids;
foreach( int row, rows )
- {
- quint64 id = The::playlist()->idAt( row );
- debug() << "About to queue proxy row"<< row;
- m_navigator->queueId( id );
- }
- if ( !rows.isEmpty() )
+ ids << The::playlist()->idAt( row );
+ queue( ids );
+}
+
+void
+Playlist::Actions::queue( const QList<quint64> &ids )
+{
+ m_navigator->queueIds( ids );
+ if ( !ids.isEmpty() )
Playlist::ModelStack::instance()->bottom()->emitQueueChanged();
}
void
-Playlist::Actions::dequeue( QList<int> rows )
+Playlist::Actions::dequeue( const QList<int> &rows )
{
DEBUG_BLOCK
diff --git a/src/playlist/PlaylistActions.h b/src/playlist/PlaylistActions.h
index 75b71fd..dee8793 100644
--- a/src/playlist/PlaylistActions.h
+++ b/src/playlist/PlaylistActions.h
@@ -121,8 +121,18 @@ public slots:
void playlistModeChanged();
void repopulateDynamicPlaylist();
- void queue( QList<int> rows );
- void dequeue( QList<int> rows );
+
+ /**
+ * Adds a list of top playlist model rows to the queue.
+ */
+ void queue( const QList<int> &rows );
+
+ /**
+ * Adds a list of playlist item unique ids to the queue.
+ */
+ void queue( const QList<quint64> &ids );
+
+ void dequeue( const QList<int> &rows );
void restoreDefaultPlaylist();
/**
diff --git a/src/playlist/PlaylistController.cpp \
b/src/playlist/PlaylistController.cpp index e8e6839..8d7ca5c 100644
--- a/src/playlist/PlaylistController.cpp
+++ b/src/playlist/PlaylistController.cpp
@@ -21,10 +21,10 @@
* this program. If not, see <http://www.gnu.org/licenses/>. \
*
****************************************************************************************/
-#include "PlaylistController.h"
-
#define DEBUG_PREFIX "Playlist::Controller"
+#include "PlaylistController.h"
+
#include "amarokconfig.h"
#include "core/support/Debug.h"
#include "EngineController.h"
@@ -121,72 +121,63 @@ Controller::insertOptioned( Meta::TrackList list, AddOptions \
options ) QMutableListIterator<Meta::TrackPtr> i( list );
while( i.hasNext() )
{
- i.next();
- debug()<<"About to check m_bottomModel->containsTrack()";
- if( m_bottomModel->containsTrack( i.value() ) )
+ if( m_bottomModel->containsTrack( i.next() ) )
i.remove();
}
}
- int topModelInsertRow;
- int visibleInsertedRowCount;
+ QString actionName = i18nc( "name of the action in undo stack", "Add tracks to \
playlist" ); + if( options & Queue )
+ actionName = i18nc( "name of the action in undo stack", "Queue tracks" );
+ if( options & Replace )
+ actionName = i18nc( "name of the action in undo stack", "Replace playlist" \
); + m_undoStack->beginMacro( actionName );
+
if( options & Replace )
{
- debug()<<"Replace";
emit replacingPlaylist(); //make sure that we clear filters
-
- m_undoStack->beginMacro( "Replace playlist" ); // TODO: does this need to be \
internationalized? clear();
-
//make sure that we turn off dynamic mode.
Amarok::actionCollection()->action( "disable_dynamic" )->trigger();
-
- int bottomModelInsertRow = insertionTopRowToBottom( 0 );
- insertionHelper( bottomModelInsertRow, list );
- topModelInsertRow = m_topModel->rowFromBottomModel( bottomModelInsertRow );
- m_undoStack->endMacro();
- visibleInsertedRowCount = m_topModel->qaim()->rowCount(); // simple
}
- else if( options & Queue )
- {
- debug()<<"Queue";
-
- int oldVisibleRowCount = m_topModel->qaim()->rowCount();
- topModelInsertRow = m_topModel->activeRow() + 1;
- while( m_topModel->queuePositionOfRow( topModelInsertRow ) )
- topModelInsertRow++; // We want to add the newly queued items after \
any items which are already queued
-
- int bottomModelInsertRow = insertionTopRowToBottom( topModelInsertRow );
- insertionHelper( bottomModelInsertRow, list );
- topModelInsertRow = m_topModel->rowFromBottomModel( bottomModelInsertRow );
- visibleInsertedRowCount = m_topModel->qaim()->rowCount() - \
oldVisibleRowCount;
-
- // Construct list of rows to be queued
- QList<int> topModelRows;
- for( int bottomModelRow = bottomModelInsertRow; bottomModelRow < \
bottomModelInsertRow + list.size(); ++bottomModelRow ) + int bottomModelRowCount = \
m_bottomModel->qaim()->rowCount(); + int bottomModelInsertRow;
+ if( options & Queue )
+ {
+ // queue is a list of playlist item ids
+ QQueue<quint64> queue = Actions::instance()->queue();
+ if( !queue.isEmpty() )
{
- quint64 playlistItemId = m_bottomModel->idAt( bottomModelRow );
- int topModelRow = m_topModel->rowForId( playlistItemId );
- if( topModelRow >= 0 ) // If a filter doesn't hide the item...
- topModelRows << topModelRow;
+ int lastQueueRow = m_bottomModel->rowForId( queue.last() );
+ debug() << "queue is:" << queue << "lastQueueRow:" << lastQueueRow;
+ bottomModelInsertRow = ( lastQueueRow >= 0 ) ? lastQueueRow + 1 : \
bottomModelRowCount; + }
+ else
+ {
+ int activeRow = m_bottomModel->activeRow();
+ bottomModelInsertRow = ( activeRow >= 0 ) ? activeRow + 1 : \
bottomModelRowCount; }
- Actions::instance()->queue( topModelRows );
}
else
- {
- debug()<<"Append";
+ bottomModelInsertRow = m_bottomModel->qaim()->rowCount();
- int oldVisibleRowCount = m_topModel->qaim()->rowCount();
- topModelInsertRow = m_topModel->qaim()->rowCount();
+ int oldVisibleRowCount = m_topModel->qaim()->rowCount();
+ insertionHelper( bottomModelInsertRow, list );
+ int visibleInsertedRowCount = m_topModel->qaim()->rowCount() - \
oldVisibleRowCount;
- int bottomModelInsertRow = insertionTopRowToBottom( topModelInsertRow );
- insertionHelper( bottomModelInsertRow, list );
- topModelInsertRow = m_topModel->rowFromBottomModel( bottomModelInsertRow );
- visibleInsertedRowCount = m_topModel->qaim()->rowCount() - \
oldVisibleRowCount; + if( options & Queue )
+ {
+ // Construct list of rows to be queued
+ QList<quint64> ids;
+ for( int bottomModelRow = bottomModelInsertRow;
+ bottomModelRow < bottomModelInsertRow + list.size(); bottomModelRow++ )
+ {
+ ids << m_bottomModel->idAt( bottomModelRow );
+ }
+ Actions::instance()->queue( ids );
}
-
- debug() << "engine playing?: " << The::engineController()->isPlaying();
+ m_undoStack->endMacro();
bool playNow = false;
if( options & DirectPlay )
@@ -210,7 +201,7 @@ Controller::insertOptioned( Meta::TrackList list, AddOptions \
options )
if ( AmarokConfig::trackProgression() == \
AmarokConfig::EnumTrackProgression::RandomTrack ||
AmarokConfig::trackProgression() == \
AmarokConfig::EnumTrackProgression::RandomAlbum ) fuzz = qrand() % \
visibleInsertedRowCount;
- Actions::instance()->play( topModelInsertRow + fuzz );
+ Actions::instance()->play( m_topModel->rowFromBottomModel( \
bottomModelInsertRow ) + fuzz ); }
emit changed();
@@ -550,29 +541,18 @@ Controller::slotLoaderWithRowFinished( const Meta::TrackList \
&tracks ) int
Controller::insertionTopRowToBottom( int topModelRow )
{
- DEBUG_BLOCK
-
if( ( topModelRow < 0 ) || ( topModelRow > m_topModel->qaim()->rowCount() ) )
{
- error() << "Row number invalid:" << topModelRow;
+ error() << "Row number invalid, using bottom:" << topModelRow;
topModelRow = m_topModel->qaim()->rowCount(); // Failsafe: append.
}
- int bottomModelRow;
if( ModelStack::instance()->sortProxy()->isSorted() )
- {
// if the playlist is sorted there's no point in placing the added tracks at \
any
// specific point in relation to another track, so we just append them.
- debug()<<"SortProxy is SORTED ... so I'll just append.";
- bottomModelRow = m_bottomModel->qaim()->rowCount();
- }
+ return m_bottomModel->qaim()->rowCount();
else
- {
- debug()<<"SortProxy is NOT SORTED ... so I'll take care of the right \
row.";
- bottomModelRow = m_topModel->rowToBottomModel( topModelRow );
- }
-
- return bottomModelRow;
+ return m_topModel->rowToBottomModel( topModelRow );
}
void
diff --git a/src/playlist/navigators/TrackNavigator.cpp \
b/src/playlist/navigators/TrackNavigator.cpp index 75d3cec..7de60fd 100644
--- a/src/playlist/navigators/TrackNavigator.cpp
+++ b/src/playlist/navigators/TrackNavigator.cpp
@@ -43,23 +43,11 @@ Playlist::TrackNavigator::TrackNavigator()
}
void
-Playlist::TrackNavigator::queueId( const quint64 id )
-{
- QList<quint64> ids;
- ids << id;
- queueIds( ids );
-}
-
-void
Playlist::TrackNavigator::queueIds( const QList<quint64> &ids )
{
- Meta::TrackPtr track;
foreach( quint64 id, ids )
{
- track = m_model->trackForId( id );
- if( !track ) // playlist might contain invalid tracks. See BUG: 302607
- continue;
- if( !m_queue.contains( id ) && track->isPlayable() )
+ if( !m_queue.contains( id ) )
m_queue.enqueue( id );
}
}
diff --git a/src/playlist/navigators/TrackNavigator.h \
b/src/playlist/navigators/TrackNavigator.h index a5fb0fc..6e8d1aa 100644
--- a/src/playlist/navigators/TrackNavigator.h
+++ b/src/playlist/navigators/TrackNavigator.h
@@ -93,7 +93,6 @@ namespace Playlist
* Queues the specified id and schedules it to be played.
*/
virtual void queueIds( const QList<quint64> &ids );
- virtual void queueId( const quint64 id );
/**
* Dequeue the specified id from the queue list
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic