-
Notifications
You must be signed in to change notification settings - Fork 741
Adjust: Allow repeat in combination with shuffle #1561
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
Adjust: Allow repeat in combination with shuffle #1561
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables the combination of repeat and shuffle modes in the Spotify Connect implementation without odd behaviors. The changes restructure how shuffling works to be more predictable and consistent when combined with repeat functionality.
Key changes:
- Introduced
ShuffleState
struct to track shuffle seed and initial track together - Modified shuffle logic to preserve the first track position for consistency across state resets
- Removed restrictions that prevented combining repeat and shuffle modes
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
connect/src/state/transfer.rs | Updates transfer logic to handle ShuffleState instead of just seed |
connect/src/state/options.rs | Adds ShuffleState struct and refactors shuffle method to use it |
connect/src/state/restrictions.rs | Removes restriction preventing shuffle when repeat context is enabled |
connect/src/state/handle.rs | Simplifies repeat context handling logic |
connect/src/state/context.rs | Updates skip track logic and removes unused skip_track field |
connect/src/state/tracks.rs | Adds repeat context check and improves skip track handling |
connect/src/state/metadata.rs | Adds initial track metadata support |
connect/src/state.rs | Updates state structure and error naming |
connect/src/shuffle_vec.rs | Enhances shuffle algorithm to preserve first track position |
connect/src/context_resolver.rs | Adds condition to prevent reshuffling in certain scenarios |
CHANGELOG.md | Documents the changes and fixes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some ideas for your consideration.
Tested your branch for my project and it works great for all combinations of shuffle and repeat. Thanks again for putting up this PR! |
Great! @photovoltex no pressure but if you're OK to merge, then by all means do, and we can release v0.7.1. |
Great to here. Then I suppose we can merge it :D |
* fix: incorrect autoplay resolver behavior when shuffling * refactor: store the initial track in the remote context * adjust: shuffle repeat interaction * chore: update .gitignore * chore: rename internal error * adjust: shuffle behavior to ensure consistency * fix: prefer repeat context over autoplay * chore: update changelog * chore: reduce complexity of shuffle * chore: test shuffle with first
This should allow to combine repeat and shuffle without any odd behaviors. Shuffling itself was quite a bit inconsistent before, so I had to adjust the shuffling a bit so that's a bit more predictable to work with.
@aome510 could you give this a try and see if there is anything weird in the combination or how the queue behaves? Thanks in advance :)
Fixes #1550