What exact version are you using? And is this happening on every view or just one specific view. I can't reproduce this at all.

3.22.1 however, this is the archlinux build.

I am able to reproduce this in any view.

Ok, I see it now if I let the song play until the end. Using the next/prev button doesn't exhibit this behaviour.

I am a newcomer and I wish to work on this bug. Is anyone else working on it? If not, can anyone point me in the right direction?

(In reply to Rithvik Patibandla from comment #4)
> I am a newcomer and I wish to work on this bug. Is anyone else working on
> it? If not, can anyone point me in the right direction?

Feel free to look into this, the only relevant info I have is comment #3. Figure out why these 2 modes of operation differ in behaviour.

Created attachment 341531 [details] [review]
fix the way next track is fetched when song ends

The player was fetching next track incorrectly when repeat options
were changed and the current song ended which led to the repeat
option effect being applied from the next track onward.

Fixes

Review of attachment 341531 [details] [review]:

Correct. Well done

This problem has been fixed in our software repository. The fix will go into the next software release. Once that release is available, you may want to check for a software upgrade provided by your Linux distribution.

I, the bug reporter can verify the fix after release. Am currently on Arch where I reported the bug. So won't take long to get the latest software officially.

Thanks all.

Review of attachment 341531 [details] [review]:

Ok, this would fix the issue it seems.

However, it is not really addressing the real issue afaics. The problem is that self.nextTrack is set at some point, but not when the playback mode (shuffle/repeat/etc.) is set. This patch just always gets the next track at the time the track is changed (after eos) and skips all the validation that was done beforehand (aka the code that fills self.nextTrack).

Honestly, I don't know why this code is seemingly so overcomplicated, but really it should be reworked to either validate or not validate at all. But right now it's validating and then just skipping over the fact that it did the whole validation step.

By the way, for the commit msg look at styling of previous commits and https://wiki.gnome.org/Git/CommitMessages (really minor point).

(In reply to Marinus Schraal from comment #10)
> Review of attachment 341531 [details] [review] [review]:
> 
> Ok, this would fix the issue it seems.
> 
> However, it is not really addressing the real issue afaics. The problem is
> that self.nextTrack is set at some point, but not when the playback mode
> (shuffle/repeat/etc.) is set. This patch just always gets the next track at
> the time the track is changed (after eos) and skips all the validation that
> was done beforehand (aka the code that fills self.nextTrack).
> 
> Honestly, I don't know why this code is seemingly so overcomplicated, but
> really it should be reworked to either validate or not validate at all. But
> right now it's validating and then just skipping over the fact that it did
> the whole validation step.
> 
> By the way, for the commit msg look at styling of previous commits and
> https://wiki.gnome.org/Git/CommitMessages (really minor point).

I went through the code again and here is what is happening,

when we change the repeat mode the function _on_repeat_setting_changed() is called.

It changes self.repeat and calls _sync_prev_next() and _sync_repeat_image() which basically change the icons for prev and next and the repeat mode icons. But they do not change self.nextTrack. The only place where any substantial changes to next track is made is _get_next_track() which is called when _validate_next_track() is called with the parameter track = None.

What I dont get is the fact that when we call play_next() we skip all the above validation entirely.

_validate_next_track is called when load() is called on a media which is called on play()

The easy way out is to let _on_glib_idle() set currentTrack as self.nextTrack and call _validate_next_track() in the method _on_repeat_setting_changed()

The longer way out will be to refactor the entire validation process as it seems kinda redundant when we are just calling _get_next_track() method again in play_next().

Created attachment 341621 [details] [review]
player.py: Change the way next track is fetched

There was a problem with the player that the repeat options were not
affecting the next track when the current song ended if they were
changed in the midst of current play.

This was due to _on_repeat_setting_changed() function not changing the
next track when repeat settings were changed.

To rectify this I added _validate_next_track() under the repeat changed
method which calls the _get_next_track() method and sets a valid next
Track variable.

I also changed the play_next method to use the nextTrack variable
instead of calling _get_next_track() again since nextTrack only after
it has been validated. Calling _get_next_track() directly made the
whole point of validating tracks moot otherwise.

Fixes

Review of attachment 341621 [details] [review]:

Apologies for the late review. 

I think this is what we have to go with right now, it fixes the issues in a somewhat correct manner. Eventually the whole class needs a rewrite, but this is not the time.

However concerning the patch, its now a patch on a patch. I just need the final fix in one patch. Also the comment is way to extensive, just keep it short and to the point. There's a bug number if one needs more info.

::: gnomemusic/player.py
@@ +244,3 @@
         self._sync_prev_next()
         self._sync_repeat_image()
+        self._validate_next_track()

this is your solution

@@ +716,2 @@
         self.stop()
+        self.currentTrack = self.nextTrack

this is reverting the previous patch

That makes sense. A fix is better than nothing.

After correct patch uploaded, merged and bug fixed should anyone open a bug with refactor proposed?

I'm seeking bug lifecycle and further tasks are part of it.

Sorry to ask here too: when is this supposed to be released? (Lifecycle again)

*** Bug 776701 has been marked as a duplicate of this bug. ***

Thanks for the work & patches here.

I made a mistake in the review, the last patch was correct all along. I 
did rewrite the commit message somewhat.