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

Attempt at fixing various playlist-updating issues #2449

Merged
merged 3 commits into from Jul 17, 2017

Conversation

Projects
None yet
2 participants
@Meriipu
Contributor

Meriipu commented Jun 25, 2017

related to issues #2446 #2253

This appears to fix (in the playlist browser):

  • A track change causing the current selection to be cleared
  • A track change causing the playlist to scroll to the new track regardless of whether Jump on track change in config is set or not
  • At least for me, when a playlist had duplicate tracks and one started playing, the last one would start playing, potentially skipping far into the playlist.

now I have no idea if anything breaks so I suppose this pull request would serve better as some discussion of some sort (or if any of these changes are horribly messy).

I would like to add that I have no idea what I am doing and I worry something fairly unobvious breaks as a result of this.

as a sidenote I have not been able to run tests locally, the tests stop after a short while at

tests/test_browsers_paned.py .Trace/breakpoint trap
#(with this in my /var/log/messages, which I could not make sense of:)
#Jun 25 10:22:40 Meowth kernel: traps: python[17475] trap int3 ip:7fb763cb5101 sp:7fffc9ecb140 error:0
@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 26, 2017

One thing that this breaks is that if a track in one playlist is activated and the user swaps to another playlist (that does not necessarily also contain the track), the next track will be played from that new selected playlist once the previous track ends.

Edit: Wait this happens in head too, nevermind my ramblings.
I suppose it is the issue that was raised in #1424 #1425 #1426
It is not entirely the issue I am having - The way the current playlist forcibly jumps to the next track every time the track changes, regardless of what the configuration setting "jump" has been set to.

The behaviour (jumping to a track in the selected playlist, rather than the next one in the original) is a bit confusing to me but I suppose I can live with that since I rarely browse a second playlist while intending to play the tracks in another. It does make it hard to just browse playlists without playing them though.

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 26, 2017

It does not work at all in head, but some interactions have the same result in this patch too:

  • Make a playlist with duplicate tracks
  • Start one track, it will start playing the one you clicked (as opposed to the last instance as in head)
  • Click a different playlist then click the original playlist again, all while the track is still playing
  • The last track is now selected; the original playlist "forgot" which instance of the track was playing.
@lazka

This comment has been minimized.

Member

lazka commented Jul 8, 2017

I'm OK with DnDing something onto the playlist not updating the song list (that's how it happens elsewhere anyway).

Just removing the code without replacement (no commenting) is fine.

For the other issues, best to tackle them one by one in a separate issue/PR.

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jul 8, 2017

So should I keep or remove the added lines for updating the songlist (is this the list of tracks that are seen in the playlist browser when a playlist is selected?)

The original behaviour in the playlist browser is that it updates the songlist with the new DnD track.
The behaviour in this commit is that it does not do the scrolling/clearing selection-stuff, but it still updates the current songlist with the DnD track.

I have not extensively tested combinations of browsers like from the menubar Browse>Open Browser>Playlists, in combination with a second browser in the main window, though some of those interactions already seem to have issues in the current version too (#2454)

@lazka lazka merged commit 0302907 into quodlibet:master Jul 17, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lazka

This comment has been minimized.

Member

lazka commented Jul 17, 2017

So should I keep or remove the added lines for updating the songlist (is this the list of tracks that are seen in the playlist browser when a playlist is selected?)

it seems to work fine here.

thanks!

@Meriipu Meriipu deleted the Meriipu:playlistissues_dev branch Jul 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment