GNOME Bugzilla – Bug 568688
RBPlayOrder missing from python bindings
Last modified: 2018-05-24 13:58:31 UTC
Please describe the problem: The play-order classes are missing from the python interface. There's quite a few of variations them, but it's easy to fix. Also, the RBShellPlayer should be fixed: get_property("play-order") returns a string (type of play order... linear, random, shuffle etc), so I propose get_property("play-order-instance") Steps to reproduce: 1. shell.get_player.get_property("source").get_property("play-order") == None 2. 3. Actual results: Expected results: Does this happen every time? Other information:
Created attachment 126994 [details] [review] Proposed fix I've created property "play-order-instance" for the shell player. We could move the old property "play-order" to "play-order-type", but I have no idea what that might break.
You shouldn't need to add the various subclasses of RBPlayOrder to the bindings, as they don't add any methods. The play-order-instance property should probably take into account sources that provide their own play order, so it should check the play-order property on the current source before returning player->priv->play_order. If not, then whatever's using the plugin is going to have to do the same thing.
IMO the plugin should be responsible for fetching the play orders. Imagine that I want to display the next 10 songs to be played: The first 5 might come from the play queue source, and the others from the current source. But I could want the choice between showing only the songs in the queue and only the songs in the source, or maybe I wanted to merge them, which requires both play orders. Maybe, I'm misunderstanding how play orders work, but they're tied to a specific source, right? And in that case it could be very useful to have an instance of both play orders. I'll happily make another patch, but please do tell me how it works... I'm very slow at understanding these 2000 line files :)
Yes, you are misunderstanding how play orders work. I'm not talking about the queue play order here. There is a global play order that is selected using the repeat and shuffle buttons, but sources can override this if they require a specific ordering. The last.fm source is the only one that actually does this at present. As it is, your patch only returns the global play order, but anywhere you might use that, you would need to check for a source-specific play order too by doing shell_player.get_active_source().props.play_order. The play queue has a separate play order again (player->priv->queue_play_order inside rb-shell-player.c). This probably should also be made available as a property on the RBShellPlayer object (queue-play-order-instance, I guess).
Created attachment 127343 [details] [review] Similar as previous but removed unnecessary python defs as suggested Okay, it appears that the play queue has its own play-order, but if I invoke get_next() it will remove the song from the queue. Therefore I switched my plugin from using play-orders, so it purely uses query-models from sources. I still think you should merge this patch, since it doesn't seem to cause much harm. And I really think the shell-player should return its own play-order instance and not the one from its sources.. gives less control.
(In reply to comment #5) > > Okay, it appears that the play queue has its own play-order, but if I invoke > get_next() it will remove the song from the queue. Are you sure? The queue play order only removes entries when it actually starts playing a different entry. > Therefore I switched my > plugin from using play-orders, so it purely uses query-models from sources. OK, it will just be wrong then. > I still think you should merge this patch, since it doesn't seem to cause much > harm. And I really think the shell-player should return its own play-order > instance and not the one from its sources.. gives less control. Control over what? I really don't see why anyone needing access to the play order would want to have the option of using the wrong one.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/rhythmbox/issues/690.