Simplify can_group_with logic

diff --git a/music_assistant/models/player.py b/music_assistant/models/player.py
index 9575dd2be9deadb7636680c00a33eef502350351..23379492585379af479754edf9d034283a57181d 100644 (file)
--- a/music_assistant/models/player.py
+++ b/music_assistant/models/player.py
@@ -1640,10 +1640,14 @@ class Player(ABC):
         # This handles cases where a native player (e.g., native AirPlay) has grouped
         # protocol players (e.g., Sonos AirPlay protocol players) that need translation
         members: list[str] = []
-        translated_members = self._translate_protocol_ids_to_visible(set(self.group_members))
-        for member in translated_members:
-            if member.player_id not in members:
-                members.append(member.player_id)
+        if self.type == PlayerType.PROTOCOL:
+            # protocol players use their own group members without translation
+            members.extend(self.group_members)
+        else:
+            translated_members = self._translate_protocol_ids_to_visible(set(self.group_members))
+            for member in translated_members:
+                if member.player_id not in members:
+                    members.append(member.player_id)
 
         # If there's an active linked protocol, include its group members (translated)
         if self.__attr_active_output_protocol and self.__attr_active_output_protocol != "native":
@@ -1730,7 +1734,6 @@ class Player(ABC):
 
         All protocol player IDs are translated to their visible parent player IDs.
         """
-        result: set[str] = set()
 
         def _should_include_player(player: Player) -> bool:
             """Check if a player should be included in the can-group-with set."""
@@ -1739,40 +1742,38 @@ class Player(ABC):
             if player.player_id == self.player_id:
                 return False  # Don't include self
             # Don't include (playing) players that have group members (they are group leaders)
-            if (
+            if (  # noqa: SIM103
                 player.state.playback_state in (PlaybackState.PLAYING, PlaybackState.PAUSED)
                 and player.group_members
-                and player.type != PlayerType.PROTOCOL
             ):
-                return False  # Regular native group leader - exclude
-            # Don't include players that are currently grouped/synced to OTHER players
-            # But DO include players grouped to THIS player (so they can be ungrouped)
-            grouped_to = player.state.synced_to or player.state.active_group
-            return grouped_to is None or grouped_to == self.player_id
+                return False
+            return True
 
         if self.__final_synced_to:
             # player is already synced/grouped, cannot group with others
-            return result
+            return set()
 
-        # always start with the native can_group_with options (expanded for provider instance IDs)
-        for player in self._expand_can_group_with():
+        expanded_can_group_with = self._expand_can_group_with()
+        # Scenario 1: Player is a protocol player - just return the (expanded) result
+        if self.type == PlayerType.PROTOCOL:
+            return {x.player_id for x in expanded_can_group_with}
+
+        result: set[str] = set()
+        # always start with the native can_group_with options (expanded from provider instance IDs)
+        # NOTE we need to translate protocol player IDs to visible player IDs here as well,
+        # to cover cases where a native player (e.g., native AirPlay) has grouped protocol players
+        # (e.g., Sonos AirPlay protocol players)
+        for player in expanded_can_group_with:
             if player.type == PlayerType.PROTOCOL:
-                # Protocol player is hidden - translate to its visible parent player
                 if not player.protocol_parent_id:
                     continue
-                visible_parent = self.mass.players.get_player(player.protocol_parent_id)
-                if not visible_parent or not _should_include_player(visible_parent):
-                    continue
-                result.add(visible_parent.player_id)
-            else:
-                if not _should_include_player(player):
+                parent_player = self.mass.players.get_player(player.protocol_parent_id)
+                if not parent_player or not _should_include_player(parent_player):
                     continue
+                result.add(parent_player.player_id)
+            elif _should_include_player(player):
                 result.add(player.player_id)
 
-        # Scenario 1: Player is a protocol player - just return the (expanded) result
-        if self.type == PlayerType.PROTOCOL:
-            return result
-
         # Scenario 2: External source is active - don't include protocol-based grouping
         # When an external source (e.g., Spotify Connect, TV) is active, grouping via
         # protocols (AirPlay, Sendspin, etc.) wouldn't work - only native grouping is available.

diff --git a/tests/core/test_player_grouping.py b/tests/core/test_player_grouping.py
index 59e0d5c8c17c8451d69420669dd0d37ead3c709d..eb7920ebfa80f618cd4d962dadcbff5a9dc68d8f 100644 (file)
--- a/tests/core/test_player_grouping.py
+++ b/tests/core/test_player_grouping.py
@@ -124,45 +124,6 @@ class TestCanGroupWithBasics:
 class TestSyncedPlayers:
     """Test behavior with synced/grouped players."""
 
-    def test_synced_player_excluded_from_others(self, mock_mass: MagicMock) -> None:
-        """
-        Test that a player synced to another is excluded from other players' can_group_with.
-
-        Regression test for: Player synced to another showing up in third player's can_group_with.
-        """
-        controller = PlayerController(mock_mass)
-        provider = MockProvider("test_provider", instance_id="test", mass=mock_mass)
-
-        # Sync leader
-        leader = MockPlayer(provider, "leader", "Leader")
-        leader._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
-        leader._attr_can_group_with = {"synced", "other"}
-        leader._attr_group_members = ["leader", "synced"]
-        leader._attr_playback_state = PlaybackState.PLAYING  # Make it playing so it gets excluded
-
-        # Synced player
-        synced = MockPlayer(provider, "synced", "Synced")
-
-        # Third player
-        other = MockPlayer(provider, "other", "Other")
-        other._attr_supported_features.add(PlayerFeature.SET_MEMBERS)
-        other._attr_can_group_with = {"leader", "synced"}
-
-        controller._players = {"leader": leader, "synced": synced, "other": other}
-        mock_mass.players = controller
-
-        # Trigger synced_to calculation
-        leader.update_state(signal_event=False)
-        synced.update_state(signal_event=False)
-        other.update_state(signal_event=False)
-
-        # The synced player should NOT appear in other's can_group_with
-        assert "synced" not in other.state.can_group_with
-        # The leader should also NOT appear (has group members)
-        assert "leader" not in other.state.can_group_with
-        # Other should only see itself as ungrouped
-        assert other.state.can_group_with == set()
-
     def test_sync_leader_excludes_itself_from_members_can_group_with(
         self, mock_mass: MagicMock
     ) -> None: