extragear/multimedia/amarok/src

Maximilian Kossick maximilian.kossick at googlemail.com
Tue Apr 22 16:33:19 CEST 2008


On Tue, Apr 22, 2008 at 3:07 PM, Dan Meltzer
<parallelgrapefruit at gmail.com> wrote:
> On 4/22/08, Maximilian Kossick <maximilian.kossick at googlemail.com> wrote:
>  > Neither EngineController nor MainWindow is a good place to put this
>  >  signal. I certainly would not look in either class when trying to
>  >  figure out how to love track. We are loving tracks, so the method
>  >  should be associated relatively closely with Meta::Track (is there any
>  >  particular reason that we only allow loving of the current track
>  >  anyway?). It would make sense to put the love() method into
>  >  Meta::Track if all tracks have this capability, but I'd put it into a
>  >  capability instead (and then provide a default implementation of that
>  >  capability for all tracks to use...one can always subclass the default
>  >  implementation to add specific behaviour). Putting love into a
>  >  capability helps us to keep the interface of Meta::Track as simple as
>  >  possible. We'd have to add that capability to all tracks (and access
>  >  that capability wherever we want to love a track), but that's still
>  >  the best solution in my opinion because it does not mix
>  >  responsibilities .
>
>  While I definately agree that mainwindow/enginecontroller probably is
>  not the best place for this, my problem with putting it in Meta::Track
>  is that in addition to the Class-Specific method of loving (maybe
>  toggling a flag in the database for SqlCollection) we also need a way
>  of allowing other parts of the app to listen for "loves".  Last.fm
>  loves would be the clear idea now, but in the future we could just as
>  easily have other competining services that have the same idea of
>  "loving" a track that want to submit whenever something gets loved.
>  I'm not sure how to do this without involving service-specific code.

Nikolaj added some way to send commands to services, maybe that would
allow us to implement love/ban/whatever without invoking specific
services. I don't know if there are other parts of Amarok which could
be interested in loving, but in that case the adapter would be a
singleton facade->problem solved. Regarding class-specific methods for
loving: SqlTrack could return a special capability sub-class which
would toggle the flag, and then call the default implementation.

Max

>
> >
>  >  It certainly makes sense to rely on the last.fm service for all
>  >  last.fm web service calls because the last.fm client code is there.
>  >  but we need some kind of adapter or proxy in libamarok that is called
>  >  whenever another part of amarok wants to interact with last.fm, and
>  >  then forwards those calls to the last.fm service (or the last.fm
>  >  service pulls the requests, but that class in libamarok must be able
>  >  to make sure that the last.fm service actually exists).
>  >
>  >  Max
>  >
>  >  On Tue, Apr 22, 2008 at 5:00 AM, Dan Meltzer
>  >  <hydrogen at notyetimplemented.com> wrote:
>  >  > SVN commit 799706 by dmeltzer:
>  >  >
>  >  >  Move the method to MainWindow for now... we may want to think about making loved a property in Meta::Track to allow for service specific implementations as well.  Thoughts Max?
>  >  >  CCMAIL: maximilian.kossick at gmail.com
>  >  >
>  >  >
>  >  >   M  +0 -7      EngineController.cpp
>  >  >   M  +0 -2      EngineController.h
>  >  >   M  +8 -1      MainWindow.cpp
>  >  >   M  +4 -0      MainWindow.h
>  >  >   M  +1 -1      amarokcore/amarokdbushandler.cpp
>  >  >   M  +3 -2      servicebrowser/lastfm/ScrobblerAdapter.cpp
>  >  >
>  >  >
>  >  >  --- trunk/extragear/multimedia/amarok/src/EngineController.cpp #799705:799706
>  >  >  @@ -318,13 +318,6 @@
>  >  >   }
>  >  >
>  >  >   void
>  >  >  -EngineController::loveTrack()
>  >  >  -{
>  >  >  -    if( m_currentTrack )
>  >  >  -        emit loveTrack( m_currentTrack );
>  >  >  -}
>  >  >  -
>  >  >  -void
>  >  >   EngineController::seek( int ms ) //SLOT
>  >  >   {
>  >  >      if( m_media->isSeekable() )
>  >  >  --- trunk/extragear/multimedia/amarok/src/EngineController.h #799705:799706
>  >  >  @@ -78,7 +78,6 @@
>  >  >      void pause();
>  >  >      void stop( bool forceInstant = false );
>  >  >      void playPause(); //pauses if playing, plays if paused or stopped
>  >  >  -    void loveTrack();
>  >  >
>  >  >      void seek( int ms );
>  >  >      void seekRelative( int ms );
>  >  >  @@ -94,7 +93,6 @@
>  >  >   signals:
>  >  >      void statusText( const QString& );
>  >  >      void trackFinished();
>  >  >  -    void loveTrack( Meta::TrackPtr );
>  >  >
>  >  >   protected:
>  >  >      void playUrl( const KUrl &url, uint offset );
>  >  >  --- trunk/extragear/multimedia/amarok/src/MainWindow.cpp #799705:799706
>  >  >  @@ -764,6 +764,13 @@
>  >  >   #endif
>  >  >   }
>  >  >
>  >  >  +void
>  >  >  +MainWindow::loveTrack()
>  >  >  +{
>  >  >  +    Meta::TrackPtr cTrack = The::engineController()->currentTrack();
>  >  >  +    if( cTrack )
>  >  >  +        emit loveTrack( cTrack );
>  >  >  +}
>  >  >   void MainWindow::activate()
>  >  >   {
>  >  >   #ifdef Q_WS_X11
>  >  >  @@ -1010,7 +1017,7 @@
>  >  >      action->setObjectName( "loveTrack" );
>  >  >      action->setGlobalShortcut( KShortcut( Qt::META + Qt::Key_L ) );
>  >  >      ac->addAction( "loveTrack", action );
>  >  >  -    connect( action, SIGNAL(triggered()), ec, SLOT(loveTrack()) );
>  >  >  +    connect( action, SIGNAL(triggered()), SLOT(loveTrack()) );
>  >  >
>  >  >      action = new KAction( i18n( "Rate Current Track: 1" ), this );
>  >  >      action->setObjectName( "rate1" );
>  >  >  --- trunk/extragear/multimedia/amarok/src/MainWindow.h #799705:799706
>  >  >  @@ -65,8 +65,12 @@
>  >  >          SideBar *sideBar() const { return m_browsers; }
>  >  >          void deleteBrowsers();
>  >  >
>  >  >  +    signals:
>  >  >  +        void loveTrack( Meta::TrackPtr );
>  >  >  +
>  >  >      public slots:
>  >  >          void showHide();
>  >  >  +        void loveTrack();
>  >  >
>  >  >      private slots:
>  >  >          void slotShrinkBrowsers( int index ) const;
>  >  >  --- trunk/extragear/multimedia/amarok/src/amarokcore/amarokdbushandler.cpp #799705:799706
>  >  >  @@ -345,7 +345,7 @@
>  >  >
>  >  >      void DbusPlayerHandler::loveTrack()
>  >  >      {
>  >  >  -        The::engineController()->loveTrack();
>  >  >  +        MainWindow::self()->loveTrack();
>  >  >      }
>  >  >
>  >  >      void DbusPlayerHandler::mediaDeviceMount()
>  >  >  --- trunk/extragear/multimedia/amarok/src/servicebrowser/lastfm/ScrobblerAdapter.cpp #799705:799706
>  >  >  @@ -14,9 +14,10 @@
>  >  >   #define DEBUG_PREFIX "lastfm"
>  >  >
>  >  >   #include "ScrobblerAdapter.h"
>  >  >  -#include "EngineController.h"
>  >  >   #include "Amarok.h"
>  >  >   #include "debug.h"
>  >  >  +#include "EngineController.h"
>  >  >  +#include "MainWindow.h"
>  >  >   #include "MetaConstants.h"
>  >  >   #include "meta/LastFmMeta.h"
>  >  >   #include "TheInstances.h"
>  >  >  @@ -32,7 +33,7 @@
>  >  >      resetVariables();
>  >  >
>  >  >      connect( m_manager, SIGNAL( status( int, QVariant ) ), this, SLOT( statusChanged( int, QVariant ) ) );
>  >  >  -    connect( The::engineController(), SIGNAL(loveTrack(Meta::TrackPtr)), SLOT(slotTrackLoved(Meta::TrackPtr)));
>  >  >  +    connect( MainWindow::self(), SIGNAL(loveTrack(Meta::TrackPtr)), SLOT(slotTrackLoved(Meta::TrackPtr)));
>  >  >
>  >  >      Scrobbler::Init init;
>  >  >      init.username = username;
>  >  >
>  >  _______________________________________________
>  >  Amarok-devel mailing list
>  >  Amarok-devel at kde.org
>  >  https://mail.kde.org/mailman/listinfo/amarok-devel
>  >
>


More information about the Amarok-devel mailing list