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 scrobble not working when play a playlist #3752

Merged
merged 1 commit into from May 25, 2023

Conversation

youthlin
Copy link
Contributor

Describe the changes you made

Since there is a replayDetectionTimer, So when the song played end, it would be set to replay, and call the pause() method. So that this.pausedOn will not be null.
Then will play the next song on the playlist, and call the start() method. However, when update() called, this.pausedOn == null is false, so this.setTrigger is not called. This make the 2nd and the rest in the playlist would NOT be scrobbled.

So when start called, we should set this.pausedOn to null~

Additional context

Since there is a replayDetectionTimer, So when the song played end, it would be set to replay, and call the `pause()` method.
So that `this.pausedOn` will not be null.
Then will play the next song on the playlist, and call the `start()` method.
However, when `update()` called, `this.pausedOn == null` is false, so `this.setTrigger` is not called. This make the 2nd and the rest in the playlist would NOT be scrobbled.
@youthlin youthlin changed the title Update timer.ts fix scrobble not working when play a playlist May 25, 2023
@yayuyokitano
Copy link
Member

Normally the connector should not be pausing as it goes to a new song so I think this bug might only be happening with a handful of connectors.

This fix should make the core more robust, and I don’t see any situation where it can break anything, so LGTM, but I want another reviewer to look at it too in case I missed some obscure gotcha.

@yayuyokitano yayuyokitano added core This issue or pull request is related to the extension core fixed labels May 25, 2023
@youthlin
Copy link
Contributor Author

Normally the connector should not be pausing as it goes to a new song so I think this bug might only be happening with a handful of connectors.

This fix should make the core more robust, and I don’t see any situation where it can break anything, so LGTM, but I want another reviewer to look at it too in case I missed some obscure gotcha.

Yes, the 163-music connector watch the a[data-action="play"] element, when the played song changed, the button would changed from data-action="pause"(when play) to data-action="play"(when switch a song) and to data-action="pause"(when the new song played)
image

@yayuyokitano yayuyokitano requested a review from inverse May 25, 2023 21:09
Copy link
Member

@inverse inverse left a comment

Choose a reason for hiding this comment

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

Can't think of any edge cases :/

@yayuyokitano yayuyokitano merged commit ec757e0 into web-scrobbler:master May 25, 2023
9 of 10 checks passed
@youthlin youthlin deleted the patch-1 branch May 26, 2023 07:29
@inverse inverse mentioned this pull request May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core This issue or pull request is related to the extension core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants