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

seeking in ended queue somehow corrupts it #94

Open
Whuppee opened this issue Oct 2, 2022 · 3 comments
Open

seeking in ended queue somehow corrupts it #94

Whuppee opened this issue Oct 2, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@Whuppee
Copy link
Collaborator

Whuppee commented Oct 2, 2022

queue had 1 song in it and had ended (not ephemeral, not looping)

tried seeking to a part of the song to restart. immediate responses and subsequent websockets show an empty queue, and /queue show errors in discord with this output. playhead must still be one, else the length check would prevent this

image

working on other things, so noting for later

@Whuppee Whuppee added the bug Something isn't working label Oct 2, 2022
@Whuppee Whuppee self-assigned this Oct 2, 2022
@Ethazeriel
Copy link
Owner

Changing the return type of getCurrent() to properly include the possibility of returning undefined makes fixing this easy - do we want to special case the seek command to call prev() if getCurrent() returns null? I assume yes and have done so in a9c4068 but my solution isn't particularly elegant - may want to review and modify

@Whuppee
Copy link
Collaborator Author

Whuppee commented Oct 4, 2022

Note that the command would fail too and we need to look through for other places we've forgotten to check for current.

Per our conversation, I think move the playhead in the same way prev() does without calling it, then seek.

I didn't give it thought at the time, but this started with the Status/Media Bar not knowing there wasn't a current track; the queue ending should probably trigger a web sync. Inelegant, but could compare the playhead to length in the idle event.

Feels intuitive (to me) in the browser, probably not in Discord, but tend towards allowing it. May still want to sync the queue ending for other presentation reasons, but seeking won't feel right under 'Current Track: None'. Unsure if we'd want to report a current track falsely there.

@Ethazeriel
Copy link
Owner

I don't remember what we discussed about implementation details, but I've made a first attempt in 549e1dc to at least not call play() twice.

have not yet tackled the websyncs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants