add LinearQueue

[Woife5]

This pull request...

• Fixes a bug
• Introduces a new feature
• Improves an existing feature
• Boosts code quality or performance

Description

This adds a new type of queue which maintinas the insertion order regardless of the user adding the song.
There is already a MR open about this issue #1165 but it seems to be stale.

Purpose

Personally we have a few designated music people on our discord who add songs in their prefered order but they always get mixed up due to FairQueue. The newly added LinearQueue would circumvent this problem.

Relevant Issue(s)

closes #581

[MichailiK]

If you'd like, join the support server if you need help and/or want to discuss of the implementation for queue types. You should be able to easily find me there (Michaili#1397).

[Woife5]

Agreed, I have adapted the PR accordingly

[MichailiK]

Having this constant here is actually nice & fine I guess, albeit out of scope of this PR.

[MichailiK]

If we want to make this truly dynamic (which I kinda do), here is what I would do:

1. Create a supplier function for all the QueueTypes:

We will add a new field to QueueType:

private final Function<AbstractQueue<?>, ? extends AbstractQueue<?>> supplier;

This supplier function will ask for an (old) queue and will return a new object that extends AbstractQueue. This works well with the constructor we have for AbstractQueue, and we can simply use method references in our fields like so:

LINEAR("\u23E9", "Linear", LinearQueue::new),
FAIR("\uD83D\uDD22", "Fair", FairQueue::new);

To obtain a new instance for the queue, I would create a simple createInstance function:

public AbstractQueue<?> createInstance(AbstractQueue<?> previousInstance)
{
    return supplier.apply(previousInstance);
}

This will then be very easy to use from the outside, something like QueueType.LINEAR.createInstance(null) would return a new (and empty) linear queue

2. Allow null in the AbstractQueue's constructor

We might not have a queue, and would like to pass null when creating a new instance, so we need to go to our AbstractQueue & change the constructor so we handle null:

AbstractQueue(AbstractQueue<T> queue)
{
    this.list = queue == null ? new LinkedList<T> : queue.getList();
}

3. Dynamic getNames

I would remove the function & instead place it in the QueueTypeCmd.

public QueueTypeCmd(Bot bot)
{
    // ...
    String queueArgs = String.join("|", Arrays.stream(QueueType.values())
            .map(x -> x.getUserFriendlyName().toLowerCase())
            .collect(Collectors.joining()));
        
    this.arguments = "[" + queueArgs + "]";
    // ...

4. Obtain queue type from user input

We can make a new function in the QueueType to look for a queue based on the name for us:

public QueueType getQueueTypeFromName(String name)
{
    for (QueueType queue : QueueType.values())
    {
        if(queue.getUserFriendlyName().equalsIgnoreCase(name))
            return queue;
    }
        
    return null;
}

We will then have to update the code in QueueTypeCmd to use our new function:

// I believe it's better if we just tell the user which queue type is being used, when no arguments are being given.
if (args.isEmpty())
{
    event.reply(currentType.getEmoji() + " Current queue type is `" + queueType.getFriendlyName() + "`");
    return;
}

QueueType newType = QueueType.getQueueTypeFromName(args);

if(newType == null)
{
   event.replyError("Invalid queue type. Valid types are `" + queueArgs + "`");
   return;
}

// TODO set new queue type
// AbstractQueue newQueue = newType.createInstance(oldQueue);

event.replySuccess("Queue type is now `" + newType.getUserFriendlyName() + "`");

[Woife5]

Wow thanks for that answer, most of these things I already implemented earlier today ^^
2. & 3. are already done
4. almost, i will change the behavior with no args

1. i will add this 👍🏼

[MichailiK]

As for 1. I'm not sure if I was clear, but the diff for QueueType should be something like this:

-   LINEAR("\u23E9", "Linear"),
+   LINEAR("\u23E9", "Linear", LinearQueue::new),
-   FAIR("\uD83D\uDD22", "Fair");
+   FAIR("\uD83D\uDD22", "Fair", FairQueue::new);

    private final String userFriendlyName;
    private final String emoji;
+   private final Function<AbstractQueue<?>, ? extends AbstractQueue<?>> supplier;

-   QueueType(final String emoji, final String userFriendlyName) {
+   QueueType(final String emoji, final String userFriendlyName, Function<AbstractQueue<?>, ? extends AbstractQueue<?>> supplier) {
        this.userFriendlyName = userFriendlyName;
        this.emoji = emoji;
+       this.supplier = supplier;
    }

+   public AbstractQueue<?> createInstance(AbstractQueue<?> previousInstance)
+   {
+       return supplier.apply(previousInstance);
+   }

[MichailiK]

This looks pretty great now! There are just these nitpicks I have about this PR. Once these are resolved, this PR will reach perfection

[MichailiK]

Oh yeah, I almost forgot: You should add an entry to the SettingsCmd. Something like this below Repeat Mode should suffice:

+ "\nQueue Type: " (s.getQueueType() == QueueType.FAIR
                     ? s.getQueueType().getUserFriendlyName() 
                     : "**"+s.getQueueType().getUserFriendlyName()+"**")

(The fair queue check is there, in case it got changed from the default value. Settings that are changed from default are bold too.)

Once the queue type is being displayed in the settings, this should be good to merge!

[Woife5]

One thing I just thought of: I added QueueTypeCmd as DJ command, is this okay or should it be an Admin command? @MichailiK

[MichailiK]

Having this as a DJ command seems the most ideal to me for now. jagrosh will take a look at the PR & optionally give his thoughts.

[MichailiK]

Hellooo! I just noticed i missed a few things that should change before this PR is perfect

[MichailiK]

Looks good! @jagrosh should take a look & merge if all's good.

[LinearQueueIsGood]

I would really like a linear queue feature with this please consider adding it thank you.

[jiimjaam]

Is there way to just add this to my own hosted version of the bot temporarily until it's officially merged (if that even happens)?

[Woife5]

Is there way to just add this to my own hosted version of the bot temporarily until it's officially merged (if that even happens)?

You can basically just build the bot yourself with this feature included. If you need any help with that you can message me on Discord: Wolfgang#3620