Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Previous track button doesn't work properly with shuttle mode enabled #3106

Closed
luk1337 opened this issue Jan 8, 2019 · 5 comments
Closed
Labels

Comments

@luk1337
Copy link
Contributor

luk1337 commented Jan 8, 2019

Steps to reproduce

  1. Enable shuttle mode
  2. Press previous track twice.

Expected Output

Previous song should be playing.

Actual Output

Current playing song is set at 0:00.

Test System

Which version of Quod Libet?

5c683a1

Which operating system

Fedora 29

Additional Information

The issue got introduced by commit id 1e9e848 and I can /fix/ my issue by doing this. I'm unsure if this is a valid fix therefore I'm opening a bug report instead of submitting this in a pull request.

diff --git a/quodlibet/quodlibet/qltk/songmodel.py b/quodlibet/quodlibet/qltk/songmodel.py
index 3e7ef5a56..b675dcc57 100644
--- a/quodlibet/quodlibet/qltk/songmodel.py
+++ b/quodlibet/quodlibet/qltk/songmodel.py
@@ -96,7 +96,7 @@ class PlaylistMux(object):
 
         if q_disable or self.pl.sourced or not keep_songs:
             self.pl.previous()
-            self.go_to(self.pl.current)
+            # self.go_to(self.pl.current)
         else:
             self.q.previous()
         self._check_sourced()
-- 
@luk1337 luk1337 added the bug label Jan 8, 2019
@luk1337 luk1337 changed the title Previous track doesn't work properly Previous track button doesn't work properly Jan 8, 2019
@frestr
Copy link
Member

frestr commented Jan 8, 2019

I'm not able to reproduce this.

Does this happen for every song, both in the song list and in the queue? Also, what's your queue settings, and do you get any output when pressing previous while running QL in debug mode (./quodlibet --debug)? (If so, please post it here.)

I'm unsure if this is a valid fix

Removing that line will introduce a bug where the song list will not begin playing if first sourcing from the queue in persistent mode, then disabling the queue, and finally pressing previous a couple of times.

@luk1337
Copy link
Contributor Author

luk1337 commented Jan 8, 2019

I'm not able to reproduce this.

Oh, it looks like it only reproducible if you have shuffle mode enabled.

Also, what's your queue settings, and do you get any output when pressing previous while running QL in debug mode (./quodlibet --debug)? (If so, please post it here.)

I don't really see anything that important in debug output. I'll post it here if you won't be able to reproduce it again after enabling shuffle mode.

@luk1337 luk1337 changed the title Previous track button doesn't work properly Previous track button doesn't work properly with shuttle mode enabled Jan 8, 2019
@frestr
Copy link
Member

frestr commented Jan 8, 2019

I can reproduce it now, thanks. I'll try to come up with a fix.

@frestr
Copy link
Member

frestr commented Jan 8, 2019

I think the following should fix it. Feel free to test it.

diff --git a/quodlibet/quodlibet/qltk/songmodel.py b/quodlibet/quodlibet/qltk/songmodel.py
index 3e7ef5a56..769c9cb48 100644
--- a/quodlibet/quodlibet/qltk/songmodel.py
+++ b/quodlibet/quodlibet/qltk/songmodel.py
@@ -66,9 +66,10 @@ class PlaylistMux(object):
                 or q_disable
                 or (keep_songs and not self.q.sourced)):
             self.pl.next()
-            # The go_to is to make sure the playlist begins playing
-            # when the queue is disabled while being sourced
-            self.go_to(self.pl.current)
+            if q_disable and self.q.sourced:
+                # The go_to is to make sure the playlist begins playing
+                # when the queue is disabled while being sourced
+                self.go_to(self.pl.current)
         else:
             self.q.next()
         self._check_sourced()
@@ -83,7 +84,8 @@ class PlaylistMux(object):
                 or q_disable
                 or (keep_songs and not self.q.sourced)):
             self.pl.next_ended()
-            self.go_to(self.pl.current)
+            if q_disable and self.q.sourced:
+                self.go_to(self.pl.current)
         else:
             self.q.next_ended()
         self._check_sourced()
@@ -96,7 +98,8 @@ class PlaylistMux(object):
 
         if q_disable or self.pl.sourced or not keep_songs:
             self.pl.previous()
-            self.go_to(self.pl.current)
+            if q_disable and self.q.sourced:
+                self.go_to(self.pl.current)
         else:
             self.q.previous()
         self._check_sourced()

@luk1337
Copy link
Contributor Author

luk1337 commented Jan 8, 2019

It seems to work properly here~

@frestr frestr closed this as completed in 5403602 Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants