[gnome-music/wip/jfelder/fix-play-and-load-logic] player: Correctly handle load and play logic

[Jean Felder <jfelder src gnome org>]

commit 91b854b62eeb8ca20e3131945079bd8ffb61554f
Author: Jean Felder <jfelder src gnome org>
Date:   Mon May 6 17:23:07 2019 +0200

    player: Correctly handle load and play logic
    
    The play method can be called in 4 cases:
    * At the beginning of a playlist to start the player (set_playlist +
      play calls)
    * To resume a song when in pause (play_pause)
    * To go to next or previous song (next and previous calls)
    * At the end of song to play the next song (via _on_eos which itself
      relies on a next call)
    
    The main difficulty inside the play method is to be able to detect if
    the current song has changed or if the player is just paused. In the
    first case, the _load method needs to be called in order to update the
    GstPlayer url.
    Initially, the test to distinguish these two cases was to check if the
    Player was on pause. This worked reliably expect in one case: it was
    impossible to change the current song while an other song was on
    pause (see #256).
    This issue was solved by checking if the song url had changed
    instead. This solved the previous issue but created an other one: the
    repeat-song mode did not work anymore. Indeed, when a song is
    repeated, the playlist url does not change at the end of the song
    (see #278).
    So, an additionnal check was added to test the repeat mode of the
    player. It also fixed the previous indeed the previous issue but
    introduced a new one. In repeat-song mode, the song restarts every
    time the player has been paused (see #280).
    
    All these issues can be solved by simply setting a "_song_changed"
    attribute inside the next and previous methods. Indeed, once the
    playlist has started, every time the current song needs to be changed
    a next or previous call has been made.
    
    Related: #256, #278
    Closes: #280

 gnomemusic/player.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
---
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index f60eed0a..8e70dbb0 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -575,6 +575,7 @@ class Player(GObject.GObject):
             GObject.BindingFlags.SYNC_CREATE)
 
         self._new_clock = True
+        self._song_changed = False
 
         self._gst_player = GstPlayer(application)
         self._gst_player.connect('clock-tick', self._on_clock_tick)
@@ -644,14 +645,14 @@ class Player(GObject.GObject):
         if self.props.current_song is None:
             return
 
-        if (song_offset is not None
-                and not self._playlist.set_song(song_offset)):
-            return False
+        if song_offset is not None:
+            if self._playlist.set_song(song_offset):
+                self._song_changed = True
+            else:
+                return
 
-        url = self._playlist.props.current_song.get_url()
-        loop_modes = [RepeatMode.SONG, RepeatMode.ALL]
-        if (url != self._gst_player.props.url
-                or self.props.repeat_mode in loop_modes):
+        if self._song_changed is True:
+            self._song_changed = False
             self._load(self._playlist.props.current_song)
 
         self._gst_player.props.state = Playback.PLAYING
@@ -673,6 +674,7 @@ class Player(GObject.GObject):
         Play the next song of the playlist, if any.
         """
         if self._playlist.next():
+            self._song_changed = True
             self.play()
 
     @log
@@ -687,6 +689,7 @@ class Player(GObject.GObject):
             return
 
         if self._playlist.previous():
+            self._song_changed = True
             self.play()
 
     @log
@@ -706,6 +709,7 @@ class Player(GObject.GObject):
         :param GtkListStore model: list of songs to play
         :param GtkTreeIter model_iter: requested song
         """
+        self._song_changed = True
         playlist_changed = self._playlist.set_playlist(
             playlist_type, playlist_id, model, iter_)