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

Queue history #1068

wants to merge 3 commits into from

Conversation

Markil3
Copy link

@Markil3 Markil3 commented Jan 4, 2021

For fat-fingered mobile users (i.e. me), it is rather easy to click a "play" button when I'm trying to build a queue, resulting in my losing work. To mitigate this, I took a page out of Windows Media Player's book and implemented a queue history. Every time the user makes a queue-resetting change that clears the queue (playing an album or playlist, for example), the current playlist is saved in the memory. Later, the user can select the "Play previous queue" option in the queue screen to cycle through the history of queues. At this point, we have just one active queue and one queue in the history (making more queues deletes the oldest one in the history), but that can easily be changed by changing the MAX_QUEUE_HISTORY variable in the SongTimeline.addSongs method.

Note that this functionality is not triggered when dequeuing songs (even when most or all of the queue is emptied). I can implement that if needed, but I didn't see the need for it

Furthermore, the single string entry has not been localized. Considering the unreliability of Google Translate and such, I figured it would be best to leave that to experts.

…s queues that they have created within the session.

Considering how often I mess up my carefully crafted queue with a wrong button, an "undo" button helps.
@@ -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.

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

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

@adrian-bl
Copy link
Member

I played with this idea in a different branch using the existing write/read-state code:

https://github.com/vanilla-music/vanilla/tree/queue-revert

The nice thing about this approach is that we can use a lot of the existing code & logic - and the increase in RAM should be minimal: We use about 12 bytes per song, so if your queue contains 100'000 items, we will use about 1.1MB of RAM to store the 'undo' version of the queue, which shouldn't be a big problem on modern android devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants