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

Queue history #1068

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -384,18 +384,19 @@ public void onTimelineChanged()
static final int MENU_SONG_FAVORITE = 12;
static final int MENU_SHOW_QUEUE = 13;
static final int MENU_HIDE_QUEUE = 14;
static final int MENU_DELETE = 15;
static final int MENU_EMPTY_QUEUE = 16;
static final int MENU_ADD_TO_PLAYLIST = 17;
static final int MENU_SHARE = 18;
static final int MENU_GO_HOME = 19;
static final int MENU_PLUGINS = 20; // used in FullPlaybackActivity
static final int MENU_MORE = 21; // toplevel menu, has no own action
static final int MENU_MORE_ALBUM = 22;
static final int MENU_MORE_ARTIST = 23;
static final int MENU_MORE_GENRE = 24;
static final int MENU_MORE_FOLDER = 25;
static final int MENU_JUMP_TO_TIME = 26;
static final int MENU_PREV_QUEUE = 15;
static final int MENU_DELETE = 16;
static final int MENU_EMPTY_QUEUE = 17;
static final int MENU_ADD_TO_PLAYLIST = 18;
static final int MENU_SHARE = 19;
static final int MENU_GO_HOME = 20;
static final int MENU_PLUGINS = 21; // used in FullPlaybackActivity
static final int MENU_MORE = 22; // toplevel menu, has no own action
static final int MENU_MORE_ALBUM = 23;
static final int MENU_MORE_ARTIST = 24;
static final int MENU_MORE_GENRE = 25;
static final int MENU_MORE_FOLDER = 26;
static final int MENU_JUMP_TO_TIME = 27;

@Override
public boolean onCreateOptionsMenu(Menu menu)
Expand All @@ -411,6 +412,9 @@ public boolean onOptionsItemSelected(MenuItem item)
case MENU_PREFS:
startActivity(new Intent(this, PreferencesActivity.class));
break;
case MENU_PREV_QUEUE:
PlaybackService.get(this).previousQueue();
break;
case MENU_CLEAR_QUEUE:
PlaybackService.get(this).clearQueue();
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,16 @@ public void enqueueFromSong(Song song, int type)
addSongs(query);
}

/**
* Jumps to a previously-created queue
* @author Markil 3
* @since 1.0.86
*/
public void previousQueue()
{
this.mTimeline.revertQueue();
}

/**
* Clear the song queue.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public boolean onCreateOptionsMenu(Menu menu) {

menu.add(0, MENU_SHOW_QUEUE, 20, R.string.show_queue);
menu.add(0, MENU_HIDE_QUEUE, 20, R.string.hide_queue);
menu.add(0, MENU_PREV_QUEUE, 20, R.string.prev_queue);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering: wouldn't showing a snackbar be better?

At least, the option should be greyed out if there is no queue we can restore.

menu.add(0, MENU_CLEAR_QUEUE, 20, R.string.dequeue_rest);
menu.add(0, MENU_EMPTY_QUEUE, 20, R.string.empty_the_queue);
menu.add(0, MENU_SAVE_QUEUE, 20, R.string.save_as_playlist);
Expand Down Expand Up @@ -316,7 +317,7 @@ public void onSlideExpansionChanged(int expansion) {
if (mMenu == null)
return; // not initialized yet

final int[] slide_visible = {MENU_CLEAR_QUEUE, MENU_EMPTY_QUEUE, MENU_SAVE_QUEUE};
final int[] slide_visible = {MENU_PREV_QUEUE, MENU_CLEAR_QUEUE, MENU_EMPTY_QUEUE, MENU_SAVE_QUEUE};
final int[] slide_hidden = {MENU_SORT, MENU_DELETE, MENU_ENQUEUE, MENU_MORE,
MENU_ADD_TO_PLAYLIST, MENU_SHARE};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.ListIterator;

Expand Down Expand Up @@ -220,6 +221,12 @@ public final class SongTimeline {
* should be unique, even if it refers to the same media.
*/
private ArrayList<Song> mSongs = new ArrayList<Song>(12);
/**
* Contains a history of previous queues.
*
* @author Markil 3
*/
private LinkedList<ArrayList<Song>> songHistory = new LinkedList<>();
/**
* The position of the current song (i.e. the playing song).
*/
Expand Down Expand Up @@ -725,6 +732,64 @@ else if (delta == SHIFT_PREVIOUS_SONG || delta == SHIFT_NEXT_SONG) {
return getSong(0);
}

/**
* Jumps to a previously-created queue
* @author Markil 3
* @since 1.0.86
*/
public void revertQueue()
{
ArrayList<Song> timeline = mSongs;
synchronized (this)
{
if (this.songHistory.size() > 0)
{
/*
* Saves the queue for later
*/
if (timeline.size() > 0)
{
this.songHistory.addFirst(new ArrayList<>(timeline));
}
timeline.clear();
timeline.addAll(this.songHistory.removeLast());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not fatal, but it would be better if we checked for songs which were deleted in the meantime.

See readState() in SongTimeline.java

mCurrentPos = 0;
int start =
mCurrentPos + 1; // Position where our modification started
Song jumpSong =
null; // Jump to this song if `data' requested it

/* Check if addAtPos is out-of-bounds OR if
* the request does not want to work at the current
* playlist position anyway
*/
if (start > timeline.size())
{
start = timeline.size();
}

if (mShuffleMode != SHUFFLE_NONE)
MediaUtils.shuffle(timeline.subList(start, start),
mShuffleMode == SHUFFLE_ALBUMS);

if (jumpSong != null)
{
int jumpPos = timeline.lastIndexOf(jumpSong);
if (jumpPos > start)
{
// Get the sublist twice to avoid a ConcurrentModificationException.
timeline.addAll(timeline.subList(start, jumpPos));
timeline.subList(start, jumpPos).clear();
}
}

broadcastChangedSongs();
}
}

changed();
}

/**
* Run the given query and add the results to the song timeline.
*
Expand All @@ -736,6 +801,8 @@ else if (delta == SHIFT_PREVIOUS_SONG || delta == SHIFT_NEXT_SONG) {
*/
public int addSongs(Context context, QueryTask query)
{
final int MAX_QUEUE_HISTORY = 1;

Cursor cursor = query.runQuery(context);
if (cursor == null) {
return 0;
Expand Down Expand Up @@ -793,6 +860,14 @@ public int addSongs(Context context, QueryTask query)
case MODE_PLAY:
case MODE_PLAY_POS_FIRST:
case MODE_PLAY_ID_FIRST:
/*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to generalize this and take a snapshot every time the queue is modified: this would also help in cases where users accidental clear the whole queue.

Maybe this could be hooked up into saveActiveSongs()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could. However, as queues get longer, we would get longer and longer snapshots in the memory. The main point of this feature is to undo massive edits. It is relatively easy to change one song. Rebuilding an entire queue is harder. With this, I feel that a little moderation would be needed, especially with memory concerns.

* Saves the queue for later
*/
this.songHistory.add(new ArrayList<>(timeline));
if (this.songHistory.size() > MAX_QUEUE_HISTORY)
{
this.songHistory.removeFirst();
}
timeline.clear();
mCurrentPos = 0;
break;
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/translatable.xml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ THE SOFTWARE.
<string name="empty_the_queue">Empty queue</string>
<string name="show_queue">Show queue</string>
<string name="hide_queue">Hide queue</string>
<string name="prev_queue">Play previous queue</string>
<string name="dequeue_rest">Dequeue rest</string>
<string name="toggle_controls">Toggle controls</string>
<string name="seek_10s_backward">Seek 10 seconds backward</string>
Expand Down