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

[WIP] Listening Rooms #128

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open

[WIP] Listening Rooms #128

wants to merge 55 commits into from

Conversation

teo
Copy link
Member

@teo teo commented Nov 12, 2012

They are rooms. You can listen to music in them. What's there not to like?

Seriously though, this is an extension of the "listen along" concept. Still WiP, but the idea is for listeners to be able to see what's upcoming and track history, and submit suggestions. Listening Rooms also appear superficially like playlists, but they are not persistent, and the track operations are very different.
This Listening Rooms concept is not like Soundrop, it is not massive democratic music listening, but rather the idea is to allow a person to host a party, and guests to occasionally chip in.

@Ramblurr
Copy link
Contributor

My holy god. This feature is finally coming! Huzzah! (pardon cheers from peanut gallery)

@teo
Copy link
Member Author

teo commented Nov 20, 2012

Since 367649e which I just pushed, all core Listening Rooms operations should work. One can start a room and become DJ, and others can join in and become listeners. Aside from a few crashes here and there, it is usable, though not quite ready for merging.
Suggestions are not implemented yet.
For the next few days, possibly more than a week, I will not be committing major changes to this branch, focusing instead on writing the thesis document based on this work. After that, but still before mid-December, I plan on finishing with the suggestions feature.
Since this heap of code won't be changing much, feel free to start reviewing.

@lfranchi
Copy link
Member

Awesome! Do you want feedback from testing as well? Or would you rather wait for your December changes?

@@ -61,6 +61,8 @@ SET( tomahawkSourcesGui ${tomahawkSourcesGui}
sourcetree/items/PlaylistItems.cpp
sourcetree/items/CategoryItems.cpp
sourcetree/items/GenericPageItems.cpp
sourcetree/items/ListeningRoomItem.cpp
sourcetree/items/ListeningRoomsCategoryItem.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Super nitpick: one is plural one is not :)

@lfranchi
Copy link
Member

One thing just came to mind---since a listening room's default playlist interface is just the source playlist interface, we probably want to change the default listen along behaviour for listening rooms to be "locked on"---that is, if a DJ skips a song, listeners should immediately skip too, unlike listening along. Being synchronized in the same room is more important.



void
ListeningRoomModel::endRoomChanges()
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we can somehow share the add/remove/move logic between Playlist and ListeningRoom. I added the m_savedInsertPos + move/etc logic to handle spotify syncing (we need incremental changes not just full revisions), but it seems fragile now to have to maintain this tricky code in two places.

This means that after this commit, all core Listening Room operations
should work, for both DJs and listeners.
Optimized LR model updating so we don't reload unless stuff has really
changed.
Also added tons of debug spam and a little debug UI.
@andriijas
Copy link

Would love to get this into master soon :) thanks!

@andriijas
Copy link

4 months since my comment. Why isnt this getting merged? :)

@teo
Copy link
Member Author

teo commented Aug 12, 2013

On Mon, Aug 12, 2013 at 11:15 AM, Andreas Cederström
[email protected] wrote:

4 months since my comment. Why isnt this getting merged? :)


Reply to this email directly or view it on GitHub.

Because it's not ready for merging yet, it needs some finishing and
fixing and there are only 24 hours in a day :(

It is on my to-do list, it will take a while more, and it will be
merged as soon as possible.

@andriijas
Copy link

meant to stress you man. just wanted to know the current status. appreciate your efforts :)

teo added 12 commits October 23, 2013 14:11
Builds ok, but functionality is broken.

Conflicts:
	resources.qrc
	src/CMakeLists.txt
	src/libtomahawk/ActionCollection.cpp
	src/libtomahawk/CMakeLists.txt
	src/libtomahawk/LatchManager.cpp
	src/libtomahawk/Source.cpp
	src/libtomahawk/Source.h
	src/libtomahawk/Typedefs.h
	src/libtomahawk/ViewManager.cpp
	src/libtomahawk/ViewManager.h
	src/libtomahawk/database/DatabaseCommand.cpp
	src/libtomahawk/playlist/PlayableItem.cpp
	src/tomahawk/TomahawkApp.cpp
	src/tomahawk/TomahawkWindow.cpp
	src/tomahawk/sourcetree/SourceDelegate.cpp
	src/tomahawk/sourcetree/SourceTreeView.cpp
	src/tomahawk/sourcetree/SourcesModel.cpp
	src/tomahawk/sourcetree/SourcesModel.h
	src/tomahawk/sourcetree/items/CategoryItems.cpp
	src/tomahawk/sourcetree/items/CategoryItems.h
Also disabled PartyModel and related widgets.
@andriijas
Copy link

All i want for christmas is listening rooms :)

@xhochy xhochy changed the title Listening Rooms [WIP] Listening Rooms May 6, 2014
@lfranchi
Copy link
Member

@teo out of curiosity, what's the current state here? :)

@teo
Copy link
Member Author

teo commented Sep 13, 2014

Unfinished, it still requires a bit of work.

@lfranchi
Copy link
Member

@teo I know this has been a while, but do you still remember what work is remaining? I figure if you write it down, there's a higher chance of it being picked up on. Maybe I'll even take a look...

@Sytten
Copy link

Sytten commented Dec 25, 2015

Any news? its a feature that lacks in tomahawk compared to other music player...

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.

5 participants