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

Add queue track number to /forceskip #154

Closed
rafaeldamasceno opened this issue Feb 17, 2022 · 10 comments · Fixed by #158
Closed

Add queue track number to /forceskip #154

rafaeldamasceno opened this issue Feb 17, 2022 · 10 comments · Fixed by #158
Assignees
Labels
💬 discussion Further information is requested ✨ feature New feature or request 🐤 good first issue Good for newcomers

Comments

@rafaeldamasceno
Copy link
Contributor

rafaeldamasceno commented Feb 17, 2022

Rationale

Sometimes you want to skip to a certain track in the queue.

Description

Add an optional positive integer to the /forceskip command that the bot will skip to if the track exists.

@afonsojramos
Copy link
Collaborator

The issue train isn't set to leave any time soon 😅

Not 100% sure if we should add this as an optional or, as I said in #151, create a new command.

@rafaeldamasceno rafaeldamasceno changed the title Add queue track number to /skip Add queue track number to /forceskip Feb 17, 2022
@rafaeldamasceno
Copy link
Contributor Author

You've just made me realize this would actually be for /forceskip 🤣 A new command is even more clutter IMO, but could I live with it. The optional doesn't bother anyone that doesn't want to use it, and the command would function normally otherwise.

@aquelemiguel
Copy link
Owner

I like it! Better have it as its own command like /skipto, I'm sure a few other music bots used this naming.
Please don't be afraid of adding more commands! This is usually better than cramming arguments to existing ones.

@aquelemiguel aquelemiguel added ✨ feature New feature or request 🐤 good first issue Good for newcomers 💬 discussion Further information is requested labels Feb 17, 2022
@rafaeldamasceno
Copy link
Contributor Author

Should it then be called /forceskipto? I’m willing to admit half of the reason I wanted it to be an optional argument is because the name sucks, or is inconsistent if we name it what you suggested :P

@aquelemiguel
Copy link
Owner

@rafaeldamasceno Both an optional arg to /forceskip and a new /skipto command sound good to me. I find the latter a bit more intuitive, but I'll leave the decision to you guys. Anything except the /forceskipto eyesore. 😄

@afonsojramos
Copy link
Collaborator

Okay okay, breathing some fresh air to this:

Get rid of forceanything and everything is either /skip or /skipto, and we check manually for the role. This way we remove the clutter of double the commands needed and easily add this feature of skipping to a certain track.

@aquelemiguel
Copy link
Owner

Okay okay, breathing some fresh air to this:

Get rid of forceanything and everything is either /skip or /skipto, and we check manually for the role. This way we remove the clutter of double the commands needed and easily add this feature of skipping to a certain track.

I really really like this.

@afonsojramos afonsojramos self-assigned this Feb 18, 2022
@rafaeldamasceno
Copy link
Contributor Author

While I appreciate the sentiment behind this solution, even if I’m a DJ I could wish to respect the skip vote and cast a vote myself. I have one last suggestion: rename the current /skip into /voteskip (because that’s what it really does) and then /forceskip into /skip. This way, /skipto makes a lot more sense.

@aquelemiguel
Copy link
Owner

While I appreciate the sentiment behind this solution, even if I’m a DJ I could wish to respect the skip vote and cast a vote myself. I have one last suggestion: rename the current /skip into /voteskip (because that’s what it really does) and then /forceskip into /skip. This way, /skipto makes a lot more sense.

I think this is the way. 🙂

@afonsojramos
Copy link
Collaborator

afonsojramos commented Feb 22, 2022

While I appreciate the sentiment behind this solution, even if I’m a DJ I could wish to respect the skip vote and cast a vote myself. I have one last suggestion: rename the current /skip into /voteskip (because that’s what it really does) and then /forceskip into /skip. This way, /skipto makes a lot more sense.

In #158 I implemented something that was talked outside GitHub, which is a merger between my initial proposal and this.

Essentially, the commands are now /skip and /voteskip, where /skip is a DJ command, and there is no /skipto since it is now an optional argument in /skip, which represents the track they want to skip to.

/voteskip does not have the possibility to skip to a certain track, since it would involve too much overhead and increase complexity too much. Additionally, I do not think it makes THAT much sense. Maybe if in the future we implement react-based voting we can reconsider, since one of the problems would be to differentiate a /voteskip skip-to:5 and a /voteskip skip-to:3 and how they would count, would we need a per-song voting counter, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💬 discussion Further information is requested ✨ feature New feature or request 🐤 good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants