Merge pull request #4846 from BlueManCZ/fix-mpris-fallback-player

fix(mpris): fall back to next non-ignored player and prefer playing players

include/modules/mpris/mpris.hpp

@@ -78,6 +78,7 @@ class Mpris : public ALabel {
 
   PlayerctlPlayerManager* manager;
   PlayerctlPlayer* player;
+  PlayerctlPlayer* last_active_player_ = nullptr;
   std::string lastStatus;
   std::string lastPlayer;
 

src/modules/mpris/mpris.cpp

@@ -178,6 +178,7 @@ Mpris::Mpris(const std::string& id, const Json::Value& config)
 }
 
 Mpris::~Mpris() {
+  if (last_active_player_ && last_active_player_ != player) g_object_unref(last_active_player_);
   if (manager != nullptr) g_object_unref(manager);
   if (player != nullptr) g_object_unref(player);
 }
@@ -458,10 +459,7 @@ auto Mpris::onPlayerStop(PlayerctlPlayer* player, gpointer data) -> void {
   if (!mpris) return;
 
   spdlog::debug("mpris: player-stop callback");
-
+  // update widget (update() handles visibility)
-  // hide widget
-  mpris->event_box_.set_visible(false);
-  // update widget
   mpris->dp.emit();
 }
 
@@ -488,7 +486,12 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
 
   char* player_status = nullptr;
   auto player_playback_status = PLAYERCTL_PLAYBACK_STATUS_STOPPED;
-  g_object_get(player, "status", &player_status, "playback-status", &player_playback_status, NULL);
+
+  // Clean up previous fallback player
+  if (last_active_player_ && last_active_player_ != player) {
+    g_object_unref(last_active_player_);
+    last_active_player_ = nullptr;
+  }
 
   std::string player_name = player_;
   if (player_name == "playerctld") {
@@ -498,19 +501,48 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
     }
     // > get the list of players [..] in order of activity
     // https://github.com/altdesktop/playerctl/blob/b19a71cb9dba635df68d271bd2b3f6a99336a223/playerctl/playerctl-common.c#L248-L249
-    players = g_list_first(players);
+    PlayerctlPlayer* first_valid_player = nullptr;
-    if (players)
+    std::string first_valid_name;
-      player_name = static_cast<PlayerctlPlayerName*>(players->data)->name;
+    for (auto* p = g_list_first(players); p != nullptr; p = p->next) {
-    else
+      auto* pn = static_cast<PlayerctlPlayerName*>(p->data);
-      return std::nullopt;  // no players found, hide the widget
+      std::string name = pn->name;
-  }
+      if (std::any_of(ignored_players_.begin(), ignored_players_.end(),
-
+                      [&](const std::string& ignored) { return name == ignored; })) {
-  if (std::any_of(ignored_players_.begin(), ignored_players_.end(),
+        spdlog::warn("mpris[{}]: ignoring player update", name);
-                  [&](const std::string& pn) { return player_name == pn; })) {
+        continue;
+      }
+      auto* tmp = playerctl_player_new_from_name(pn, &error);
+      if (error || !tmp) continue;
+      if (!first_valid_player) {
+        first_valid_player = tmp;
+        first_valid_name = name;
+      }
+      PlayerctlPlaybackStatus status;
+      g_object_get(tmp, "playback-status", &status, NULL);
+      if (status == PLAYERCTL_PLAYBACK_STATUS_PLAYING) {
+        if (tmp != first_valid_player) g_object_unref(first_valid_player);
+        last_active_player_ = tmp;
+        player_name = name;
+        break;
+      }
+      if (tmp != first_valid_player) g_object_unref(tmp);
+    }
+    if (!last_active_player_) {
+      if (!first_valid_player) return std::nullopt;
+      last_active_player_ = first_valid_player;
+      player_name = first_valid_name;
+    }
+  } else if (std::any_of(ignored_players_.begin(), ignored_players_.end(),
+                         [&](const std::string& pn) { return player_name == pn; })) {
     spdlog::warn("mpris[{}]: ignoring player update", player_name);
     return std::nullopt;
+  } else {
+    last_active_player_ = player;
   }
 
+  g_object_get(last_active_player_, "status", &player_status, "playback-status",
+               &player_playback_status, NULL);
+
   // make status lowercase
   player_status[0] = std::tolower(player_status[0]);
 
@@ -524,28 +556,29 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
       .length = std::nullopt,
   };
 
-  if (auto* artist_ = playerctl_player_get_artist(player, &error)) {
+  if (auto* artist_ = playerctl_player_get_artist(last_active_player_, &error)) {
     spdlog::debug("mpris[{}]: artist = {}", info.name, artist_);
     info.artist = artist_;
     g_free(artist_);
   }
   if (error) goto errorexit;
 
-  if (auto* album_ = playerctl_player_get_album(player, &error)) {
+  if (auto* album_ = playerctl_player_get_album(last_active_player_, &error)) {
     spdlog::debug("mpris[{}]: album = {}", info.name, album_);
     info.album = album_;
     g_free(album_);
   }
   if (error) goto errorexit;
 
-  if (auto* title_ = playerctl_player_get_title(player, &error)) {
+  if (auto* title_ = playerctl_player_get_title(last_active_player_, &error)) {
     spdlog::debug("mpris[{}]: title = {}", info.name, title_);
     info.title = title_;
     g_free(title_);
   }
   if (error) goto errorexit;
 
-  if (auto* length_ = playerctl_player_print_metadata_prop(player, "mpris:length", &error)) {
+  if (auto* length_ =
+          playerctl_player_print_metadata_prop(last_active_player_, "mpris:length", &error)) {
     spdlog::debug("mpris[{}]: mpris:length = {}", info.name, length_);
     auto len = std::chrono::microseconds(std::strtol(length_, nullptr, 10));
     auto len_h = std::chrono::duration_cast<std::chrono::hours>(len);
@@ -557,7 +590,7 @@ auto Mpris::getPlayerInfo() -> std::optional<PlayerInfo> {
   if (error) goto errorexit;
 
   {
-    auto position_ = playerctl_player_get_position(player, &error);
+    auto position_ = playerctl_player_get_position(last_active_player_, &error);
     if (error) {
       // it's fine to have an error here because not all players report a position
       g_error_free(error);
@@ -609,12 +642,13 @@ bool Mpris::handleToggle(GdkEventButton* const& e) {
   });
 
   // Command pattern: encapsulate each button's action
+  auto* target = last_active_player_ ? last_active_player_ : player;
   const ButtonAction actions[] = {
-      {1, "on-click", [&]() { playerctl_player_play_pause(player, &error); }},
+      {1, "on-click", [&]() { playerctl_player_play_pause(target, &error); }},
-      {2, "on-click-middle", [&]() { playerctl_player_previous(player, &error); }},
+      {2, "on-click-middle", [&]() { playerctl_player_previous(target, &error); }},
-      {3, "on-click-right", [&]() { playerctl_player_next(player, &error); }},
+      {3, "on-click-right", [&]() { playerctl_player_next(target, &error); }},
-      {8, "on-click-backward", [&]() { playerctl_player_previous(player, &error); }},
+      {8, "on-click-backward", [&]() { playerctl_player_previous(target, &error); }},
-      {9, "on-click-forward", [&]() { playerctl_player_next(player, &error); }},
+      {9, "on-click-forward", [&]() { playerctl_player_next(target, &error); }},
   };
 
   for (const auto& action : actions) {