-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Conversation
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 |
There was a problem hiding this 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
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. |
Hmm, I must've forgotten it, will add it, thank you |
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. |
events/interactionCreate.js
Outdated
|
||
if ( | ||
focused.value.match( | ||
/(?:https:\/\/open\.spotify\.com\/|spotify:)(?:.+)?(track|playlist|artist|episode|show|album)[\/:]([A-Za-z0-9]+)/ || |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
I've done it my way, if you want to fix it then go ahead instead of arguing. |
we're going the long route hm |
There was a problem hiding this 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.
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. |
events/interactionCreate.js
Outdated
name: track.title, | ||
value: track.uri, | ||
})) | ||
); |
There was a problem hiding this comment.
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
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 |
Add |
feat: Autocompletion to play command
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
Preview
For songs:
For playlists:
For playlists if you don't choose one of the options: