From 43a33beeb3045e34bf119a4ffdaf8b893c6ff9c7 Mon Sep 17 00:00:00 2001
From: Komodo <45665554+Komodo5197@users.noreply.github.com>
Date: Sat, 1 Feb 2025 07:12:27 -0500
Subject: [PATCH] [Redesign] Fix sort order 'Random' while offline (#1037)

* Fix sort order 'Random' while offline.

* Add comment.
---
 .../MusicScreen/music_screen_tab_view.dart    | 102 +++++++++---------
 1 file changed, 51 insertions(+), 51 deletions(-)

diff --git a/lib/components/MusicScreen/music_screen_tab_view.dart b/lib/components/MusicScreen/music_screen_tab_view.dart
index 47170bfd4..c94c776e1 100644
--- a/lib/components/MusicScreen/music_screen_tab_view.dart
+++ b/lib/components/MusicScreen/music_screen_tab_view.dart
@@ -1,18 +1,16 @@
 import 'dart:async';
 import 'dart:math';
 
-import 'package:collection/collection.dart';
 import 'package:finamp/components/Buttons/cta_medium.dart';
-import 'package:finamp/components/Buttons/simple_button.dart';
 import 'package:finamp/services/finamp_user_helper.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
+import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 import 'package:flutter_tabler_icons/flutter_tabler_icons.dart';
 import 'package:get_it/get_it.dart';
 import 'package:hive/hive.dart';
 import 'package:infinite_scroll_pagination/infinite_scroll_pagination.dart';
 import 'package:scroll_to_index/scroll_to_index.dart';
-import 'package:flutter_gen/gen_l10n/app_localizations.dart';
 
 import '../../models/finamp_models.dart';
 import '../../models/jellyfin_models.dart';
@@ -163,7 +161,7 @@ class _MusicScreenTabViewState extends State<MusicScreenTabView>
               settings.onlyShowFavourites && settings.trackOfflineFavorites);
     }
 
-    var items = offlineItems.map((e) => e.baseItem).whereNotNull().toList();
+    var items = offlineItems.map((e) => e.baseItem).nonNulls.toList();
 
     items = sortItems(items, settings.tabSortBy[widget.tabContentType],
         settings.tabSortOrder[widget.tabContentType]);
@@ -524,53 +522,55 @@ class _DeferredLoadingAlwaysScrollableScrollPhysics
 
 List<BaseItemDto> sortItems(
     List<BaseItemDto> itemsToSort, SortBy? sortBy, SortOrder? sortOrder) {
-  itemsToSort.sort((a, b) {
-    switch (sortBy ?? SortBy.sortName) {
-      case SortBy.sortName:
-        if (a.nameForSorting == null || b.nameForSorting == null) {
-          // Returning 0 is the same as both being the same
-          return 0;
-        } else {
-          return a.nameForSorting!.compareTo(b.nameForSorting!);
-        }
-      case SortBy.albumArtist:
-        if (a.albumArtist == null || b.albumArtist == null) {
-          return 0;
-        } else {
-          return a.albumArtist!.compareTo(b.albumArtist!);
-        }
-      case SortBy.communityRating:
-        if (a.communityRating == null || b.communityRating == null) {
-          return 0;
-        } else {
-          return a.communityRating!.compareTo(b.communityRating!);
-        }
-      case SortBy.criticRating:
-        if (a.criticRating == null || b.criticRating == null) {
-          return 0;
-        } else {
-          return a.criticRating!.compareTo(b.criticRating!);
-        }
-      case SortBy.dateCreated:
-        if (a.dateCreated == null || b.dateCreated == null) {
-          return 0;
-        } else {
-          return a.dateCreated!.compareTo(b.dateCreated!);
-        }
-      case SortBy.premiereDate:
-        if (a.premiereDate == null || b.premiereDate == null) {
-          return 0;
-        } else {
-          return a.premiereDate!.compareTo(b.premiereDate!);
-        }
-      case SortBy.random:
-        // We subtract the result by one so that we can get -1 values
-        // (see comareTo documentation)
-        return Random().nextInt(2) - 1;
-      default:
-        throw UnimplementedError("Unimplemented offline sort mode $sortBy");
-    }
-  });
+  if (sortBy == SortBy.random) {
+    itemsToSort.shuffle();
+  } else {
+    itemsToSort.sort((a, b) {
+      switch (sortBy ?? SortBy.sortName) {
+        case SortBy.sortName:
+          if (a.nameForSorting == null || b.nameForSorting == null) {
+            // Returning 0 is the same as both being the same
+            return 0;
+          } else {
+            return a.nameForSorting!.compareTo(b.nameForSorting!);
+          }
+        case SortBy.albumArtist:
+          if (a.albumArtist == null || b.albumArtist == null) {
+            return 0;
+          } else {
+            return a.albumArtist!.compareTo(b.albumArtist!);
+          }
+        case SortBy.communityRating:
+          if (a.communityRating == null || b.communityRating == null) {
+            return 0;
+          } else {
+            return a.communityRating!.compareTo(b.communityRating!);
+          }
+        case SortBy.criticRating:
+          if (a.criticRating == null || b.criticRating == null) {
+            return 0;
+          } else {
+            return a.criticRating!.compareTo(b.criticRating!);
+          }
+        case SortBy.dateCreated:
+          if (a.dateCreated == null || b.dateCreated == null) {
+            return 0;
+          } else {
+            return a.dateCreated!.compareTo(b.dateCreated!);
+          }
+        case SortBy.premiereDate:
+          if (a.premiereDate == null || b.premiereDate == null) {
+            return 0;
+          } else {
+            return a.premiereDate!.compareTo(b.premiereDate!);
+          }
+        // SortBy.random is handled outside this switch as per-comparison logic does not produce a good shuffle
+        default:
+          throw UnimplementedError("Unimplemented offline sort mode $sortBy");
+      }
+    });
+  }
+
   return sortOrder == SortOrder.descending
       ? itemsToSort.reversed.toList()
       : itemsToSort;