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

Music channel on Discord only accepts www.youtube.com as host name #4

Closed
simonthor opened this issue Mar 25, 2020 · 2 comments · Fixed by #5
Closed

Music channel on Discord only accepts www.youtube.com as host name #4

simonthor opened this issue Mar 25, 2020 · 2 comments · Fixed by #5
Assignees
Labels
bug Something isn't working

Comments

@simonthor
Copy link
Contributor

Describe the bug
music channel on discord only accepts www.youtube.com host name, not e.g. youtu.be, a shorter version of the same URL.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Digital Ungdom's discord server (https://discordapp.com/channels/468044152970674176)
  2. Click on #music group (bottom left).
  3. Enter $play https://youtu.be/dQw4w9WgXcQ in the text bar and press enter.
  4. The message is deleted immediately.

Expected behavior
The music should be played instead of deleting the message.

Screenshots
https://gyazo.com/ff1ce8687001a3f7aa6b64afb8072fc5 (made by @simpmaestro on Discord)

Possible solutions

  1. add "youtu.be" to host name check. This solution is simple but not scalable. For example, if we later would like to add a feature where it can play shortened links in general, the list of allowed links would become too long.
  2. Check if link contains a video or not. This solution requires a lot more coding but is more scalable. This can for example look for special HTML tags such as <iframe> or
@kelszo kelszo self-assigned this Mar 26, 2020
@kelszo kelszo added the bug Something isn't working label Mar 26, 2020
@kelszo
Copy link
Member

kelszo commented Mar 26, 2020

Another common youtube host is m.youtube.com (for mobiles). The current pull request (#5) should be updated to support this.

@simonthor
Copy link
Contributor Author

Another common youtube host is m.youtube.com (for mobiles). The current pull request (#5) should be updated to support this.

Good point. I fixed this in the latest commit (see ee54a5b).

@kelszo kelszo closed this as completed in #5 Mar 26, 2020
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

Successfully merging a pull request may close this issue.

2 participants