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