Fix use of incorrect playlist index when playing from an associated track table

[Utagai]

This closes #628.

We had a poor assumption here, that the selected playlist index was always guaranteed to be the playlist for which the loaded track table's context referred to. However, this is not always true. Take the following steps:

1. Enter the playlists table.
2. Active (press Enter) a playlist and subsequently load its track table.
3. Leave the track table view and go back to the playlists table.
4. Move the selection to another playlist, but do not activate (press Enter) it.
5. Re-enter the track table, and try to play a song.

Step 5 will attempt to play a song on the selected playlist, which is the one you never activated, leading to an unexpected song being played back. Instead, we need a particular state for the activated playlist index, which this PR adds and uses instead. This avoids the problem described in the issue.

[Utagai]

Ooo, I think I may get the Hacktoberfest shirt this year, since I think this is my 4th PR. 😄

@Rigellute It's worth noting that this PR highlights a fundamental weakness in the codebase that I think we've seen issues with already: index state (to some degree, we have lots of small little pieces of state under App, and it's all quite hard to ensure the correctness of). It's hard to get this stuff right. I would be lying if I said I had an idea for an improvement. My hunch right now though is that we likely should be storing some kind of generic context/state-monad that can answer more higher level questions like "What is the current SimplifiedPlaylist?" with Option<SimplifiedPlaylist>, rather than us having to deal with indexes and such. At the very least this may separate that complexity elsewhere, which can then make it easier to reason about how we might go about removing index usage except for at the tui interface level.

[Utagai]

From my look through the codebase, there is no other way in which we enter into the TableContext::MyPlaylists context, so this is all that is necessary to track the "active" playlist.

[sputnick1124]

I agree that there is way too much state packed into the App object. I think encapsulating some of that state bit by bit into dedicated objects is a start to solve this problem. Specifically with pageable lists, this PR is a start to address that problem.

For instance, see this commit where delegation for managing the Library saved artists list is passed off to the ScrollableResultPages object. This simplifies a lot of the code IMO since App shouldn't have to store or care about the selected index or the list contents directly anymore.

There's still a bit of work to be done to get all the cases of this, but most of the results we care about should actually be pageable, so I think once this massive overhaul (or some version of it) gets through, we will have a lot fewer instances of this issue cropping up.

[Utagai]

Just saw your draft PR @sputnick1124! Thanks for that work, it will definitely improve the code a lot!

I agree the pageable results overhaul is a great step. We need to keep going in this direction of encapsulating the state into dedicated objects, whether that be better paging abstractions or context introductions, etc.

In fact, I'm a little jealous you got to work on that refactor, it looks like it was a satisfying experience 🙂.

[Rigellute]

Thanks @Utagai. Indeed, managing index state and pagination is a major problem. Will be looking at #635 next