add ShuffleOrder.cloneAndSet() to provide startIndex to shuffle order

[nift4]

Shuffle order may want to ensure that the song that's going to be played (was chosen by user) is also the first song in shuffle playlist, as that makes sure all other songs will be playing afterwards. If first song gets random position in shuffled playlist, playback may stop before playing every song as it will start in the middle of shuffled list.

[tianyif]

1. It's better to keep the statement general and avoid mentioning "playback" here. How do you think of "The index of the new element in the unshuffled order that should be the first in the shuffled order"

2. Instead of saying INDEX_UNSET should be passed when the new list is empty, shall we say startIndex will be ignored if the new list is empty or it is larger than the size (inclusive) of the new list? And we can still allow a INDEX_UNSET which explicitly suggests that the first element of the shuffled order is not specified.

What do you think?

[nift4]

Hi, thanks for the feedback. These suggestions make sense to me and I've applied them.

[tianyif]

Thanks for the change! Still for the INDEX_UNSET case, instead of saying "or {@link C#INDEX_UNSET} if the index is unknown", would it be better to say "or {@link C#INDEX_UNSET} if the the first element in the shuffled order is not specified"? Personally it sounds more general and gives out a sign that not enforcing the first element is allowed.

Also for L292 - "if access to the {@code startIndex} is desired", what do you think of making it more straightforward, eg. "if the first element in the shuffled order should be set to the one whose index in the unshuffled order is {@code startIndex}"?

Once this is in the internal review stage, we can get a third opinion on this as well.

[nift4]

Thanks for the change! Still for the INDEX_UNSET case, instead of saying "or {@link C#INDEX_UNSET} if the index is unknown", would it be better to say "or {@link C#INDEX_UNSET} if the the first element in the shuffled order is not specified"? Personally it sounds more general and gives out a sign that not enforcing the first element is allowed.

Makes sense to me.

Also for L292 - "if access to the {@code startIndex} is desired", what do you think of making it more straightforward, eg. "if the first element in the shuffled order should be set to the one whose index in the unshuffled order is {@code startIndex}"?

I am a bit concerned that this changes the messaging too much in the direction of "startIndex being first in the shuffled list is the only correct way". While I don't actually see a reason to do otherwise, it has been the default for a long time now, and DefaultShuffleOrder will still do it after this change - or maybe I should change DefaultShuffleOrder to respect startIndex and change the text to state that startIndex being first in the shuffled list is the recommended way like you said?

[nift4]

Sorry, I misread that, you said "if [it] should be set [...]", and not "it should be set". And changing DefaultShuffleOrder is probably not a good idea on second thought due to backwards compatibility. Pushed the new wording, let me know whether it looks good.

[tianyif]

Cool, this looks good to me! I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks!