Grouped tracks are considered as a single track.

[Vinzzzze]

consider the grouped tracks as one track so the queued tracks cannot interrupt the grouped tracks play :

• add a parameter to the Playlist::next_row method to handle this
• add a method Song::IsOnSameGrouping
• add the attribute next_song_after_queued_ to remember the position where to go back when the queued rest is finish

add a configuration to define the behavior of the queued tracks in front of the grouped tracks :

• add the GROUPED_BEFORE_QUEUE_DEFAULT constexpr that define the default value for this configuration (default value set to 1 to keep the current behavior and ignore the grouped tracks)
• add the update_grouped_before_queue method to set the playlist objects with the value set in the ui
• use left_grouped_song_before_queue_ & init_grouped_song_before_queue_ to determine what to do next when we have queued tracks while reading grouped tracks
• add the ui in the backendsettingspage,in the background management tab.
• add a tool-tip on the created spinbox to explain how this parameter is used.
• translations had been prepared, but only the french one had been done.

correct the previous row to navigate in the playlist :

• update the Playlist::previous_row code
• use only the navigation in the playlist to determine the previous row
• if we are on queued track read, use next_song_after_queued_ attribute to determine the position where to go back (that is to say that queued tracks are ignored in this navigation)

Values for the added parameter :
0 : wait for the end of all the grouped tracks on the same group than the current read track before playing the queued track(s)
1 : ignore the grouping attribute and read the queued track after the end of the current played track
n : wait for the end of the read of the n tracks of the current played track (the current one is the first of the n). If there is less than n grouped tracks to read, wait for the end of all the grouped tracks (same group than the current read track) to start the read of the queued track(s). If the read track is not a grouped track, the queued track(s) will be read just after it.
No maximum had been set.
I will use it this 0 : never interrupt grouped tracks.

Summary by CodeRabbit

• New Features

  • Added a “Grouping Before Queue” option to playlist preferences, including a descriptive tooltip and a persisted numeric setting.

• Improved Playback Behavior

  • Enhanced grouped-track queue handling so queued playback returns/resumes more consistently within matching album/grouping contexts.
  • Improved “next song” state management when navigating tracks and when reshuffling or mutating playlists, preventing stale queued-next behavior.

[jonaski]

I'm not sure if I want to add this, have to think about it. I doubt many users use shuffle by grouping and if we were to add something like this it makes sense to have the option for album shuffle too, not just grouping.
One thing I see: There is no need to bring the settings logic into SmartPlaylistSearchPreview, it only uses the Playlist model for view, no play logic is used there.

[jonaski]

Another thing, please don't add translations updates into the PR's, it's just unnecessary noise, it's better to update the translation sources with make update_translations separately after the PR's are merged and let Crowin fill in the new tr's for all the languages.

[Vinzzzze]

Hello,
I removed the translation files, then with the «make update_translations» I can regenerate the en_US file, and I have to create a PR with this file alone ?

For the SmartPlaylistSearchPreview, I use default parameter value. As it is a QObject derivated class, I left the Parent parameter at last position.

[coderabbitai]

🧹 Nitpick comments (2)

src/settings/playlistsettingspage.cpp (2)

58-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Improve tooltip localization quality by translating full sentences.

Current text is fragmented ("0:"/"1:" outside translation + leading-space sentence fragments), which is harder to localize cleanly and produces awkward English phrasing.

Suggested refactor

-         "<span style=\"font-weight:600;\">0:</span> "_L1 +
-         QObject::tr(" is used to say that the queued track will wait for the end of the current track group before being played.") +
+         QObject::tr("<span style=\"font-weight:600;\">0:</span> Play all tracks in the current group before queued tracks.") +
...
-         "<span style=\"font-weight:600;\">1:</span> "_L1 +
-         QObject::tr(" is used to say that the queued track will be played after the end of the current track, whatever it belongs to a group or not.") +
+         QObject::tr("<span style=\"font-weight:600;\">1:</span> Play queued tracks after the current track ends.") +

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/settings/playlistsettingspage.cpp` around lines 58 - 75, The tooltip in
PlaylistSettingsPage::GroupingBeforeQueueToolTip is split into fragmented
translated pieces, making localization awkward and the English phrasing
unnatural. Refactor the HTML text so each explanation is a full translatable
sentence, including the “0:” and “1:” meanings and the surrounding text, rather
than concatenating leading-space fragments and partial clauses. Keep the same
tooltip content, but build it from complete QObject::tr strings so translators
can reorder and adapt the sentences cleanly.

---

34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Decouple settings UI from playlist/playlist.h for default value ownership.

Pulling GROUPED_BEFORE_QUEUE_DEFAULT from playlist/playlist.h adds an avoidable UI→playlist dependency. Consider moving that default to playlist settings constants (next to kGroupingBeforeQueue) so this page only includes settings-layer headers.

Suggested refactor

-#include "playlist/playlist.h"
+#include "constants/playlistsettings.h"

-  ui_->spinbox_grouping_before_queue->setValue(s.value(kGroupingBeforeQueue, GROUPED_BEFORE_QUEUE_DEFAULT).toInt());
+  ui_->spinbox_grouping_before_queue->setValue(
+      s.value(kGroupingBeforeQueue, kGroupingBeforeQueueDefault).toInt());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/settings/playlistsettingspage.cpp` at line 34, The playlist settings page
is depending directly on playlist/playlist.h just to read
GROUPED_BEFORE_QUEUE_DEFAULT; move that default into the settings/constants
layer alongside kGroupingBeforeQueue so playlistsettingspage.cpp only includes
settings-layer headers. Update the relevant settings constant definition and any
references in the playlist settings UI to use the new settings-owned default
symbol, removing the playlist header include from this page.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/settings/playlistsettingspage.cpp`:
- Around line 58-75: The tooltip in
PlaylistSettingsPage::GroupingBeforeQueueToolTip is split into fragmented
translated pieces, making localization awkward and the English phrasing
unnatural. Refactor the HTML text so each explanation is a full translatable
sentence, including the “0:” and “1:” meanings and the surrounding text, rather
than concatenating leading-space fragments and partial clauses. Keep the same
tooltip content, but build it from complete QObject::tr strings so translators
can reorder and adapt the sentences cleanly.
- Line 34: The playlist settings page is depending directly on
playlist/playlist.h just to read GROUPED_BEFORE_QUEUE_DEFAULT; move that default
into the settings/constants layer alongside kGroupingBeforeQueue so
playlistsettingspage.cpp only includes settings-layer headers. Update the
relevant settings constant definition and any references in the playlist
settings UI to use the new settings-owned default symbol, removing the playlist
header include from this page.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c60375b7-ff2b-4496-9f27-42ce871825ac

📥 Commits

Reviewing files that changed from the base of the PR and between 7b09eb9 and cd02e05.

📒 Files selected for processing (12)

• src/constants/playlistsettings.h
• src/core/player.cpp
• src/core/song.cpp
• src/core/song.h
• src/playlist/playlist.cpp
• src/playlist/playlist.h
• src/playlist/playlistmanager.cpp
• src/playlist/playlistmanager.h
• src/settings/playlistsettingspage.cpp
• src/settings/playlistsettingspage.h
• src/settings/playlistsettingspage.ui
• src/smartplaylists/smartplaylistsearchpreview.cpp

✅ Files skipped from review due to trivial changes (1)

• src/settings/playlistsettingspage.h

🚧 Files skipped from review as they are similar to previous changes (10)

• src/smartplaylists/smartplaylistsearchpreview.cpp
• src/constants/playlistsettings.h
• src/playlist/playlistmanager.cpp
• src/core/song.h
• src/core/song.cpp
• src/core/player.cpp
• src/playlist/playlistmanager.h
• src/settings/playlistsettingspage.ui
• src/playlist/playlist.h
• src/playlist/playlist.cpp

[Vinzzzze]

Can you please confirm that you found the suggested refactor more readable ? For me, it is not the case, but if you agree I will do it.
The suggested refactor I am talking about is :

-         "<span style=\"font-weight:600;\">0:</span> "_L1 +
-         QObject::tr(" is used to say that the queued track will wait for the end of the current track group before being played.") +
+         QObject::tr("<span style=\"font-weight:600;\">0:</span> Play all tracks in the current group before queued tracks.") +
...
-         "<span style=\"font-weight:600;\">1:</span> "_L1 +
-         QObject::tr(" is used to say that the queued track will be played after the end of the current track, whatever it belongs to a group or not.") +
+         QObject::tr("<span style=\"font-weight:600;\">1:</span> Play queued tracks after the current track ends.")