aquelemiguel / parrot

🦜 A hassle-free, highly performant, self-hosted Discord music bot with YouTube and Spotify support. Powered by yt-dlp.
MIT License
135 stars 44 forks source link

Play command flags #51

Closed rafaeldamasceno closed 2 years ago

rafaeldamasceno commented 3 years ago

Rationale

When playing a playlist, you sometimes want to shuffle the playlist before playing it. Sometimes you might even want to play it right after the current track, or even interrupt the current track and skip right away. These can be all achieved via flags and it was a much appreciated feature from Groovy.

Description

When using the play command, adding a flag modifies the behavior of the play command.

These were the available flags in Groovy (with the exception of -choose, which seems more appropriate as a search command). They were also available as shorthand flags with the first letter only:

aquelemiguel commented 3 years ago

Good request, I really like this flag idea! 👏 I was unaware Groovy did this, we only really used Rhythm on our server.

A couple questions: in the past, we have merged #24, which is its own command for enqueueing a song to the top of the queue (without skipping). I'd assume you suggest transferring that code to be usable only in the --next flag?

Also, what does the --all flag do? Currently, Parrot automatically detects whether the query is a playlist link and queues all its songs (similarly to Rhythm's behavior). Or does --all fetch the entire playlist via "video inserted in playlist"-type links?

rafaeldamasceno commented 3 years ago

If #43 is properly implemented, I wouldn't mind keeping both the playtop command and -next flag. They would do the same, but there wouldn't be any issue at all since they would use the same code, and you could appease to both previous Rythm and Groovy users.

Regarding the -all flag, you're exactly right with your latter example. Currently Parrot only queues the actual linked video, which I think should be the correct behavior. With the flag passed, it would instead queue the entire playlist. In any case, this flag, together with -choose, was rarely used by anyone, so I wouldn't mind not having it implemented either.

rafaeldamasceno commented 2 years ago

I'd like to return to this feature, since it's the one I miss the most and I want something to start out with the code as well. Do you think it makes sense to keep the flags in the query, or should I make these optional booleans in the slash command?

afonsojramos commented 2 years ago

Definitely adding these as optional booleans is the way to go moving forward for any additional arguments imo!

aquelemiguel commented 2 years ago

@rafaeldamasceno I reckon this is a good chance to use select menus as they support both single and multiple selections. Was it possible to use multiple flags on Groovy?

rafaeldamasceno commented 2 years ago

Was it possible to use multiple flags on Groovy?

Yes, it was.

@rafaeldamasceno I reckon this is a good chance to use select menus as they support both single and multiple selections.

Yes and no... Do we want to break the current flow of just posting the query and having the bot reply? We'd need to have this intermediate step to select the flags (which I don't even know if is possible to do in the same slash command as the one that receives the query), or create a new command for using the flags. None of these options seem good to me.

afonsojramos commented 2 years ago

@rafaeldamasceno have you tested it out? I'm pretty sure it is pretty seamless using discord's built-in optionals.

rafaeldamasceno commented 2 years ago

@afonsojramos No, but I'm pretty sure that select menus are not part of the command options, but instead part of the message components. This would mean having to use play (or any other command for this purpose), having the client render the select menu, and only then we would have the correct flags. This contrasts versus using play and having all the flags as optional boolean application command options, and when you hit enter, you are done.

aquelemiguel commented 2 years ago

@rafaeldamasceno You're absolutely right, I was confusing that with application command option choices. Unsure of the limitations (this is still quite new) but maybe something like this? 🤷‍♂️

image

aquelemiguel commented 2 years ago

@rafaeldamasceno Do you want me to assign you to this topic?

rafaeldamasceno commented 2 years ago

@rafaeldamasceno Do you want me to assign you to this topic?

Yes, please.

aquelemiguel commented 2 years ago

Just an FIY, after #135 we may use custom yt-dlp arguments to help us implement some flags. Could be handy, but haven't checked in detail.

aquelemiguel commented 2 years ago

@rafaeldamasceno As discussed, moving the issue to @StaticRocket. @StaticRocket Please confirm with a comment here so I can lock it to yourself.

StaticRocket commented 2 years ago

If @rafaeldamasceno is fine with it then sure!

Sorry for jumping on this issue before asking, I just wanted to see a proof of concept for the playlist unravel flag I mentioned earlier yesterday.

rafaeldamasceno commented 2 years ago

@aquelemiguel Regarding the examples you gave for the options I presented in #140, they are both correct.

The only feasible ways I see to implement this are two:

joao-conde commented 2 years ago

All different play modes implemented as subcommands, closing.

aquelemiguel commented 2 years ago

Keeping it closed for now. If needed, let's open a separate issue regarding multiple flags for tracking. But realistically, we must wait until Discord supports some sort of variadic arguments to justify any changes.