Fixes recently played songs list to consistently display in correct order #742
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #713 where the recently played songs list was not displaying in the correct order. The fix addresses an inconsistency between the initial data load (which returns a reversed list) and the SignalR real-time updates (which send the list in chronological order).
Key Changes:
- Added
Reverse()call to the SignalR event handler to ensure real-time updates display songs in the same order as the initial load
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Psychoboy I've opened a new pull request, #743, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Use LINQ Reverse().ToList() to avoid mutating original songs list Co-authored-by: Psychoboy <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Psychoboy <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| recentlyPlayedSongs = songs.ToList(); | ||
| recentlyPlayedSongs.Reverse(); |
There was a problem hiding this comment.
While this fix works to maintain consistent ordering between initial load and SignalR updates, it would be more maintainable to fix this at the source in YtPlayer.cs. The SendLastPlayedSongs method should reverse the list before sending (like GetRecentlyPlayedSongs does on line 886 of YtPlayer.cs), ensuring all clients receive consistently ordered data. This client-side reversal creates a discrepancy where GetRecentlyPlayedSongs returns reversed data but SendLastPlayedSongs sends unreversed data, requiring all clients to know about and handle this inconsistency.
fixes #713