D8942: Make the player status a per-player object in the MPRIS plugin

Thomas Posch noreply at phabricator.kde.org
Tue Nov 28 03:42:40 UTC 2017


thomasp added a comment.


  I think you should give `MprisPlayer` a reference to `MprisPlugin` and move the setters into `MprisPlayer` too.
  Then change `MprisActivity.targetPlayer` to be a `MprisPlayer`.
  
  Otherwise this looks like an improvement. Just my 2 cents.

INLINE COMMENTS

> MprisActivity.java:156
> +                                }
> +                                if ((targetPlayer == null && selectedPlayer != null) || (targetPlayer != null && !targetPlayer.equals(selectedPlayer))) {
> +                                    targetPlayer = selectedPlayer;

`targetPlayer` is always `null` here. Please simplify beginning from line 143.

> MprisActivity.java:207
> +        //Hacks for Spotify because it reports incorrect info about what it supports
> +        boolean isSpotify = "spotify".equals(playerStatus.getPlayer().toLowerCase());
> +

`MprisPlayer` could report correct info.

REPOSITORY
  R225 KDE Connect - Android application

REVISION DETAIL
  https://phabricator.kde.org/D8942

To: mtijink, #kde_connect
Cc: thomasp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20171128/d6c7ca18/attachment.html>


More information about the KDEConnect mailing list