-
Notifications
You must be signed in to change notification settings - Fork 75
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
Added search if Play command is used without a URL #32
Conversation
@@ -20,8 +22,14 @@ class PlayCommand( | |||
) : Command("play", "p") { | |||
override suspend fun CommandContext.invoke() { | |||
if (!ensureVoiceChannel()) return | |||
val identifier = argumentText | |||
apm.loadItem(identifier, Loader(this, player, identifier)) | |||
var identifier = argumentText |
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.
This can be simplified a lot. Using exception handling as control flow is an antipattern.
val identifier = if(argumentText.startsWith("http")) argumentText else "ytsearch:$identifier"
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.
That was initially my solution, but I feel like it's a bit of a cheat and a more robust solution would cover the unlikely but still possible scenario where someone tries to play something starting with 'http'. Worth thinking about.
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.
A more robust solution would be argumentText.startsWith("http://") || argumentText.startsWith("https://")
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 can't argue with that now 😄
This branch is a welcome addition! I was searching for this functionality today, lamenting that I was going to have to implement it myself. I pulled your branch and it's running happily now on a Raspberry Pi 4 😄 Thanks a lot for Ukulele and this addition! |
Conflicts for this are resolved at |
I hope @bUbirz won't mind me bumping this. |
These changes were brought in under #53 so this PR may now be a duplicate to close 🙂 |
Addresses #14
Primitive implementation just to get the functionality added. Similar to how some other bots overload their Play command.