[Amarok] Fix: Automatic playlist scrolling was broken in so

[Seb Ruiz]

2009/10/7 Mark Kretschmann <kretschmann at kde.org>:
> commit a9a46b6ceab835c646b89d744bfcc5a7176ef5c1
> Author:     Mark Kretschmann <kretschmann at kde.org>
> AuthorDate: Wed Oct 7 12:50:25 2009 +0200
> Commit:     Mark Kretschmann <kretschmann at kde.org>
> CommitDate: Wed Oct 7 12:50:25 2009 +0200
>
>    Fix: Automatic playlist scrolling was broken in some cases.
>
>    The actual bug was very very nasty. EngineObserver used QSet for storing
>    observer pointers. This is what Qt docs say:
>    "QSet is unordered, so an iterator's sequence cannot be assumed to be predictable."
>    Now we're using QList, whose iteration order is predictable.
>
>    Thanks to Anthony Vital for pointing this out.
>
>    BUG: 193459
>
> diff --git a/ChangeLog b/ChangeLog
> index 2518826..b89cadf 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -38,6 +38,7 @@ VERSION 2.2.1
>       the lyrics applet.
>
>   BUGFIXES:
> +    * Automatic playlist scrolling was broken in some cases. (BR 193459)
>     * The On Screen Display now uses a sane font size with all display
>       resolutions. (BR 195186)
>     * Fix rare regression where inaccessible subfolders (due to permissions)
> diff --git a/src/EngineObserver.cpp b/src/EngineObserver.cpp
> index d62f40d..37b85fe 100644
> --- a/src/EngineObserver.cpp
> +++ b/src/EngineObserver.cpp
> @@ -209,13 +209,13 @@ EngineSubject::attach( EngineObserver *observer )
>     if( object )
>         connect( object, SIGNAL( destroyed( QObject* ) ), this, SLOT( observerDestroyed( QObject* ) ) );
>
> -    Observers.insert( observer );
> +    Observers.append( observer );
>  }
>
>  void
>  EngineSubject::detach( EngineObserver *observer )
>  {
> -    Observers.remove( observer );
> +    Observers.removeAll( observer );
>  }
>
>  void
> @@ -224,7 +224,7 @@ EngineSubject::observerDestroyed( QObject* object ) //SLOT
>     DEBUG_BLOCK
>
>     EngineObserver* observer = reinterpret_cast<EngineObserver*>( object ); // cast is OK, it's guaranteed to be an EngineObserver
> -    Observers.remove( observer );
> +    Observers.removeAll( observer );
>
>     debug() << "Removed EngineObserver: " << observer;
>  }
> diff --git a/src/EngineObserver.h b/src/EngineObserver.h
> index c9f023c..92f15f6 100644
> --- a/src/EngineObserver.h
> +++ b/src/EngineObserver.h
> @@ -24,7 +24,6 @@
>  #include <Phonon/Global>
>  #include <QHash>
>  #include <QPointer>
> -#include <QSet>
>
>  class EngineSubject;
>  class QString;
> @@ -187,7 +186,7 @@ private:
>     bool isMetaDataSpam( QHash<qint64, QString> newMetaData );
>
>     QList<QHash<qint64, QString> > m_metaDataHistory;
> -    QSet<EngineObserver*> Observers;
> +    QList<EngineObserver*> Observers;
>     Phonon::State m_realState; // To work around the buffering issue
>  };
>

Curious - how does this solve the problem? Why should it matter what
order the observers are in - I dislike that there is any code that
relies on some deterministic order for iterating over them (each
observer should be disjoint from the other)


-- 
Seb Ruiz

http://www.sebruiz.net/
http://amarok.kde.org/