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

feat: Autocompletion to play command #1108

Merged
merged 6 commits into from Jan 22, 2023
Merged

Conversation

XstreamSpeed
Copy link
Contributor

@XstreamSpeed XstreamSpeed commented Oct 22, 2022

Please describe the changes this PR makes and why it should be merged:
This pull request adds a common feature within music bots, autocomplete for searching, it first searches for the song and once the track is loaded, it responds with options, this works with Spotify, SoundCloud and YouTube playlists as well, the code has been tested numerous times to avoid any errors.

If you have anything which you would like to be changes, please comment it below.

Status and versioning classification:
NodeJS: 18x
DiscordJS: 13.8.0

  • Code changes have been tested against the Discord API, or there are no code changes

Preview

For songs:
image

For playlists:
image

For playlists if you don't choose one of the options:
image

@XstreamSpeed XstreamSpeed changed the title Add autocomplete to play.js - interactionCreate.js Add autocompletion to play command Oct 22, 2022
@GazorHuman
Copy link
Contributor

Hi, did u forget that 'await' expressions are only allowed within async functions? you can fix that by adding async in line 8 interactionCreate.js
like this module.exports = async (client, interaction) => {

Copy link
Owner

@SudhanPlayz SudhanPlayz left a comment

Choose a reason for hiding this comment

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

fix the changes and commit

@SudhanPlayz SudhanPlayz changed the title Add autocompletion to play command feat: Autocompletion to play command Oct 23, 2022
@Neko-Life
Copy link
Contributor

Sounds like a good idea. Only a few questions.

What's with that huge regex match? Won't there will always be track uri in the search result? If the regex matches then every option will have the same exact link that the user provided but might have different track title, what's the point in auto completing that?

Using switch case in there is kinda redundant and messy. What if we want to update many other commands to have autocomplete? You can extend the SlashCommand class to add autocomplete handler if it doesn't have the functionality already or just put the code in the run method instead of switching inside the event handler to make it look cleaner and more manageable.

@XstreamSpeed
Copy link
Contributor Author

XstreamSpeed commented Oct 23, 2022

Hi, did u forget that 'await' expressions are only allowed within async functions? you can fix that by adding async in line 8 interactionCreate.js like this module.exports = async (client, interaction) => {

Hmm, I must've forgotten it, will add it, thank you

@XstreamSpeed
Copy link
Contributor Author

Sounds like a good idea. Only a few questions.

What's with that huge regex match? Won't there will always be track uri in the search result? If the regex matches then every option will have the same exact link that the user provided but might have different track title, what's the point in auto completing that?

Using switch case in there is kinda redundant and messy. What if we want to update many other commands to have autocomplete? You can extend the SlashCommand class to add autocomplete handler if it doesn't have the functionality already or just put the code in the run method instead of switching inside the event handler to make it look cleaner and more manageable.

For your first question, I added it so that users only search within those given regexs', if you see closely you will notice it has checks for YouTube, SoundCould, Spotify, Deezer, Facebook and Apple from which this bot can play music from, its just an extra check to prevent someone from playing content from other sites.

For your second question, switch case may or may not be redundant or messy but it definitely looks ok-ish to me, if you want to add autocomplete to other commands as well, you can still do it using the switch case thing, here is an example, and yes, we could extend the SlashCommand class to have an autocomplete handler but for now I won't be doing that as I do not have too great of an understanding with the structures, but you or any other contributor is welcome to do so.

@Neko-Life
Copy link
Contributor

Sounds like a good idea. Only a few questions.
What's with that huge regex match? Won't there will always be track uri in the search result? If the regex matches then every option will have the same exact link that the user provided but might have different track title, what's the point in auto completing that?
Using switch case in there is kinda redundant and messy. What if we want to update many other commands to have autocomplete? You can extend the SlashCommand class to add autocomplete handler if it doesn't have the functionality already or just put the code in the run method instead of switching inside the event handler to make it look cleaner and more manageable.

For your first question, I added it so that users only search within those given regexs', if you see closely you will notice it has checks for YouTube, SoundCould, Spotify, Deezer, Facebook and Apple from which this bot can play music from, its just an extra check to prevent someone from playing content from other sites.

For your second question, switch case may or may not be redundant or messy but it definitely looks ok-ish to me, if you want to add autocomplete to other commands as well, you can still do it using the switch case thing, here is an example, and yes, we could extend the SlashCommand class to have an autocomplete handler but for now I won't be doing that as I do not have too great of an understanding with the structures, but you or any other contributor is welcome to do so.

"an extra check" why should you wanna limit people from playing whatever they want? Do you think lavalink will magically able to play every song from every existing platform? The play command already handle this and it handles it right by using lavalink's status code, lavalink is the one who decides if the bot can play the link or not. And FYI your regexes doesn't check for youtube and soundcloud link as you claim. Just what's this "fancy unecessary bloat regexes" are for really? Add more mess to your code?

Your "ok-ish" code hurts my eyes. It's a literal mess, it's so dense and you didn't even indent it properly. Where did you get this from? StackOverflow?

Move the code elsewhere and format it properly. If you wanna do it properly learn how to extend a class.


if (
focused.value.match(
/(?:https:\/\/open\.spotify\.com\/|spotify:)(?:.+)?(track|playlist|artist|episode|show|album)[\/:]([A-Za-z0-9]+)/ ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't even work as expected. This will only check for spotify link. Hilarious

Copy link
Collaborator

Choose a reason for hiding this comment

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

/^.*(youtu.be\/|list=)([^#\&\?]*).*/, this should check youtube link

@XstreamSpeed
Copy link
Contributor Author

Sounds like a good idea. Only a few questions.
What's with that huge regex match? Won't there will always be track uri in the search result? If the regex matches then every option will have the same exact link that the user provided but might have different track title, what's the point in auto completing that?
Using switch case in there is kinda redundant and messy. What if we want to update many other commands to have autocomplete? You can extend the SlashCommand class to add autocomplete handler if it doesn't have the functionality already or just put the code in the run method instead of switching inside the event handler to make it look cleaner and more manageable.

For your first question, I added it so that users only search within those given regexs', if you see closely you will notice it has checks for YouTube, SoundCould, Spotify, Deezer, Facebook and Apple from which this bot can play music from, its just an extra check to prevent someone from playing content from other sites.
For your second question, switch case may or may not be redundant or messy but it definitely looks ok-ish to me, if you want to add autocomplete to other commands as well, you can still do it using the switch case thing, here is an example, and yes, we could extend the SlashCommand class to have an autocomplete handler but for now I won't be doing that as I do not have too great of an understanding with the structures, but you or any other contributor is welcome to do so.

"an extra check" why should you wanna limit people from playing whatever they want? Do you think lavalink will magically able to play every song from every existing platform? The play command already handle this and it handles it right by using lavalink's status code, lavalink is the one who decides if the bot can play the link or not. And FYI your regexes doesn't check for youtube and soundcloud link as you claim. Just what's this "fancy unecessary bloat regexes" are for really? Add more mess to your code?

Your "ok-ish" code hurts my eyes. It's a literal mess, it's so dense and you didn't even indent it properly. Where did you get this from? StackOverflow?

Move the code elsewhere and format it properly. If you wanna do it properly learn how to extend a class.

I've done it my way, if you want to fix it then go ahead instead of arguing.

@XstreamSpeed
Copy link
Contributor Author

we're going the long route hm

Copy link
Contributor

@Assassin654 Assassin654 left a comment

Choose a reason for hiding this comment

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

I like it, however, is there a way to make it so it does not autofill your response? for example I want to play a youtube playlist, but instead, it just selects a song on enter instead of playing the link. to avoid this you have to arrow out of your query or hit enter before the results pop up, but would rather just be able to have the options available to choose from, not auto-enter it. would also be cool if you can have the duration of the song, but idk fully how the new autofill works.

@XstreamSpeed
Copy link
Contributor Author

If you do not select any of the autocomplete options it will automatically play the entire YouTube or Spotify playlist

@Assassin654
Copy link
Contributor

If you do not select any of the autocomplete options it will automatically play the entire YouTube or Spotify playlist

hmm, well it does not seem to do that for me. as soon as it loads in the bar above, it is automatically highlighted. When I press enter it is then selected. to play a yt playlist, I have to exit the query then press enter, or before it finishes looking up the autocomplete.

@XstreamSpeed
Copy link
Contributor Author

If you do not select any of the autocomplete options it will automatically play the entire YouTube or Spotify playlist

hmm, well it does not seem to do that for me. as soon as it loads in the bar above, it is automatically highlighted. When I press enter it is then selected. to play a yt playlist, I have to exit the query then press enter, or before it finishes looking up the autocomplete.

Well, yes, if you exit the query it plays the entire playlist, I could also add an option to the choices called "Play ${playlist.name}" to help make it better, although can't really do it now since I have exams, if you can do it then I would be very grateful.

name: track.title,
value: track.uri,
}))
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

await interaction.respond(
                    sliced.map((track) => ({
                      name: track.title,
                      value: focused.value,
                    }))
                  ).catch(() => { });
                  return;
              } else {
                await interaction.respond(
                  sliced.map((track) => ({
                    name: track.title,
                    value: track.uri,
                  }))
                ).catch(() => { });;

Should add .catch(() => { }); so the bot can ignore Unknown Interaction Error incase your bot have bad connection

@LewdHuTao
Copy link
Collaborator

LewdHuTao commented Dec 25, 2022

hmm, well it does not seem to do that for me. as soon as it loads in the bar above, it is automatically highlighted. When I press enter it is then selected. to play a yt playlist, I have to exit the query then press enter, or before it finishes looking up the autocomplete.

const yt = require("youtube-sr").default;

if (interaction.isAutocomplete()) {
      const url = interaction.options.getString("query")
      if (url === "") return;

      const match = [
        /^((?:https?:)?\/\/)?((?:www|m)\.)?((?:youtube(-nocookie)?\.com|youtu.be))(\/(?:[\w\-]+\?v=|embed\/|v\/)?)([\w\-]+)(\S+)?$/,
        /^(?:spotify:|https:\/\/[a-z]+\.spotify\.com\/(track\/|user\/(.*)\/playlist\/|playlist\/))(.*)$/,
        /^https?:\/\/(?:www\.)?deezer\.com\/[a-z]+\/(track|album|playlist)\/(\d+)$/,
        /^(?:(https?):\/\/)?(?:(?:www|m)\.)?(soundcloud\.com|snd\.sc)\/(.*)$/,
        /(?:https:\/\/music\.apple\.com\/)(?:.+)?(artist|album|music-video|playlist)\/([\w\-\.]+(\/)+[\w\-\.]+|[^&]+)\/([\w\-\.]+(\/)+[\w\-\.]+|[^&]+)/
      ].some(function (match) {
        return match.test(url) == true;
      });

      async function checkRegex() {
        if (match == true) {
          let choice = []
          choice.push({ name: url, value: url })
          await interaction.respond(choice).catch(() => { });
        }
      }

      const Random = "ytsearch"[Math.floor(Math.random() * "ytsearch".length)];

      if (interaction.commandName == "play") {
        checkRegex()
        let choice = []
        await yt.search(url || Random, { safeSearch: false, limit: 25 }).then(result => {
          result.forEach(x => { choice.push({ name: x.title, value: x.url }) })
        });
        return await interaction.respond(choice).catch(() => { });
      } else if (result.loadType === "LOAD_FAILED" || "NO_MATCHES")
        return;
    }

This new code should fix the problem you said but it will no longer show all track in the playlist you want to play. Well we can make the code cleaner if we move all the regex to a new file

Preview

image

image
Add track limit to 25

@DevAmiyo
Copy link
Contributor

This new code should fix the problem you said but it will no longer show all track in the playlist you want to play.

Add "youtube-sr": "^4.3.4" to package.json

@LewdHuTao LewdHuTao merged commit 9e17fb8 into SudhanPlayz:v5 Jan 22, 2023
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

7 participants