[gnome-music/wip/jfelder/simplify-player-model: 5/5] player: Use its own simplified model

[Jean Felder <jfelder src gnome org>]

commit 93c900bec97f7b06715e0c67dbdd8285b15545f5
Author: Jean Felder <jfelder src gnome org>
Date:   Sun Feb 25 16:30:46 2018 +0100

    player: Use its own simplified model
    
    Player keeps an internal reference to the currently visible view, wich
    can be different from the currently played model. The main idea is to
    isolate player model. when set_playlist method from player is called, a
    simple copy is done. Therefore, when view changes, player model is not
    updated. This new model needs to stay synchronized by calling add_song
    or remove_song appropriately.
    This is not a fix. Just a workaround before player.py rewrite.
    Add an enum for player model fields.
    Remove 'song-position-changed' signal from playlists as it is not
    necessary anymore.
    
    Closes: #136

 gnomemusic/mpris.py                      |   8 +-
 gnomemusic/player.py                     | 100 +++++++++++++++++--------
 gnomemusic/playlists.py                  |   4 -
 gnomemusic/views/playlistview.py         | 124 ++++++++++++-------------------
 gnomemusic/views/songsview.py            |  10 ++-
 gnomemusic/widgets/albumwidget.py        |   4 +-
 gnomemusic/widgets/artistalbumswidget.py |  10 +--
 7 files changed, 132 insertions(+), 128 deletions(-)
---
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index 4781274a..1c6e9ccf 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -350,7 +350,7 @@ class MediaPlayer2Service(Server):
     @log
     def _get_media_from_id(self, track_id):
         for track in self.player.playlist:
-            media = track[self.player.playlist_field]
+            media = track[self.player.Field.SONG]
             if track_id == self._get_media_id(media):
                 return media
         return None
@@ -358,7 +358,7 @@ class MediaPlayer2Service(Server):
     @log
     def _get_track_list(self):
         if self.player.playlist:
-            return [self._get_media_id(track[self.player.playlist_field])
+            return [self._get_media_id(track[self.player.Field.SONG])
                     for track in self.player.playlist]
         else:
             return []
@@ -496,7 +496,7 @@ class MediaPlayer2Service(Server):
     def _on_playlist_modified(self, path=None, _iter=None, data=None):
         if self.player.currentTrack and self.player.currentTrack.valid():
             path = self.player.currentTrack.get_path()
-            currentTrack = self.player.playlist[path][self.player.playlist_field]
+            currentTrack = self.player.playlist[path][self.player.Field.SONG]
             track_list = self._get_track_list()
             self.TrackListReplaced(track_list, self._get_media_id(currentTrack))
             self.PropertiesChanged(MediaPlayer2Service.MEDIA_PLAYER2_TRACKLIST_IFACE,
@@ -591,7 +591,7 @@ class MediaPlayer2Service(Server):
 
     def GoTo(self, track_id):
         for track in self.player.playlist:
-            media = track[self.player.playlist_field]
+            media = track[self.player.Field.SONG]
             if track_id == self._get_media_id(media):
                 self.player.set_playlist(
                     self.player.playlistType, self.player.playlistId,
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index d64c2a29..f97c0dac 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -31,6 +31,7 @@
 # delete this exception statement from your version.
 
 from collections import deque
+from enum import IntEnum
 import logging
 from random import randint
 import time
@@ -96,6 +97,11 @@ class Player(GObject.GObject):
         'thumbnail-updated': (GObject.SignalFlags.RUN_FIRST, None, ()),
     }
 
+    class Field(IntEnum):
+        """Enum for player model fields"""
+        SONG = 0
+        DISCOVERY_STATUS = 1
+
     def __repr__(self):
         return '<Player>'
 
@@ -107,7 +113,6 @@ class Player(GObject.GObject):
         self.playlist = None
         self.playlistType = None
         self.playlistId = None
-        self.playlist_field = 5
         self.currentTrack = None
         self._current_track_uri = None
         self._missingPluginMessages = []
@@ -138,9 +143,6 @@ class Player(GObject.GObject):
         self.bus.connect('message::eos', self._on_bus_eos)
         self._setup_view()
 
-        self.playlist_insert_handler = 0
-        self.playlist_delete_handler = 0
-
         self._lastfm = LastFmScrobbler()
 
     @GObject.Property
@@ -334,8 +336,10 @@ class Player(GObject.GObject):
         media = self.get_current_media()
         if media is not None:
             if self.currentTrack and self.currentTrack.valid():
-                currentTrack = self.playlist.get_iter(self.currentTrack.get_path())
-                self.playlist.set_value(currentTrack, self.discovery_status_field, DiscoveryStatus.FAILED)
+                currentTrack = self.playlist.get_iter(
+                    self.currentTrack.get_path())
+                cols = self.playlist[currentTrack]
+                cols[self.Field.DISCOVERY_STATUS] = DiscoveryStatus.FAILED
             uri = media.get_url()
         else:
             uri = 'none'
@@ -380,7 +384,28 @@ class Player(GObject.GObject):
         self.play()
 
     @log
-    def _on_playlist_size_changed(self, path, _iter=None, data=None):
+    def add_song(self, model, path, _iter):
+        new_row = model[_iter]
+        self.playlist.insert_with_valuesv(
+            int(path.to_string()),
+            [self.Field.SONG, self.Field.DISCOVERY_STATUS],
+            [new_row[5], new_row[self._discovery_status_field]])
+        self._validate_next_track()
+        self._sync_prev_next()
+
+    @log
+    def remove_song(self, model, path):
+        iter_remove = self.playlist.get_iter_from_string(path.to_string())
+        if (self.currentTrack.get_path().to_string() == path.to_string()):
+            if self.has_next():
+                self.play_next()
+            elif self.has_previous():
+                self.play_previous()
+            else:
+                self.Stop()
+
+        self.playlist.remove(iter_remove)
+        self._validate_next_track()
         self._sync_prev_next()
 
     @log
@@ -593,7 +618,7 @@ class Player(GObject.GObject):
 
         if self.currentTrack and self.currentTrack.valid():
             currentTrack = self.playlist.get_iter(self.currentTrack.get_path())
-            uri = self.playlist[currentTrack][self.playlist_field].get_url()
+            uri = self.playlist[currentTrack][self.Field.SONG].get_url()
             self._current_track_uri = uri
             self.emit('playlist-item-changed', self.playlist, currentTrack)
             self.emit('current-changed')
@@ -605,7 +630,8 @@ class Player(GObject.GObject):
     def _on_next_item_validated(self, info, error, _iter):
         if error:
             print("Info %s: error: %s" % (info, error))
-            self.playlist.set_value(_iter, self.discovery_status_field, DiscoveryStatus.FAILED)
+            failed = DiscoveryStatus.FAILED
+            self.playlist[_iter][self.Field.DISCOVERY_STATUS] = failed
             nextTrack = self.playlist.iter_next(_iter)
 
             if nextTrack:
@@ -622,8 +648,8 @@ class Player(GObject.GObject):
             return
 
         _iter = self.playlist.get_iter(self.nextTrack.get_path())
-        status = self.playlist[_iter][self.discovery_status_field]
-        next_song = self.playlist[_iter][self.playlist_field]
+        status = self.playlist[_iter][self.Field.DISCOVERY_STATUS]
+        next_song = self.playlist[_iter][self.Field.SONG]
         url = next_song.get_url()
 
         # Skip remote songs discovery
@@ -719,29 +745,40 @@ class Player(GObject.GObject):
         else:
             self.set_playing(True)
 
+    @log
+    def _create_model(self, model, model_iter):
+        new_model = Gtk.ListStore(GObject.TYPE_OBJECT, GObject.TYPE_INT)
+        song_id = model[model_iter][5].get_id()
+        new_path = None
+        for row in model:
+            current_iter = new_model.insert_with_valuesv(
+                -1, [self.Field.SONG, self.Field.DISCOVERY_STATUS],
+                [row[5], row[self._discovery_status_field]])
+            if row[5].get_id() == song_id:
+                new_path = new_model.get_path(current_iter)
+
+        return new_model, new_path
+
     # FIXME: set the discovery field to 11 to be safe, but for some
     # models it is 12.
     @log
-    def set_playlist(self, type, id, model, iter, discovery_status_field=11):
-        old_playlist = self.playlist
-        if old_playlist != model:
-            self.playlist = model
-            if self.playlist_insert_handler:
-                old_playlist.disconnect(self.playlist_insert_handler)
-            if self.playlist_delete_handler:
-                old_playlist.disconnect(self.playlist_delete_handler)
-
-        self.playlistType = type
-        self.playlistId = id
-        self.currentTrack = Gtk.TreeRowReference.new(model, model.get_path(iter))
-        self.discovery_status_field = discovery_status_field
-
-        if old_playlist != model:
-            self.playlist_insert_handler = model.connect('row-inserted', self._on_playlist_size_changed)
-            self.playlist_delete_handler = model.connect('row-deleted', self._on_playlist_size_changed)
+    def set_playlist(
+            self, type_, id_, model, iter_, discovery_status_field=11):
+        self._discovery_status_field = discovery_status_field
+        self.playlist, playlist_path = self._create_model(model, iter_)
+        self.currentTrack = Gtk.TreeRowReference.new(
+            self.playlist, playlist_path)
+
+        if type_ != self.playlistType or id_ != self.playlistId:
             self.emit('playlist-changed')
-        self.emit('current-changed')
 
+        self.playlistType = type_
+        self.playlistId = id_
+
+        if self._get_playing():
+            self._sync_prev_next()
+
+        self.emit('current-changed')
         GLib.idle_add(self._validate_next_track)
 
     @log
@@ -1050,9 +1087,10 @@ class Player(GObject.GObject):
         if not self.currentTrack or not self.currentTrack.valid():
             return None
         currentTrack = self.playlist.get_iter(self.currentTrack.get_path())
-        if self.playlist.get_value(currentTrack, self.discovery_status_field) == DiscoveryStatus.FAILED:
+        failed = DiscoveryStatus.FAILED
+        if self.playlist[currentTrack][self.Field.DISCOVERY_STATUS] == failed:
             return None
-        return self.playlist[currentTrack][self.playlist_field]
+        return self.playlist[currentTrack][self.Field.SONG]
 
 
 class MissingCodecsDialog(Gtk.MessageDialog):
diff --git a/gnomemusic/playlists.py b/gnomemusic/playlists.py
index 2cd72be6..fae14d8a 100644
--- a/gnomemusic/playlists.py
+++ b/gnomemusic/playlists.py
@@ -126,9 +126,6 @@ class Playlists(GObject.GObject):
         'song-removed-from-playlist': (
             GObject.SignalFlags.RUN_FIRST, None, (Grl.Media, Grl.Media)
         ),
-        'song-position-changed': (
-            GObject.SignalFlags.RUN_FIRST, None, (Grl.Media, Grl.Media)
-        ),
     }
 
     instance = None
@@ -441,7 +438,6 @@ class Playlists(GObject.GObject):
         """
         def update_callback(conn, res, data):
             conn.update_finish(res)
-            self.emit('song-position-changed', playlist, data)
 
         for item, new_position in zip(items, new_positions):
             self.tracker.update_async(
diff --git a/gnomemusic/views/playlistview.py b/gnomemusic/views/playlistview.py
index 98665248..26f09356 100644
--- a/gnomemusic/views/playlistview.py
+++ b/gnomemusic/views/playlistview.py
@@ -150,8 +150,6 @@ class PlaylistView(BaseView):
         playlists.connect('playlist-updated', self._on_playlist_update)
         playlists.connect(
             'song-added-to-playlist', self._on_song_added_to_playlist)
-        playlists.connect(
-            'song-position-changed', self._on_song_position_changed)
 
         self.show_all()
 
@@ -272,9 +270,11 @@ class PlaylistView(BaseView):
                 'Playlist', self._current_playlist.get_id()):
             return False
 
-        self.model[current_iter][10] = True
-        if self.model[current_iter][8] != self._error_icon_name:
-            self._iter_to_clean = current_iter.copy()
+        pos_str = playlist.get_path(current_iter).to_string()
+        iter_ = self.model.get_iter_from_string(pos_str)
+        self.model[iter_][10] = True
+        if self.model[iter_][8] != self._error_icon_name:
+            self._iter_to_clean = iter_.copy()
             self._iter_to_clean_model = self.model
 
         return False
@@ -409,20 +409,27 @@ class PlaylistView(BaseView):
         if abs(new_pos - prev_pos) == 1:
             return
 
-        # If playing song position has changed, update player's playlist.
-        if self.player.playing and not self.player.currentTrack.valid():
-            pos = new_pos
-            if new_pos > prev_pos:
-                pos -= 1
-            new_iter = self.model.get_iter_from_string(str(pos))
-            self._iter_to_clean = new_iter
-            self.player.set_playlist(
-                'Playlist', self._current_playlist.get_id(), self.model,
-                new_iter)
-
         first_pos = min(new_pos, prev_pos)
         last_pos = max(new_pos, prev_pos)
 
+        # update player's playlist.
+        if self.player.running_playlist(
+                'Playlist', self._current_playlist.get_id()):
+            playing_old_path = self.player.currentTrack.get_path().to_string()
+            playing_old_pos = int(playing_old_path)
+            iter_ = model.get_iter_from_string(playing_old_path)
+            # if playing song position has changed
+            if playing_old_pos >= first_pos and playing_old_pos < last_pos:
+                current_player_song = self.player.get_current_media()
+                for row in model:
+                    if row[5].get_id() == current_player_song.get_id():
+                        iter_ = row.iter
+                        self._iter_to_clean = iter_
+                        self._iter_to_clean_model = model
+                        break
+            self.player.set_playlist(
+                'Playlist', self._current_playlist.get_id(), model, iter_)
+
         positions = []
         songs = []
         for pos in range(first_pos, last_pos):
@@ -586,18 +593,18 @@ class PlaylistView(BaseView):
         """
         if not song:
             self._update_songs_count()
-            if self.player.playlist:
-                self.player._validate_next_track()
             self.emit('playlist-songs-loaded')
-            return
+            return None
 
         title = utils.get_media_title(song)
         song.set_title(title)
         artist = utils.get_artist_name(song)
-        model.insert_with_valuesv(index, [2, 3, 5, 9],
-                                  [title, artist, song, song.get_favourite()])
+        iter_ = model.insert_with_valuesv(
+            index, [2, 3, 5, 9],
+            [title, artist, song, song.get_favourite()])
 
         self._songs_count += 1
+        return iter_
 
     @log
     def _update_songs_count(self):
@@ -708,8 +715,12 @@ class PlaylistView(BaseView):
             playlist = song_todelete['playlist']
             if (self._current_playlist
                     and playlist.get_id() == self._current_playlist.get_id()):
-                self._add_song_to_model(
+                iter_ = self._add_song_to_model(
                     song_todelete['song'], self.model, song_todelete['index'])
+                if self.player.running_playlist(
+                        'Playlist', self._current_playlist.get_id()):
+                    path = self.model.get_path(iter_)
+                    self.player.add_song(self.model, path, iter_)
                 self._update_songs_count()
             self._songs_todelete.pop(media_id)
 
@@ -775,23 +786,14 @@ class PlaylistView(BaseView):
         """Add new playlist to sidebar"""
         self._add_playlist_to_sidebar(playlist)
 
-    @log
-    def _row_is_playing(self, playlist, row):
-        """Check if row is being played"""
-        if (self._is_current_playlist(playlist)
-                and self.player.currentTrack is not None
-                and self.player.currentTrack.valid()):
-                track_path = self.player.currentTrack.get_path()
-                track_path_str = track_path.to_string()
-                if (row.path is not None
-                        and row.path.to_string() == track_path_str):
-                    return True
-        return False
-
     @log
     def _on_song_added_to_playlist(self, playlists, playlist, item):
         if self._is_current_playlist(playlist):
-            self._add_song_to_model(item, self.model)
+            iter_ = self._add_song_to_model(item, self.model)
+            if self.player.running_playlist(
+                    'Playlist', self._current_playlist.get_id()):
+                path = self.model.get_path(iter_)
+                self.player.add_song(self.model, path, iter_)
 
     @log
     def _remove_song_from_playlist(self, playlist, item):
@@ -800,49 +802,19 @@ class PlaylistView(BaseView):
         else:
             return
 
-        # checks if the to be removed track is now being played
         for row in model:
             if row[5].get_id() == item.get_id():
+                iter_ = row.iter
+                if self.player.running_playlist(
+                        'Playlist', self._current_playlist.get_id()):
+                    path = model.get_path(iter_)
+                    self.player.remove_song(model, path)
+                model.remove(iter_)
+                break
 
-                is_being_played = self._row_is_playing(playlist, row)
-
-                next_iter = model.iter_next(row.iter)
-                model.remove(row.iter)
-
-                # Reload the model and switch to next song
-                if is_being_played:
-                    if next_iter is None:
-                        # Get first track if next track is not valid
-                        next_iter = model.get_iter_first()
-                        if next_iter is None:
-                            # Last track was removed
-                            return
-
-                    self._iter_to_clean = None
-                    self._update_model(self.player, model, next_iter)
-                    self.player.set_playlist(
-                        'Playlist', playlist.get_id(), model, next_iter)
-                    self.player.set_playing(True)
-
-                # Update songs count
-                self._songs_count -= 1
-                self._update_songs_count()
-                return
-
-    @log
-    def _on_song_position_changed(self, playlists, playlist, item):
-        """ If song is currently played, update next track"""
-        if not self._is_current_playlist(playlist):
-            return
-
-        if self.player.playing:
-            for row in self.model:
-                if (row[5].get_id() == item.get_id()
-                        and self._row_is_playing(playlist, row)):
-                    self._iter_to_clean = row.iter
-                    self.player.set_playlist(
-                        'Playlist', playlist.get_id(), self.model, row.iter)
-                    return
+        self._songs_count -= 1
+        self._update_songs_count()
+        return
 
     @log
     def populate(self):
diff --git a/gnomemusic/views/songsview.py b/gnomemusic/views/songsview.py
index d844f6d6..0d444b45 100644
--- a/gnomemusic/views/songsview.py
+++ b/gnomemusic/views/songsview.py
@@ -110,11 +110,13 @@ class SongsView(BaseView):
         if not player.running_playlist('Songs', None):
             return False
 
-        self.model[current_iter][10] = True
-        path = self.model.get_path(current_iter)
+        pos_str = playlist.get_path(current_iter).to_string()
+        iter_ = self.model.get_iter_from_string(pos_str)
+        self.model[iter_][10] = True
+        path = self.model.get_path(iter_)
         self._view.get_generic_view().scroll_to_path(path)
-        if self.model[current_iter][8] != self._error_icon_name:
-            self._iter_to_clean = current_iter.copy()
+        if self.model[iter_][8] != self._error_icon_name:
+            self._iter_to_clean = iter_.copy()
         return False
 
     def _add_item(self, source, param, item, remaining=0, data=None):
diff --git a/gnomemusic/widgets/albumwidget.py b/gnomemusic/widgets/albumwidget.py
index 427de770..fe9f5aed 100644
--- a/gnomemusic/widgets/albumwidget.py
+++ b/gnomemusic/widgets/albumwidget.py
@@ -293,7 +293,7 @@ class AlbumWidget(Gtk.EventBox):
         if not player.running_playlist('Album', self._album):
             return True
 
-        current_song = playlist[current_iter][player.playlist_field]
+        current_song = playlist[current_iter][player.Field.SONG]
 
         self._duration = 0
 
@@ -301,7 +301,7 @@ class AlbumWidget(Gtk.EventBox):
         _iter = playlist.get_iter_first()
 
         while _iter:
-            song = playlist[_iter][player.playlist_field]
+            song = playlist[_iter][player.Field.SONG]
             song_widget = song.song_widget
             self._duration += song.get_duration()
             escaped_title = GLib.markup_escape_text(
diff --git a/gnomemusic/widgets/artistalbumswidget.py b/gnomemusic/widgets/artistalbumswidget.py
index ca5a0b61..be651e19 100644
--- a/gnomemusic/widgets/artistalbumswidget.py
+++ b/gnomemusic/widgets/artistalbumswidget.py
@@ -134,20 +134,16 @@ class ArtistAlbumsWidget(Gtk.Box):
 
     @log
     def _update_model(self, player, playlist, current_iter):
-        # this is not our playlist, return
-        if playlist != self._model:
-            # TODO, only clean once, but that can wait util we have clean
-            # the code a bit, and until the playlist refactoring.
-            # the overhead is acceptable for now
+        if not player.running_playlist('Artist', self.artist):
             self._clean_model()
             return False
 
-        current_song = playlist[current_iter][5]
+        current_song = playlist[current_iter][player.Field.SONG]
         song_passed = False
         itr = playlist.get_iter_first()
 
         while itr:
-            song = playlist[itr][5]
+            song = playlist[itr][player.Field.SONG]
             song_widget = song.song_widget
 
             if not song_widget.can_be_played: