Androz2091 / discord-player

🎧 Complete framework to simplify the implementation of music commands using discord.js v14
https://discord-player.js.org/
MIT License
605 stars 191 forks source link

Playlist "share" link doesn't get parsed properly #1808

Closed Crocross closed 1 year ago

Crocross commented 1 year ago

Describe the bug If you grab the share link of a YT playlist it doesn't seem to be able to resolve it (without manually removing the share portion).

To Reproduce Steps to reproduce the behavior:

  1. Go to a YT playlist page (ex. https://www.youtube.com/playlist?list=PLRxX1Jhp-oqUhk_VQPuyVxwVhRPeuxNYQ)
  2. Click on the share icon and copy the link (ex. https://youtube.com/playlist?list=PLRxX1Jhp-oqUhk_VQPuyVxwVhRPeuxNYQ&si=9vQkdM4MnJl_H6HZ)
  3. Try to use that link in a bot
  4. If you remove the &si part if towrks (ex. https://youtube.com/playlist?list=PLRxX1Jhp-oqUhk_VQPuyVxwVhRPeuxNYQ)

Expected behavior The link is parsed and resolves correctly.

Please complete the following information:

Additional context I tested a tracks share link and that works with no issue although the link format is different as well for tracks: https://youtu.be/BnSkt6V3qF0?si=eecB9Cbpaa_rBT-E

twlite commented 1 year ago

issue lies in https://github.com/Androz2091/discord-player/blob/master/packages/discord-player/src/utils/QueryResolver.ts

Crocross commented 1 year ago

Before I get into the options I see possible I'll differentiate the type of URL patterns I noticed.

The difference in the URL pattern is whether or not the list= is the first URL parameter or not.

I think for the case of the first URL type it makes sense to always return a playlist. For the second URL type there are a few ways to handle it:

  1. Return the playlist the track belongs to
  2. Return the track only
  3. Return the playlist followed by the track(s) on the playlist

I don't know how easy/difficult it is to do the third option but I added it anyhow. The other changes would be minor and shouldn't be very difficult.

twlite commented 1 year ago

We need to rewrite query resolver. I think it makes sense to instantiate URL object for further validation instead of depending upon plain old regex only? What do you think

twlite commented 1 year ago

Query resolver can then return modified url that extractors can process. This way, we have to only update discord-player package.

Query resolver has not been updated properly since like version 4 or something. I think the time has come 😄