Reworked incorrect choice handling and centralized it

app/src/main/java/org/schabi/newpipe/RouterActivity.java

@@ -24,6 +24,7 @@ import android.widget.Toast;
 import androidx.annotation.DrawableRes;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.StringRes;
 import androidx.appcompat.app.AlertDialog;
 import androidx.appcompat.app.AppCompatActivity;
 import androidx.appcompat.content.res.AppCompatResources;
@@ -259,42 +260,37 @@ public class RouterActivity extends AppCompatActivity {
     protected void onSuccess() {
         final SharedPreferences preferences = PreferenceManager
                 .getDefaultSharedPreferences(this);
-        final String selectedChoiceKey = preferences
-                .getString(getString(R.string.preferred_open_action_key),
-                        getString(R.string.preferred_open_action_default));
 
-        final String showInfoKey = getString(R.string.show_info_key);
+        final ChoiceAvailabilityChecker choiceChecker = new ChoiceAvailabilityChecker(
+                getChoicesForService(currentService, currentLinkType),
+                preferences.getString(getString(R.string.preferred_open_action_key),
+                        getString(R.string.preferred_open_action_default)));
 
-        if (selectedChoiceKey.equals(getString(R.string.always_ask_open_action_key))) {
+        // Check for non-player related choices
-            final List<AdapterChoiceItem> choices =
+        if (choiceChecker.isAvailableAndSelected(
-                    getChoicesForService(currentService, currentLinkType);
+                R.string.show_info_key,
+                R.string.download_key,
+                R.string.add_to_playlist_key)) {
+            handleChoice(choiceChecker.getSelectedChoiceKey());
+            return;
+        }
+        // Check if the choice is player related
+        if (choiceChecker.isAvailableAndSelected(
+                R.string.video_player_key,
+                R.string.background_player_key,
+                R.string.popup_player_key)) {
+
+            final String selectedChoice = choiceChecker.getSelectedChoiceKey();
 
-            switch (choices.size()) {
-                case 1:
-                    handleChoice(choices.get(0).key);
-                    break;
-                case 0:
-                    handleChoice(showInfoKey);
-                    break;
-                default:
-                    showDialog(choices);
-                    break;
-            }
-        } else if (selectedChoiceKey.equals(showInfoKey)
-                || selectedChoiceKey.equals(getString(R.string.download_key))
-                || selectedChoiceKey.equals(getString(R.string.add_to_playlist_key))
-        ) {
-            handleChoice(selectedChoiceKey);
-        } else {
             final boolean isExtVideoEnabled = preferences.getBoolean(
                     getString(R.string.use_external_video_player_key), false);
             final boolean isExtAudioEnabled = preferences.getBoolean(
                     getString(R.string.use_external_audio_player_key), false);
             final boolean isVideoPlayerSelected =
-                    selectedChoiceKey.equals(getString(R.string.video_player_key))
+                    selectedChoice.equals(getString(R.string.video_player_key))
-                            || selectedChoiceKey.equals(getString(R.string.popup_player_key));
+                            || selectedChoice.equals(getString(R.string.popup_player_key));
             final boolean isAudioPlayerSelected =
-                    selectedChoiceKey.equals(getString(R.string.background_player_key));
+                    selectedChoice.equals(getString(R.string.background_player_key));
 
             if (currentLinkType != LinkType.STREAM
                     && (isExtAudioEnabled && isAudioPlayerSelected
@@ -302,25 +298,72 @@ public class RouterActivity extends AppCompatActivity {
             ) {
                 Toast.makeText(this, R.string.external_player_unsupported_link_type,
                         Toast.LENGTH_LONG).show();
-                handleChoice(showInfoKey);
+                handleChoice(getString(R.string.show_info_key));
                 return;
             }
 
             final List<StreamingService.ServiceInfo.MediaCapability> capabilities =
                     currentService.getServiceInfo().getMediaCapabilities();
 
-            boolean serviceSupportsChoice = false;
+            // Check if the service supports the choice
-            if (isVideoPlayerSelected) {
+            if (isVideoPlayerSelected && capabilities.contains(VIDEO)
-                serviceSupportsChoice = capabilities.contains(VIDEO);
+                    || isAudioPlayerSelected && capabilities.contains(AUDIO)) {
-            } else if (isAudioPlayerSelected) {
+                handleChoice(selectedChoice);
-                serviceSupportsChoice = capabilities.contains(AUDIO);
-            }
-
-            if (serviceSupportsChoice) {
-                handleChoice(selectedChoiceKey);
             } else {
-                handleChoice(showInfoKey);
+                handleChoice(getString(R.string.show_info_key));
             }
+            return;
+        }
+
+        // Default / Ask always
+        final List<AdapterChoiceItem> availableChoices = choiceChecker.getAvailableChoices();
+        switch (availableChoices.size()) {
+            case 1:
+                handleChoice(availableChoices.get(0).key);
+                break;
+            case 0:
+                handleChoice(getString(R.string.show_info_key));
+                break;
+            default:
+                showDialog(availableChoices);
+                break;
+        }
+    }
+
+    /**
+     * This is a helper class for checking if the choices are available and/or selected.
+     */
+    class ChoiceAvailabilityChecker {
+        private final List<AdapterChoiceItem> availableChoices;
+        private final String selectedChoiceKey;
+
+        ChoiceAvailabilityChecker(
+                @NonNull final List<AdapterChoiceItem> availableChoices,
+                @NonNull final String selectedChoiceKey) {
+            this.availableChoices = availableChoices;
+            this.selectedChoiceKey = selectedChoiceKey;
+        }
+
+        public List<AdapterChoiceItem> getAvailableChoices() {
+            return availableChoices;
+        }
+
+        public String getSelectedChoiceKey() {
+            return selectedChoiceKey;
+        }
+
+        public boolean isAvailableAndSelected(@StringRes final int... wantedKeys) {
+            return Arrays.stream(wantedKeys).anyMatch(this::isAvailableAndSelected);
+        }
+
+        public boolean isAvailableAndSelected(@StringRes final int wantedKey) {
+            final String wanted = getString(wantedKey);
+            // Check if the wanted option is selected
+            if (!selectedChoiceKey.equals(wanted)) {
+                return false;
+            }
+            // Check if it's available
+            return availableChoices.stream().anyMatch(item -> wanted.equals(item.key));
         }
     }
 
@@ -425,87 +468,61 @@ public class RouterActivity extends AppCompatActivity {
 
     private List<AdapterChoiceItem> getChoicesForService(final StreamingService service,
                                                          final LinkType linkType) {
-        final Context context = getThemeWrapperContext();
-
-        final List<AdapterChoiceItem> returnList = new ArrayList<>();
-        final List<StreamingService.ServiceInfo.MediaCapability> capabilities
-                = service.getServiceInfo().getMediaCapabilities();
-
-        final SharedPreferences preferences = PreferenceManager
-                .getDefaultSharedPreferences(this);
-        final boolean isExtVideoEnabled = preferences.getBoolean(
-                getString(R.string.use_external_video_player_key), false);
-        final boolean isExtAudioEnabled = preferences.getBoolean(
-                getString(R.string.use_external_audio_player_key), false);
-
-        final AdapterChoiceItem videoPlayer = new AdapterChoiceItem(
-                getString(R.string.video_player_key), getString(R.string.video_player),
-                R.drawable.ic_play_arrow);
         final AdapterChoiceItem showInfo = new AdapterChoiceItem(
                 getString(R.string.show_info_key), getString(R.string.show_info),
                 R.drawable.ic_info_outline);
-        final AdapterChoiceItem popupPlayer = new AdapterChoiceItem(
+        final AdapterChoiceItem videoPlayer = new AdapterChoiceItem(
-                getString(R.string.popup_player_key), getString(R.string.popup_player),
+                getString(R.string.video_player_key), getString(R.string.video_player),
-                R.drawable.ic_picture_in_picture);
+                R.drawable.ic_play_arrow);
         final AdapterChoiceItem backgroundPlayer = new AdapterChoiceItem(
                 getString(R.string.background_player_key), getString(R.string.background_player),
                 R.drawable.ic_headset);
-        final AdapterChoiceItem addToPlaylist = new AdapterChoiceItem(
+        final AdapterChoiceItem popupPlayer = new AdapterChoiceItem(
-                getString(R.string.add_to_playlist_key), getString(R.string.add_to_playlist),
+                getString(R.string.popup_player_key), getString(R.string.popup_player),
-                R.drawable.ic_add);
+                R.drawable.ic_picture_in_picture);
 
+        final List<AdapterChoiceItem> returnedItems = new ArrayList<>();
+        returnedItems.add(showInfo); // Always present
+        final List<StreamingService.ServiceInfo.MediaCapability> capabilities =
+                service.getServiceInfo().getMediaCapabilities();
 
         if (linkType == LinkType.STREAM) {
-            if (isExtVideoEnabled) {
-                // show both "show info" and "video player", they are two different activities
-                returnList.add(showInfo);
-                returnList.add(videoPlayer);
-            } else {
-                final MainPlayer.PlayerType playerType = PlayerHolder.getInstance().getType();
-                if (capabilities.contains(VIDEO)
-                        && PlayerHelper.isAutoplayAllowedByUser(context)
-                        && playerType == null || playerType == MainPlayer.PlayerType.VIDEO) {
-                    // show only "video player" since the details activity will be opened and the
-                    // video will be auto played there. Since "show info" would do the exact same
-                    // thing, use that as a key to let VideoDetailFragment load the stream instead
-                    // of using FetcherService (see comment in handleChoice())
-                    returnList.add(new AdapterChoiceItem(
-                            showInfo.key, videoPlayer.description, videoPlayer.icon));
-                } else {
-                    // show only "show info" if video player is not applicable, auto play is
-                    // disabled or a video is playing in a player different than the main one
-                    returnList.add(showInfo);
-                }
-            }
-
             if (capabilities.contains(VIDEO)) {
-                returnList.add(popupPlayer);
+                returnedItems.add(videoPlayer);
+                returnedItems.add(popupPlayer);
             }
             if (capabilities.contains(AUDIO)) {
-                returnList.add(backgroundPlayer);
+                returnedItems.add(backgroundPlayer);
             }
             // download is redundant for linkType CHANNEL AND PLAYLIST (till playlist downloading is
             // not supported )
-            returnList.add(new AdapterChoiceItem(getString(R.string.download_key),
+            returnedItems.add(new AdapterChoiceItem(getString(R.string.download_key),
                     getString(R.string.download),
                     R.drawable.ic_file_download));
 
             // Add to playlist is not necessary for CHANNEL and PLAYLIST linkType since those can
             // not be added to a playlist
-            returnList.add(addToPlaylist);
+            returnedItems.add(new AdapterChoiceItem(getString(R.string.add_to_playlist_key),
-
+                    getString(R.string.add_to_playlist),
+                    R.drawable.ic_add));
         } else {
-            returnList.add(showInfo);
+            final SharedPreferences preferences = PreferenceManager
+                    .getDefaultSharedPreferences(this);
+            final boolean isExtVideoEnabled = preferences.getBoolean(
+                    getString(R.string.use_external_video_player_key), false);
+            final boolean isExtAudioEnabled = preferences.getBoolean(
+                    getString(R.string.use_external_audio_player_key), false);
+
             if (capabilities.contains(VIDEO) && !isExtVideoEnabled) {
-                returnList.add(videoPlayer);
+                returnedItems.add(videoPlayer);
-                returnList.add(popupPlayer);
+                returnedItems.add(popupPlayer);
             }
             if (capabilities.contains(AUDIO) && !isExtAudioEnabled) {
-                returnList.add(backgroundPlayer);
+                returnedItems.add(backgroundPlayer);
             }
         }
 
-        return returnList;
+        return returnedItems;
     }
 
     private Context getThemeWrapperContext() {
@@ -567,7 +584,8 @@ public class RouterActivity extends AppCompatActivity {
 
         // stop and bypass FetcherService if InfoScreen was selected since
         // StreamDetailFragment can fetch data itself
-        if (selectedChoiceKey.equals(getString(R.string.show_info_key))) {
+        if (selectedChoiceKey.equals(getString(R.string.show_info_key))
+                || canHandleChoiceLikeShowInfo(selectedChoiceKey)) {
             disposables.add(Observable
                     .fromCallable(() -> NavigationHelper.getIntentByLink(this, currentUrl))
                     .subscribeOn(Schedulers.io())
@@ -590,6 +608,32 @@ public class RouterActivity extends AppCompatActivity {
         finish();
     }
 
+    // show only "video player" since the details activity will be opened and the
+    // video will be auto played there.
+    private boolean canHandleChoiceLikeShowInfo(final String selectedChoiceKey) {
+        // "video player" can be handled like "show info" when...
+        if (selectedChoiceKey.equals(getString(R.string.video_player_key))) {
+            // Autoplay is enabled
+            if (!PlayerHelper.isAutoplayAllowedByUser(getThemeWrapperContext())) {
+                return false;
+            }
+
+            final boolean isExtVideoEnabled = PreferenceManager.getDefaultSharedPreferences(this)
+                    .getBoolean(getString(R.string.use_external_video_player_key), false);
+            // Ignore it when it's done via an external player
+            if (isExtVideoEnabled) {
+                return false;
+            }
+
+            // The player is not running or in Video-mode/type
+            final MainPlayer.PlayerType playerType = PlayerHolder.getInstance().getType();
+            return playerType == null || playerType == MainPlayer.PlayerType.VIDEO;
+            // Since "show info" would do the exact same thing, use that as a key to let
+            // VideoDetailFragment load the stream instead of using FetcherService
+        }
+        return false;
+    }
+
     private void openAddToPlaylistDialog() {
         // Getting the stream info usually takes a moment
         // Notifying the user here to ensure that no confusion arises
@@ -672,8 +716,8 @@ public class RouterActivity extends AppCompatActivity {
         final int icon;
 
         AdapterChoiceItem(final String key, final String description, final int icon) {
-            this.description = description;
             this.key = key;
+            this.description = description;
             this.icon = icon;
         }
     }