Merge pull request #4382 from quodlibet/revert-4255

Revert "Fix shuffle history (#4255)"

quodlibet/order/__init__.py

@@ -6,9 +6,7 @@
 # the Free Software Foundation; either version 2 of the License, or
 # (at your option) any later version.
 
-from typing import Any, Dict, List, Optional
-
-from gi.repository import Gtk
+from typing import Optional
 
 from quodlibet import _, print_d
 
@@ -105,42 +103,39 @@ class OrderRemembered(Order):
     """Shared class for all the shuffle modes that keep a memory
     of their previously played songs."""
 
-    _played: List[Gtk.TreeIter]
-
     def __init__(self):
         super().__init__()
         self._played = []
 
     def next(self, playlist, iter):
         if iter is not None:
-            self._played.append(iter)
+            self._played.append(playlist.get_path(iter).get_indices()[0])
 
     def previous(self, playlist, iter):
-        if self._played:
-            return self._played.pop()
-        return None
+        try:
+            path = self._played.pop()
+        except IndexError:
+            return None
+        else:
+            return playlist.get_iter(path)
 
     def set(self, playlist, iter):
         if iter is not None:
-            self._played.append(iter)
+            self._played.append(playlist.get_path(iter).get_indices()[0])
         return iter
 
     def reset(self, playlist):
         del(self._played[:])
 
-    def remaining(self, playlist) -> Dict[int, Any]:
+    def remaining(self, playlist):
         """Gets a map of all song indices to their song from the `playlist`
         that haven't yet been played"""
-
-        def get_index(iter):
-            return playlist.get_path(iter).get_indices()[0]
-
-        played = set(map(get_index, self._played))
+        all_indices = set(range(len(playlist)))
+        played = set(self._played)
         print_d("Played %d of %d song(s)" % (len(self._played), len(playlist)))
-        remaining = (
-            (get_index(iter), value) for iter, value in playlist.iterrows())
-        return {
-            index: song for (index, song) in remaining if index not in played}
+        remaining = list(all_indices.difference(played))
+        all_songs = playlist.get()
+        return {i: all_songs[i] for i in remaining}
 
 
 class OrderInOrder(Order):

quodlibet/order/reorder.py

@@ -24,14 +24,15 @@ class OrderShuffle(Reorder, OrderRemembered):
 
     def next(self, playlist, iter):
         super().next(playlist, iter)
-        remaining = self.remaining(playlist)
+        played = set(self._played)
+        songs = set(range(len(playlist)))
+        remaining = songs.difference(played)
 
-        if not remaining:
-            self.reset(playlist)
-            return None
+        if remaining:
+            return playlist.get_iter((random.choice(list(remaining)),))
 
-        index = random.choice(list(remaining))
-        return playlist.get_iter((index,))
+        self.reset(playlist)
+        return None
 
 
 class OrderWeighted(Reorder, OrderRemembered):
@@ -44,7 +45,7 @@ def next(self, playlist, iter):
         remaining = self.remaining(playlist)
 
         # Don't try to search through an empty / played playlist.
-        if not remaining:
+        if len(remaining) <= 0:
             self.reset(playlist)
             return None
 

quodlibet/qltk/queue.py

@@ -9,7 +9,7 @@
 
 import os
 import time
-from typing import Any, Optional, Sequence
+from typing import Optional
 
 from gi.repository import Gtk, Gdk, Pango, GLib
 
@@ -345,35 +345,7 @@ def __queue_disable(self, disabled):
 
 
 class QueueModel(PlaylistModel):
-    """
-    A play list model for queues.
-
-    In contrast to `PlaylistModel`, when new songs have been set, its play order
-    will disregard the current song from the old set of songs and start from
-    scratch.
-    """
-
-    __reset = False
-
-    def set(self, songs: Sequence[Any]):
-        self.__reset = True
-        super().set(songs)
-
-    def next(self):
-        print_d(f"Using {self.order}.next_explicit() to get next song")
-
-        iter_ = None if self.__reset else self.current_iter
-        self.__reset = False
-        self.current_iter = self.order.next_explicit(self, iter_)
-
-    def previous(self):
-        iter_ = None if self.__reset else self.current_iter
-        self.__reset = False
-        self.current_iter = self.order.previous_explicit(self, iter_)
-
-    def go_to(self, song_or_iter, explicit=False, source=None):
-        self.__reset = False
-        return super().go_to(song_or_iter, explicit, source)
+    """Own class for debugging"""
 
 
 class PlayQueue(SongList):

quodlibet/qltk/songlist.py

@@ -9,7 +9,7 @@
 # the Free Software Foundation; either version 2 of the License, or
 # (at your option) any later version.
 
-from typing import List, Optional, Sequence, Tuple
+from typing import List, Tuple
 
 from gi.repository import Gtk, GLib, Gdk, GObject
 from senf import uri2fsn
@@ -501,13 +501,15 @@ def toggle_column_sort(self, column, replace=True, refresh=True):
 
         # set the indicators
         default_order = Gtk.SortType.ASCENDING
+        reversed_ = False
         for c in self.get_columns():
             if c is column:
                 if c.get_sort_indicator():
                     if dont_reverse:
                         order = c.get_sort_order()
                     else:
                         order = not c.get_sort_order()
+                        reversed_ = True
                 else:
                     order = default_order
                 c.set_sort_order(order)
@@ -519,13 +521,10 @@ def toggle_column_sort(self, column, replace=True, refresh=True):
 
         if refresh:
             songs = self.get_songs()
-            song_order = self._get_song_order(songs)
-            self.model.reorder(song_order)
-
-            selection = self.get_selection()
-            _, paths = selection.get_selected_rows()
-            if paths:
-                self.scroll_to_cell(paths[0], use_align=True, row_align=0.5)
+            if reversed_:
+                # python sort is faster if presorted
+                songs.reverse()
+            self.set_songs(songs, scroll_select=True)
 
         self.emit("orders-changed")
 
@@ -784,18 +783,14 @@ def get_songs(self):
             return []
         return model.get()
 
-    def _get_song_order(self, songs: List[AudioFile]) -> Optional[Sequence[int]]:
-        """Returns mapping from new position to position in given list of songs
-        when sorted based on the column sort orders"""
+    def _sort_songs(self, songs: List[AudioFile]):
+        """Sort passed songs in place based on the column sort orders"""
 
-        orders = self.get_sort_orders()
-        if orders:
-            song_order = list(range(len(songs)))
-            for key, reverse in self.__get_song_sort_key_func(orders):
-                song_order.sort(key=lambda i: key(songs[i]), reverse=reverse)
-            return song_order
-        else:
-            return None
+        order = self.get_sort_orders()
+        if not order:
+            return
+        for key, reverse in self.__get_song_sort_key_func(order):
+            songs.sort(key=key, reverse=reverse)
 
     def __get_song_sort_key_func(self, order):
         last_tag = None
@@ -860,15 +855,13 @@ def set_songs(self, songs: List[AudioFile], sorted: bool = False,
         model = self.get_model()
         assert model is not None
 
-        song_order = None
-
         if not sorted:
             # make sure some sorting is set and visible
             if not self.is_sorted():
                 default = self.find_default_sort_column()
                 if default:
                     self.toggle_column_sort(default, refresh=False)
-            song_order = self._get_song_order(songs)
+            self._sort_songs(songs)
         else:
             self.clear_sort()
 
@@ -878,8 +871,6 @@ def set_songs(self, songs: List[AudioFile], sorted: bool = False,
 
         with self.without_model() as model:
             model.set(songs)
-            if song_order:
-                model.reorder(song_order)
 
         # scroll to the first selected or current song and restore
         # selection for the first selected item if there was one
@@ -1004,7 +995,8 @@ def __song_updated(self, librarian, songs):
         Warning: This makes the row-changed signal useless.
         """
         model = self.get_model()
-        if config.getboolean("song_list", "auto_sort") and self.is_sorted():
+        if not config.getboolean("memory", "shuffle", False) and \
+            config.getboolean("song_list", "auto_sort") and self.is_sorted():
             iters, _, complete = self.__find_iters_in_selection(songs)
 
             if not complete:

quodlibet/qltk/songmodel.py

@@ -7,11 +7,8 @@
 # the Free Software Foundation; either version 2 of the License, or
 # (at your option) any later version.
 
-from typing import Any, Iterable, List, Optional, Sequence, Type
-
 from gi.repository import Gtk
 
-from quodlibet.order import Order
 from quodlibet.qltk.playorder import OrderInOrder
 from quodlibet.qltk.models import ObjectStore
 from quodlibet.util import print_d
@@ -154,10 +151,10 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.__iter = None
 
-    last_current: Optional[Any] = None
+    last_current = None
     """The last valid current song"""
 
-    def set(self, songs: Sequence[Any]):
+    def set(self, songs):
         """Clear the model and add the passed songs"""
 
         print_d("Filling view model with %d songs." % len(songs))
@@ -169,13 +166,13 @@ def set(self, songs: Sequence[Any]):
             if song is oldsong:
                 self.__iter = iter_
 
-    def get(self) -> List[Any]:
+    def get(self):
         """A list of all contained songs"""
 
         return list(self.itervalues())
 
     @property
-    def current(self) -> Optional[Any]:
+    def current(self):
         """The current song or None"""
 
         return self.__iter and self.get_value(self.__iter, 0)
@@ -203,7 +200,7 @@ def current_iter(self, iter_):
         self.__iter = iter_
         self.last_current = self.current
 
-    def find(self, song: Any):
+    def find(self, song):
         """Returns the iter to the first occurrence of song in the model
         or None if it wasn't found.
         """
@@ -218,13 +215,18 @@ def find(self, song: Any):
                 return iter_
         return
 
-    def find_all(self, songs: Iterable[Any]):
+    def find_all(self, songs):
         """Returns a list of iters for all occurrences of all songs.
         (since a song can be in the model multiple times)
         """
 
         songs = set(songs)
-        return [iter_ for iter_, value in self.iterrows() if value in songs]
+        found = []
+        append = found.append
+        for iter_, value in self.iterrows():
+            if value in songs:
+                append(iter_)
+        return found
 
     def remove(self, iter_):
         if self.__iter and self[iter_].path == self[self.__iter].path:
@@ -242,16 +244,23 @@ def __contains__(self, song):
 class PlaylistModel(TrackCurrentModel):
     """A play list model for song lists"""
 
-    order: Order
-    """The active play order"""
+    order = None
+    """The active `PlayOrder`"""
 
     sourced = False
     """True in case this model is the source of the currently playing song"""
 
-    def __init__(self, order_cls: Type[Order] = OrderInOrder):
+    def __init__(self, order_cls=OrderInOrder):
         super().__init__(object)
         self.order = order_cls()
 
+        # The playorder plugins use paths atm to remember songs so
+        # we need to reset them if the paths change somehow.
+        self.__sigs = []
+        for sig in ['row-deleted', 'row-inserted', 'rows-reordered']:
+            s = self.connect(sig, lambda pl, *x: self.order.reset(pl))
+            self.__sigs.append(s)
+
     def next(self):
         """Switch to the next song"""
 
@@ -302,11 +311,15 @@ def go_to(self, song_or_iter, explicit=False, source=None):
 
         return self.current_iter
 
-    def set(self, songs: Sequence[Any]):
+    def set(self, songs):
         """Clear the model and add the passed songs"""
 
         self.order.reset(self)
+        for signal_id in self.__sigs:
+            self.handler_block(signal_id)
         super().set(songs)
+        for signal_id in self.__sigs:
+            self.handler_unblock(signal_id)
 
     def reset(self):
         """Switch to the first song"""