Add a sub-menu to the shuffle playlist menu to decide how to shuffle.

src/core/mainwindow.cpp

@@ -367,6 +367,7 @@ MainWindow::MainWindow(Application *app,
       collection_show_duplicates_(nullptr),
       collection_show_untagged_(nullptr),
       playlist_menu_(new QMenu(this)),
+      shuffle_playlist_menu_(new QMenu(this)),
       playlist_play_pause_(nullptr),
       playlist_stop_after_(nullptr),
       playlist_undoredo_(nullptr),
@@ -592,7 +593,6 @@ MainWindow::MainWindow(Application *app,
   QObject::connect(ui_->action_toggle_show_sidebar, &QAction::toggled, this, &MainWindow::ToggleSidebar);
   QObject::connect(ui_->action_about_strawberry, &QAction::triggered, this, &MainWindow::ShowAboutDialog);
   QObject::connect(ui_->action_about_qt, &QAction::triggered, qApp, &QApplication::aboutQt);
-  QObject::connect(ui_->action_shuffle, &QAction::triggered, &*app_->playlist_manager(), &PlaylistManager::ShuffleCurrent);
   QObject::connect(ui_->action_open_file, &QAction::triggered, this, &MainWindow::AddFile);
   QObject::connect(ui_->action_open_cd, &QAction::triggered, this, &MainWindow::AddCDTracks);
   QObject::connect(ui_->action_add_file, &QAction::triggered, this, &MainWindow::AddFile);
@@ -628,10 +628,20 @@ MainWindow::MainWindow(Application *app,
   ui_->button_scrobble->setDefaultAction(ui_->action_toggle_scrobbling);
   ui_->button_love->setDefaultAction(ui_->action_love);
 
+  // Shuffle playlist sub-menu
+  QActionGroup *shuffle_playlist_group = new QActionGroup(this);
+  shuffle_playlist_group->addAction(ui_->action_shuffle_playlist_all);
+  shuffle_playlist_group->addAction(ui_->action_shuffle_playlist_albums);
+  shuffle_playlist_group->addAction(ui_->action_shuffle_playlist_grouping);
+  shuffle_playlist_menu_->addActions(shuffle_playlist_group->actions());
+
+  QObject::connect(shuffle_playlist_group, &QActionGroup::triggered, this, &MainWindow::ShufflePlaylistActionTriggered);
+
   ui_->playlist->SetActions(ui_->action_new_playlist, ui_->action_load_playlist, ui_->action_save_playlist, ui_->action_clear_playlist, ui_->action_next_playlist, /* These two actions aren't associated */ ui_->action_previous_playlist /* to a button but to the main window */, ui_->action_save_all_playlists);
   // Add the shuffle and repeat action groups to the menu
   ui_->action_shuffle_mode->setMenu(ui_->playlist_sequence->shuffle_menu());
   ui_->action_repeat_mode->setMenu(ui_->playlist_sequence->repeat_menu());
+  ui_->action_shuffle->setMenu(shuffle_playlist_menu_);
 
   // Stop actions
   QMenu *stop_menu = new QMenu(this);
@@ -3620,3 +3630,13 @@ void MainWindow::ProcessMetadataQueue() {
   }
 
 }
+
+void MainWindow::ShufflePlaylistActionTriggered(QAction *action) {
+
+  PlaylistSequence::ShuffleMode mode = PlaylistSequence::ShuffleMode::All;
+  if (action == ui_->action_shuffle_playlist_albums) mode = PlaylistSequence::ShuffleMode::Albums;
+  if (action == ui_->action_shuffle_playlist_grouping) mode = PlaylistSequence::ShuffleMode::Grouping;
+
+  app_->playlist_manager()->ShuffleCurrent(mode);
+
+}

src/core/mainwindow.h

@@ -277,6 +277,8 @@ class MainWindow : public QMainWindow, public PlatformInterface {
   void FetchStreamingMetadata();
   void ProcessMetadataQueue();
 
+  void ShufflePlaylistActionTriggered(QAction *action);
+
  public Q_SLOTS:
   void CommandlineOptionsReceived(const QByteArray &string_options);
   void Raise();
@@ -362,6 +364,7 @@ class MainWindow : public QMainWindow, public PlatformInterface {
   QAction *collection_show_untagged_;
 
   QMenu *playlist_menu_;
+  QMenu *shuffle_playlist_menu_;
   QAction *playlist_play_pause_;
   QAction *playlist_stop_after_;
   QAction *playlist_undoredo_;

src/core/mainwindow.ui

@@ -662,9 +662,6 @@
    <property name="text">
     <string>S&amp;huffle playlist</string>
    </property>
-   <property name="shortcut">
-    <string>Ctrl+H</string>
-   </property>
   </action>
   <action name="action_add_file">
    <property name="text">
@@ -863,6 +860,33 @@
     <string>Import data from last.fm...</string>
    </property>
   </action>
+  <action name="action_shuffle_playlist_all">
+   <property name="checkable">
+    <bool>false</bool>
+   </property>
+   <property name="text">
+    <string>Single track as random element</string>
+   </property>
+   <property name="shortcut">
+    <string>Ctrl+H</string>
+   </property>
+  </action>
+  <action name="action_shuffle_playlist_albums">
+   <property name="checkable">
+    <bool>false</bool>
+   </property>
+   <property name="text">
+    <string>Album as random element</string>
+   </property>
+  </action>
+  <action name="action_shuffle_playlist_grouping">
+   <property name="checkable">
+    <bool>false</bool>
+   </property>
+   <property name="text">
+    <string>Grouped tracks as random element</string>
+   </property>
+  </action>
  </widget>
  <layoutdefault spacing="6" margin="11"/>
  <customwidgets>

src/playlist/playlist.cpp

@@ -2014,27 +2014,26 @@ void Playlist::ReloadItems(const QList<int> &rows) {
 
 }
 
-void Playlist::Shuffle() {
-
-  PlaylistItemPtrList new_items(items_);
+void Playlist::Shuffle(const PlaylistSequence::ShuffleMode shuffle_mode) {
 
   int begin = 0;
-  if (current_item_index_.isValid()) {
-    if (new_items[0] != new_items[current_item_index_.row()]) {
-      std::swap(new_items[0], new_items[current_item_index_.row()]);
-    }
-    begin = 1;
-  }
 
   if (dynamic_playlist_ && current_item_index_.isValid()) {
-    begin += current_item_index_.row() + 1;
+    begin += current_item_index_.row();
   }
 
+  QList<int> index_items;
   const int count = static_cast<int>(items_.count());
   for (int i = begin; i < count; ++i) {
-    const int new_pos = i + (rand() % (count - i));
+    index_items.push_back(i);
+  }
+
+  ReshuffleIndices(index_items, shuffle_mode, begin, true);
+
+  PlaylistItemPtrList new_items;
 
-    std::swap(new_items[i], new_items[new_pos]);
+  for (int idx_shuffle : std::as_const(index_items)) {
+    new_items.push_back(items_[idx_shuffle]);
   }
 
   undo_stack_->push(new PlaylistUndoCommandShuffleItems(this, new_items));
@@ -2048,7 +2047,29 @@ bool AlbumShuffleComparator(const QHash<QString, int> &album_key_positions, cons
   const int left_pos = album_key_positions[album_keys[left]];
   const int right_pos = album_key_positions[album_keys[right]];
 
+  // With this return, I keep the tracks in the order they have in the playlist
   if (left_pos == right_pos) return left < right;
+
+  // Sort the albums by their position in the playlist
+  return left_pos < right_pos;
+
+}
+
+// to the contrary of the function upper, I will keep the tracks in the order of their position in the album
+// not with the position they have in the playlist
+bool AlbumShuffleComparatorTrackOrder(const QHash<QString, int> &album_key_positions, const QHash<int, QString> &album_keys, const PlaylistItemPtrList &items, const int left, const int right) {
+
+  const int left_pos = album_key_positions[album_keys[left]];
+  const int right_pos = album_key_positions[album_keys[right]];
+
+  if (left_pos == right_pos) {
+    auto &&left_song = items[left]->EffectiveMetadata();
+    auto &&right_song = items[right]->EffectiveMetadata();
+
+    if (left_song.disc() == right_song.disc()) return left_song.track() < right_song.track();
+
+    return left_song.disc() < right_song.disc();
+  }
   return left_pos < right_pos;
 
 }
@@ -2057,18 +2078,38 @@ bool AlbumShuffleComparator(const QHash<QString, int> &album_key_positions, cons
 
 void Playlist::ReshuffleIndices() {
 
-  const PlaylistSequence::ShuffleMode shuffle_mode = ShuffleMode();
+  // First, cancel the replay position
+  // TODO : uncomment the code below when the branch «Grouped tracks are considered as single track» will be merged
+  // next_song_after_queued_ = -1;
+
+  current_virtual_index_ = ReshuffleIndices(virtual_items_, ShuffleMode(), 0, false);
+
+}
+
+int Playlist::ReshuffleIndices(QList<int>& virtual_items, const PlaylistSequence::ShuffleMode shuffle_mode, const int base_reference, const bool album_keep_track_order) {
+
+  static std::mt19937 rng{std::random_device{}()};
+
   switch (shuffle_mode) {
     case PlaylistSequence::ShuffleMode::Off:{
       // No shuffling - sort the virtual item list normally.
-      std::sort(virtual_items_.begin(), virtual_items_.end());
+      std::sort(virtual_items.begin(), virtual_items.end());
       break;
     }
 
     case PlaylistSequence::ShuffleMode::All:
     case PlaylistSequence::ShuffleMode::InsideAlbum:{
-      std::random_device rd;
-      std::shuffle(virtual_items_.begin(), virtual_items_.end(), std::mt19937(rd()));
+      std::shuffle(virtual_items.begin(), virtual_items.end(), rng);
+
+      // If the user is currently playing a song, force its track to be first
+      // Also check last_played_row() for cases where current_row() hasn't been set yet (e.g., on app startup)
+      int reference_row = current_row();
+      if (reference_row == -1 && last_played_row() != -1) {
+        reference_row = last_played_row();
+      }
+      if (reference_row > base_reference) {
+        std::swap(virtual_items[0], virtual_items[reference_row - base_reference]);
+      }
       break;
     }
 
@@ -2077,15 +2118,14 @@ void Playlist::ReshuffleIndices() {
       QSet<QString> album_key_set;     // unique keys
 
       // Find all the unique albums in the playlist
-      for (QList<int>::const_iterator it = virtual_items_.constBegin(); it != virtual_items_.constEnd(); ++it) {
+      for (QList<int>::const_iterator it = virtual_items.constBegin(); it != virtual_items.constEnd(); ++it) {
         const int index = *it;
         const QString key = items_[index]->EffectiveMetadata().AlbumKey();
         album_keys[index] = key;
         album_key_set << key;
       }
 
       // Shuffle them
-      static std::mt19937 rng{std::random_device{}()};
       QStringList shuffled_album_keys = album_key_set.values();
       std::shuffle(shuffled_album_keys.begin(), shuffled_album_keys.end(), rng);
 
@@ -2098,8 +2138,8 @@ void Playlist::ReshuffleIndices() {
       if (reference_row != -1) {
         const QString key = items_[reference_row]->EffectiveMetadata().AlbumKey();
         const qint64 pos = shuffled_album_keys.indexOf(key);
-        if (pos >= 1) {
-          std::swap(shuffled_album_keys[0], shuffled_album_keys[pos]);
+        if (pos > base_reference) {
+          std::swap(shuffled_album_keys[0], shuffled_album_keys[pos - base_reference]);
         }
       }
 
@@ -2109,8 +2149,18 @@ void Playlist::ReshuffleIndices() {
         album_key_positions[shuffled_album_keys[i]] = i;
       }
 
+      if (album_keep_track_order) {
+        // Sort the virtual items : use AlbumShuffleComparator with a cheap-to-copy lambda comparator
+        // force the track order to be the album one
+        std::stable_sort(virtual_items.begin(), virtual_items.end(), [&album_key_positions, &album_keys, this](const int lhs, const int rhs) {
+          return AlbumShuffleComparatorTrackOrder(album_key_positions, album_keys, items_, lhs, rhs);
+        });
+
+        break;
+      }
       // Sort the virtual items : use AlbumShuffleComparator with a cheap-to-copy lambda comparator
-      std::stable_sort(virtual_items_.begin(), virtual_items_.end(), [&album_key_positions, &album_keys](const int lhs, const int rhs) {
+      // keep the track order they have in the playlist
+      std::stable_sort(virtual_items.begin(), virtual_items.end(), [&album_key_positions, &album_keys](const int lhs, const int rhs) {
         return AlbumShuffleComparator(album_key_positions, album_keys, lhs, rhs);
       });
 
@@ -2121,15 +2171,14 @@ void Playlist::ReshuffleIndices() {
       QSet<QString> grouping_key_set;    // unique keys
 
       // Find all the unique grouping keys in the playlist
-      for (QList<int>::const_iterator it = virtual_items_.constBegin(); it != virtual_items_.constEnd(); ++it) {
+      for (QList<int>::const_iterator it = virtual_items.constBegin(); it != virtual_items.constEnd(); ++it) {
         const int index = *it;
         const QString key = items_[index]->EffectiveMetadata().GroupingKey();
         grouping_keys[index] = key;
         grouping_key_set << key;
       }
 
       // Shuffle them
-      static std::mt19937 rng{std::random_device{}()};
       QStringList shuffled_grouping_keys = grouping_key_set.values();
       std::shuffle(shuffled_grouping_keys.begin(), shuffled_grouping_keys.end(), rng);
 
@@ -2142,8 +2191,8 @@ void Playlist::ReshuffleIndices() {
       if (reference_row != -1) {
         const QString key = items_[reference_row]->EffectiveMetadata().GroupingKey();
         const qint64 pos = shuffled_grouping_keys.indexOf(key);
-        if (pos >= 1) {
-          std::swap(shuffled_grouping_keys[0], shuffled_grouping_keys[pos]);
+        if (pos > base_reference) {
+          std::swap(shuffled_grouping_keys[0], shuffled_grouping_keys[pos - base_reference]);
         }
       }
 
@@ -2153,22 +2202,23 @@ void Playlist::ReshuffleIndices() {
         grouping_key_positions[shuffled_grouping_keys[i]] = i;
       }
 
-      // Sort the virtual items: use AlbumShuffleComparator as a grouping-key comparator with a cheap-to-copy lambda
-      std::stable_sort(virtual_items_.begin(), virtual_items_.end(), [&grouping_key_positions, &grouping_keys](const int lhs, const int rhs) {
-        return AlbumShuffleComparator(grouping_key_positions, grouping_keys, lhs, rhs);
+      // Sort the virtual items: use AlbumShuffleComparatorTrackOrder as a grouping-key comparator with a cheap-to-copy lambda
+      // This sort method will keep the track number order so important in the grouped read
+      std::stable_sort(virtual_items.begin(), virtual_items.end(), [&grouping_key_positions, &grouping_keys, this](const int lhs, const int rhs) {
+        return AlbumShuffleComparatorTrackOrder(grouping_key_positions, grouping_keys, items_, lhs, rhs);
       });
       break;
     }
   }
 
-  // Update current virtual index
+  // I keep the computation of the virtual index because it is only return, if you want to ignore it, you can
+  // besides, it will be usefull in the cases I want to use the updated method
   if (current_item_index_.isValid()) {
-    current_virtual_index_ = static_cast<int>(virtual_items_.indexOf(current_item_index_.row()));
-  }
-  else {
-    current_virtual_index_ = -1;
+    return static_cast<int>(virtual_items.indexOf(current_item_index_.row()));
   }
 
+  return -1;
+
 }
 
 void Playlist::set_sequence(PlaylistSequence *v) {

src/playlist/playlist.h

@@ -290,6 +290,8 @@ class Playlist : public QAbstractListModel {
 
   void ItemReload(const QPersistentModelIndex &idx, const bool metadata_edit);
 
+  void Shuffle(const PlaylistSequence::ShuffleMode shuffle_mode = PlaylistSequence::ShuffleMode::All);
+
  public Q_SLOTS:
   void set_current_row(const int i, const Playlist::AutoScroll autoscroll = Playlist::AutoScroll::Maybe, const bool is_stopping = false, const bool force_inform = false);
   void Paused();
@@ -303,7 +305,6 @@ class Playlist : public QAbstractListModel {
   void Clear();
   void RemoveDuplicateSongs();
   void RemoveUnavailableSongs();
-  void Shuffle();
 
   void ShuffleModeChanged(const PlaylistSequence::ShuffleMode shuffle_mode);
 
@@ -356,6 +357,8 @@ class Playlist : public QAbstractListModel {
   void MoveItemsWithoutUndo(int start, const QList<int> &dest_rows);
   void ReOrderWithoutUndo(const PlaylistItemPtrList &new_items);
 
+  int ReshuffleIndices(QList<int>& virtual_items, const PlaylistSequence::ShuffleMode shuffle_mode, const int base_reference, const bool album_keep_track_order);
+
   void RemoveItemsNotInQueue();
 
   // Removes rows with given indices from this playlist.

src/playlist/playlistmanager.cpp

@@ -409,6 +409,10 @@ void PlaylistManager::ShuffleCurrent() {
   current()->Shuffle();
 }
 
+void PlaylistManager::ShuffleCurrent(const PlaylistSequence::ShuffleMode shuffle_mode) {
+  current()->Shuffle(shuffle_mode);
+}
+
 void PlaylistManager::RemoveDuplicatesCurrent() {
   current()->RemoveDuplicateSongs();
 }

src/playlist/playlistmanager.h

@@ -94,6 +94,8 @@ class PlaylistManager : public PlaylistManagerInterface {
   PlaylistParser *parser() const override { return parser_; }
   PlaylistContainer *playlist_container() const override { return playlist_container_; }
 
+  void ShuffleCurrent(const PlaylistSequence::ShuffleMode shuffle_mode) override;
+
  public Q_SLOTS:
   void New(const QString &name, const SongList &songs = SongList(), const QString &special_type = QString()) override;
   void Load(const QString &filename) override;
@@ -118,6 +120,7 @@ class PlaylistManager : public PlaylistManagerInterface {
 
   // Convenience slots that defer to either current() or active()
   void ClearCurrent() override;
+  // TODO : this method is only used for external unit tests : the version with parameter is the one that is used.
   void ShuffleCurrent() override;
   void RemoveDuplicatesCurrent() override;
   void RemoveUnavailableCurrent() override;