Skip to content
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

Fix player sync #777

Merged
merged 2 commits into from Mar 27, 2022
Merged

Fix player sync #777

merged 2 commits into from Mar 27, 2022

Conversation

larslindnilsson
Copy link
Contributor

When calling the sync command through the CLI with an unknown playerid, the first player in the clients array would be added (instead of ignoring the unknown playerid).

This PR should fix that.

@michaelherger
Copy link
Member

Thanks for your PR! Can you give me an example request which would show the wrong behaviour?

@larslindnilsson
Copy link
Contributor Author

larslindnilsson commented Mar 24, 2022

00:04:20:1e:e4:a0 sync 00:04:20:06:fd:5e

The first player is the one I use as "master". And if the second player is turned off (disconnected), a random of my other players would be added to the sync group (based on the order of the players in the clients array). It can also be reproduced by using a random MAC as the second player.

Comment on lines 3079 to 3082
if (!defined $buddy) {
my @clients = Slim::Player::Client::clients();
if (defined $clients[$newbuddy]) {
if (looks_like_number($newbuddy) && defined $clients[$newbuddy]) {
$buddy = $clients[$newbuddy];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the new check to the outer condition. There's no need to read the @clients list if the $newbuddy isn't a number.

And feel free to update Changelog8.html too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the PR

Updated changelog
@mherger mherger merged commit d182ad6 into LMS-Community:public/8.3 Mar 27, 2022
@mherger
Copy link
Contributor

mherger commented Mar 27, 2022

Thanks!

@larslindnilsson larslindnilsson deleted the SyncFix branch March 27, 2022 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants