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

Play command flags #51

Closed
5 tasks done
rafaeldamasceno opened this issue Oct 26, 2021 · 17 comments · Fixed by #140 or #144
Closed
5 tasks done

Play command flags #51

rafaeldamasceno opened this issue Oct 26, 2021 · 17 comments · Fixed by #140 or #144
Assignees
Labels
✨ feature New feature or request

Comments

@rafaeldamasceno
Copy link
Contributor

rafaeldamasceno commented Oct 26, 2021

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:

  • -all: This instructs Groovy to queue all returned songs (...) when you queue a YouTube video in a playlist.
  • -shuffle: (...) randomize the order of songs in the playlist before it's added to the queue (...)
  • -next: (...) the song you queued is placed directly after the playing track in the queue. (...)
  • -jump: Instantly jumps to the track you just queued. (...)
  • -reverse: Queue the playlist in reverse order.
@aquelemiguel
Copy link
Owner

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
Copy link
Contributor Author

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.

@aquelemiguel aquelemiguel added the ✨ feature New feature or request label Jan 4, 2022
@rafaeldamasceno
Copy link
Contributor Author

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
Copy link
Collaborator

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

@aquelemiguel
Copy link
Owner

@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
Copy link
Contributor Author

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
Copy link
Collaborator

afonsojramos commented Jan 24, 2022

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

@rafaeldamasceno
Copy link
Contributor Author

@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
Copy link
Owner

@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
Copy link
Owner

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

@rafaeldamasceno
Copy link
Contributor Author

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

Yes, please.

@aquelemiguel
Copy link
Owner

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
Copy link
Owner

aquelemiguel commented Feb 13, 2022

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

@StaticRocket
Copy link
Contributor

StaticRocket commented Feb 13, 2022

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
Copy link
Contributor Author

@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:

  • The first one is to have it as booleans like /play query:query all:true shuffle:true where the all flags are optionals and the ones you want to activate need to be set to true, since it's the closest thing to flags (and unfortunately it's not looking like it will change anytime soon. I don't think it is cumbersome to set, but I agree it looks atrocious. The advantage is that there's helper text and it's obvious to the user what is going to happen when setting values to the flags.
  • The alternative would be /play query:query flags:as, where the flags parameter will have letters for each flag you want to activate. Since flags would be an optional, there isn't any need to add dashes or anything. For me, this is the way that would make it fastest for power users to interact with the bot and now that I think about it, is about the same way as Rythm worked and advertised these features (as advanced usage flags).

@joao-conde
Copy link
Collaborator

joao-conde commented Feb 17, 2022

All different play modes implemented as subcommands, closing.

@aquelemiguel
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature New feature or request
Projects
None yet
5 participants