Clarify the naming around the ID3 settings and methods,

ultrasonic/src/main/kotlin/org/moire/ultrasonic/adapters/ArtistRowBinder.kt

@@ -115,7 +115,7 @@ class ArtistRowBinder(
     }
 
     private fun showArtistPicture(): Boolean {
-        return ActiveServerProvider.isID3Enabled() && Settings.shouldShowArtistPicture
+        return ActiveServerProvider.shouldUseId3Tags() && Settings.shouldShowArtistPicture
     }
 
     /**

ultrasonic/src/main/kotlin/org/moire/ultrasonic/data/ActiveServerProvider.kt

@@ -270,8 +270,8 @@ class ActiveServerProvider(
         /**
          * Queries if ID3 tags should be used
          */
-        fun isID3Enabled(): Boolean {
+        fun shouldUseId3Tags(): Boolean {
-            return Settings.shouldUseId3Tags && (!isOffline() || Settings.useId3TagsOffline)
+            return Settings.id3TagsEnabledOnline && (!isOffline() || Settings.id3TagsEnabledOffline)
         }
 
         /**

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/AlbumListFragment.kt

@@ -138,8 +138,8 @@ class AlbumListFragment(
     )
 
     private fun getListOfSortOrders(): List<SortOrder> {
-        val useId3 = Settings.shouldUseId3Tags
+        val useId3 = Settings.id3TagsEnabledOnline
-        val useId3Offline = Settings.useId3TagsOffline
+        val useId3Offline = Settings.id3TagsEnabledOffline
         val isOnline = !ActiveServerProvider.isOffline()
 
         val supported = mutableListOf<SortOrder>()

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/EntryListFragment.kt

@@ -16,6 +16,7 @@ import androidx.navigation.fragment.findNavController
 import io.reactivex.rxjava3.disposables.CompositeDisposable
 import org.moire.ultrasonic.R
 import org.moire.ultrasonic.adapters.FolderSelectorBinder
+import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.domain.Artist
 import org.moire.ultrasonic.domain.GenericEntry
 import org.moire.ultrasonic.domain.Identifiable
@@ -23,7 +24,6 @@ import org.moire.ultrasonic.service.RxBus
 import org.moire.ultrasonic.service.plusAssign
 import org.moire.ultrasonic.subsonic.DownloadAction
 import org.moire.ultrasonic.subsonic.DownloadHandler
-import org.moire.ultrasonic.util.Settings
 
 /**
  * An extension of the MultiListFragment, with a few helper functions geared
@@ -39,7 +39,7 @@ abstract class EntryListFragment<T : GenericEntry> : MultiListFragment<T>() {
      */
     private fun showFolderHeader(): Boolean {
         return listModel.showSelectFolderHeader() && !listModel.isOffline() &&
-            !Settings.shouldUseId3Tags
+            !ActiveServerProvider.shouldUseId3Tags()
     }
 
     override fun onContextMenuItemSelected(menuItem: MenuItem, item: T): Boolean {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/NowPlayingFragment.kt

@@ -111,7 +111,7 @@ class NowPlayingFragment : Fragment() {
                 nowPlayingArtist!!.text = artist
 
                 nowPlayingAlbumArtImage!!.setOnClickListener {
-                    val id3 = Settings.shouldUseId3Tags
+                    val id3 = Settings.id3TagsEnabledOnline
                     val action = NavigationGraphDirections.toTrackCollection(
                         isAlbum = id3,
                         id = if (id3) file.albumId else file.parent,

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/PlayerFragment.kt

@@ -78,6 +78,7 @@ import org.moire.ultrasonic.adapters.TrackViewBinder
 import org.moire.ultrasonic.api.subsonic.models.AlbumListType
 import org.moire.ultrasonic.audiofx.EqualizerController
 import org.moire.ultrasonic.data.ActiveServerProvider.Companion.isOffline
+import org.moire.ultrasonic.data.ActiveServerProvider.Companion.shouldUseId3Tags
 import org.moire.ultrasonic.data.RatingUpdate
 import org.moire.ultrasonic.domain.Identifiable
 import org.moire.ultrasonic.domain.MusicDirectory
@@ -591,10 +592,10 @@ class PlayerFragment :
             }
         }
 
-        if (isOffline() || !Settings.shouldUseId3Tags) {
+        // Only show the menu if the ID3 tags are available
-            popup.menu.findItem(R.id.menu_show_artist)?.isVisible = false
+        popup.menu.findItem(R.id.menu_show_artist)?.isVisible = shouldUseId3Tags()
-        }
 
+        // Only show the lyrics when the user is online
         popup.menu.findItem(R.id.menu_lyrics)?.isVisible = !isOffline()
         popup.show()
         return popup
@@ -614,7 +615,7 @@ class PlayerFragment :
             R.id.menu_show_artist -> {
                 if (track == null) return false
 
-                if (Settings.shouldUseId3Tags) {
+                if (Settings.id3TagsEnabledOnline) {
                     val action = PlayerFragmentDirections.playerToAlbumsList(
                         type = AlbumListType.SORTED_BY_NAME,
                         byArtist = true,
@@ -630,7 +631,7 @@ class PlayerFragment :
             R.id.menu_show_album -> {
                 if (track == null) return false
 
-                val albumId = if (Settings.shouldUseId3Tags) track.albumId else track.parent
+                val albumId = if (shouldUseId3Tags()) track.albumId else track.parent
 
                 val action = PlayerFragmentDirections.playerToSelectAlbum(
                     id = albumId,

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/SettingsFragment.kt

@@ -39,8 +39,8 @@ import org.moire.ultrasonic.util.ErrorDialog
 import org.moire.ultrasonic.util.FileUtil.ultrasonicDirectory
 import org.moire.ultrasonic.util.InfoDialog
 import org.moire.ultrasonic.util.Settings
+import org.moire.ultrasonic.util.Settings.id3TagsEnabledOnline
 import org.moire.ultrasonic.util.Settings.preferences
-import org.moire.ultrasonic.util.Settings.shouldUseId3Tags
 import org.moire.ultrasonic.util.Storage
 import org.moire.ultrasonic.util.TimeSpanPreference
 import org.moire.ultrasonic.util.TimeSpanPreferenceDialogFragmentCompat
@@ -354,8 +354,8 @@ class SettingsFragment :
             debugLogToFile?.summary = ""
         }
 
-        showArtistPicture?.isEnabled = shouldUseId3Tags
+        showArtistPicture?.isEnabled = id3TagsEnabledOnline
-        useId3TagsOffline?.isEnabled = shouldUseId3Tags
+        useId3TagsOffline?.isEnabled = id3TagsEnabledOnline
     }
 
     private fun setHideMedia(hide: Boolean) {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/fragment/TrackCollectionFragment.kt

@@ -582,7 +582,7 @@ open class TrackCollectionFragment(
             } else {
                 setTitle(name)
 
-                if (ActiveServerProvider.isID3Enabled()) {
+                if (ActiveServerProvider.shouldUseId3Tags()) {
                     if (isAlbum) {
                         listModel.getAlbum(refresh2, id, name)
                     } else {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/model/AlbumListModel.kt

@@ -12,9 +12,9 @@ import androidx.lifecycle.MutableLiveData
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import org.moire.ultrasonic.api.subsonic.models.AlbumListType
+import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.domain.Album
 import org.moire.ultrasonic.service.MusicServiceFactory
-import org.moire.ultrasonic.util.Settings
 
 class AlbumListModel(application: Application) : GenericListModel(application) {
 
@@ -69,7 +69,7 @@ class AlbumListModel(application: Application) : GenericListModel(application) {
             // If appending the existing list, set the offset from where to load
             if (append) offset += (size + loadedUntil)
 
-            musicDirectory = if (Settings.shouldUseId3Tags) {
+            musicDirectory = if (ActiveServerProvider.shouldUseId3Tags()) {
                 service.getAlbumList2(
                     albumListType, size,
                     offset, musicFolderId
@@ -119,7 +119,7 @@ class AlbumListModel(application: Application) : GenericListModel(application) {
         val isAlphabetical = (lastType == AlbumListType.SORTED_BY_NAME) ||
             (lastType == AlbumListType.SORTED_BY_ARTIST)
 
-        return !isOffline() && !Settings.shouldUseId3Tags && isAlphabetical
+        return !isOffline() && !ActiveServerProvider.shouldUseId3Tags() && isAlphabetical
     }
 
     private fun isCollectionSortable(albumListType: AlbumListType): Boolean {

ultrasonic/src/main/kotlin/org/moire/ultrasonic/model/ArtistListModel.kt

@@ -43,7 +43,7 @@ class ArtistListModel(application: Application) : GenericListModel(application)
 
         val musicFolderId = activeServer.musicFolderId
 
-        val result = if (ActiveServerProvider.isID3Enabled()) {
+        val result = if (ActiveServerProvider.shouldUseId3Tags()) {
             musicService.getArtists(refresh)
         } else {
             musicService.getIndexes(musicFolderId, refresh)

ultrasonic/src/main/kotlin/org/moire/ultrasonic/model/GenericListModel.kt

@@ -26,7 +26,6 @@ import org.moire.ultrasonic.domain.MusicFolder
 import org.moire.ultrasonic.service.MusicService
 import org.moire.ultrasonic.service.MusicServiceFactory
 import org.moire.ultrasonic.util.CommunicationError
-import org.moire.ultrasonic.util.Settings
 
 /**
  * An abstract Model, which can be extended to retrieve a list of items from the API
@@ -89,7 +88,7 @@ open class GenericListModel(application: Application) :
         withContext(Dispatchers.IO) {
             val musicService = MusicServiceFactory.getMusicService()
             val isOffline = ActiveServerProvider.isOffline()
-            val useId3Tags = Settings.shouldUseId3Tags
+            val useId3Tags = ActiveServerProvider.shouldUseId3Tags()
 
             try {
                 load(isOffline, useId3Tags, musicService, refresh)

ultrasonic/src/main/kotlin/org/moire/ultrasonic/model/TrackCollectionModel.kt

@@ -13,12 +13,12 @@ import androidx.lifecycle.viewModelScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
+import org.moire.ultrasonic.data.ActiveServerProvider
 import org.moire.ultrasonic.domain.MusicDirectory
 import org.moire.ultrasonic.domain.Track
 import org.moire.ultrasonic.service.DownloadService
 import org.moire.ultrasonic.service.DownloadState
 import org.moire.ultrasonic.service.MusicServiceFactory
-import org.moire.ultrasonic.util.Settings
 import org.moire.ultrasonic.util.Util
 
 /*
@@ -72,7 +72,7 @@ class TrackCollectionModel(application: Application) : GenericListModel(applicat
             val service = MusicServiceFactory.getMusicService()
             val musicDirectory: MusicDirectory
 
-            musicDirectory = if (Settings.shouldUseId3Tags) {
+            musicDirectory = if (ActiveServerProvider.shouldUseId3Tags()) {
                 Util.getSongsFromSearchResult(service.getStarred2())
             } else {
                 Util.getSongsFromSearchResult(service.getStarred())

ultrasonic/src/main/kotlin/org/moire/ultrasonic/playback/AutoMediaBrowserCallback.kt

@@ -51,7 +51,6 @@ import org.moire.ultrasonic.service.MediaPlayerController
 import org.moire.ultrasonic.service.MusicServiceFactory
 import org.moire.ultrasonic.service.RatingManager
 import org.moire.ultrasonic.util.MainThreadExecutor
-import org.moire.ultrasonic.util.Settings
 import org.moire.ultrasonic.util.Util
 import org.moire.ultrasonic.util.buildMediaItem
 import org.moire.ultrasonic.util.toMediaItem
@@ -119,7 +118,6 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
 
     private val musicService get() = MusicServiceFactory.getMusicService()
     private val isOffline get() = ActiveServerProvider.isOffline()
-    private val useId3Tags get() = Settings.shouldUseId3Tags
     private val musicFolderId get() = activeServerProvider.getActiveServer().musicFolderId
 
     /**
@@ -661,7 +659,7 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
             var childMediaId: String = MEDIA_ARTIST_ITEM
 
             var artists = serviceScope.future {
-                if (!isOffline && useId3Tags) {
+                if (ActiveServerProvider.shouldUseId3Tags()) {
                     // TODO this list can be big so we're not refreshing.
                     //  Maybe a refresh menu item can be added
                     callWithErrorHandling { musicService.getArtists(false) }
@@ -716,7 +714,7 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
 
         return mainScope.future {
             val albums = serviceScope.future {
-                if (!isOffline && useId3Tags) {
+                if (ActiveServerProvider.shouldUseId3Tags()) {
                     callWithErrorHandling { musicService.getAlbumsOfArtist(id, name, false) }
                 } else {
                     callWithErrorHandling {
@@ -788,7 +786,7 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
             val offset = (page ?: 0) * DISPLAY_LIMIT
 
             val albums = serviceScope.future {
-                if (useId3Tags) {
+                if (ActiveServerProvider.shouldUseId3Tags()) {
                     callWithErrorHandling {
                         musicService.getAlbumList2(
                             type, DISPLAY_LIMIT, offset, null
@@ -1190,7 +1188,7 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
 
     private fun listSongsInMusicService(id: String, name: String?): MusicDirectory? {
         return serviceScope.future {
-            if (!ActiveServerProvider.isOffline() && Settings.shouldUseId3Tags) {
+            if (ActiveServerProvider.shouldUseId3Tags()) {
                 callWithErrorHandling { musicService.getAlbumAsDir(id, name, false) }
             } else {
                 callWithErrorHandling { musicService.getMusicDirectory(id, name, false) }
@@ -1200,7 +1198,7 @@ class AutoMediaBrowserCallback(var player: Player, val libraryService: MediaLibr
 
     private fun listStarredSongsInMusicService(): SearchResult? {
         return serviceScope.future {
-            if (Settings.shouldUseId3Tags) {
+            if (ActiveServerProvider.shouldUseId3Tags()) {
                 callWithErrorHandling { musicService.getStarred2() }
             } else {
                 callWithErrorHandling { musicService.getStarred() }

ultrasonic/src/main/kotlin/org/moire/ultrasonic/service/RESTMusicService.kt

@@ -18,7 +18,7 @@ import org.moire.ultrasonic.api.subsonic.models.JukeboxAction
 import org.moire.ultrasonic.api.subsonic.throwOnFailure
 import org.moire.ultrasonic.api.subsonic.toStreamResponse
 import org.moire.ultrasonic.data.ActiveServerProvider
-import org.moire.ultrasonic.data.ActiveServerProvider.Companion.isOffline
+import org.moire.ultrasonic.data.ActiveServerProvider.Companion.shouldUseId3Tags
 import org.moire.ultrasonic.domain.Album
 import org.moire.ultrasonic.domain.Artist
 import org.moire.ultrasonic.domain.Bookmark
@@ -44,7 +44,6 @@ import org.moire.ultrasonic.domain.toIndexList
 import org.moire.ultrasonic.domain.toMusicDirectoryDomainEntity
 import org.moire.ultrasonic.domain.toTrackEntity
 import org.moire.ultrasonic.util.FileUtil
-import org.moire.ultrasonic.util.Settings
 import timber.log.Timber
 
 /**
@@ -181,7 +180,7 @@ open class RESTMusicService(
         criteria: SearchCriteria
     ): SearchResult {
         return try {
-            if (!isOffline() && Settings.shouldUseId3Tags) {
+            if (shouldUseId3Tags()) {
                 search3(criteria)
             } else {
                 search2(criteria)

ultrasonic/src/main/kotlin/org/moire/ultrasonic/subsonic/DownloadHandler.kt

@@ -14,7 +14,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import org.moire.ultrasonic.R
-import org.moire.ultrasonic.data.ActiveServerProvider.Companion.isOffline
+import org.moire.ultrasonic.data.ActiveServerProvider.Companion.shouldUseId3Tags
 import org.moire.ultrasonic.domain.MusicDirectory
 import org.moire.ultrasonic.domain.Track
 import org.moire.ultrasonic.service.DownloadService
@@ -181,11 +181,11 @@ class DownloadHandler(
         val musicService = getMusicService()
         val songs: MutableList<Track> = LinkedList()
         val root: MusicDirectory
-        if (!isOffline() && isArtist && Settings.shouldUseId3Tags) {
+        if (shouldUseId3Tags() && isArtist) {
-            getSongsForArtist(id, songs)
+            return getSongsForArtist(id)
         } else {
             if (isDirectory) {
-                root = if (!isOffline() && Settings.shouldUseId3Tags)
+                root = if (shouldUseId3Tags())
                     musicService.getAlbumAsDir(id, name, false)
                 else
                     musicService.getMusicDirectory(id, name, false)
@@ -219,23 +219,19 @@ class DownloadHandler(
         }
         val musicService = getMusicService()
         for ((id1, _, _, title) in parent.getAlbums()) {
-            val root: MusicDirectory = if (
+            val root: MusicDirectory = if (shouldUseId3Tags())
-                !isOffline() &&
+                musicService.getAlbumAsDir(id1, title, false)
-                Settings.shouldUseId3Tags
+            else
-            ) musicService.getAlbumAsDir(id1, title, false)
+                musicService.getMusicDirectory(id1, title, false)
-            else musicService.getMusicDirectory(id1, title, false)
             getSongsRecursively(root, songs)
         }
     }
 
     @Throws(Exception::class)
     private fun getSongsForArtist(
-        id: String,
+        id: String
-        songs: MutableCollection<Track>
+    ): MutableList<Track> {
-    ) {
+        val songs: MutableList<Track> = LinkedList()
-        if (songs.size > maxSongs) {
-            return
-        }
         val musicService = getMusicService()
         val artist = musicService.getAlbumsOfArtist(id, "", false)
         for ((id1) in artist) {
@@ -250,6 +246,7 @@ class DownloadHandler(
                 }
             }
         }
+        return songs
     }
 }
 

ultrasonic/src/main/kotlin/org/moire/ultrasonic/util/Settings.kt

@@ -168,11 +168,11 @@ object Settings {
     // Normally you don't need to use these Settings directly,
     // use ActiveServerProvider.isID3Enabled() instead
     @JvmStatic
-    var shouldUseId3Tags by BooleanSetting(getKey(R.string.setting_key_id3_tags), true)
+    var id3TagsEnabledOnline by BooleanSetting(getKey(R.string.setting_key_id3_tags), true)
 
     // See comment above.
     @JvmStatic
-    var useId3TagsOffline by BooleanSetting(getKey(R.string.setting_key_id3_tags_offline), true)
+    var id3TagsEnabledOffline by BooleanSetting(getKey(R.string.setting_key_id3_tags_offline), true)
 
     var activeServer by IntSetting(getKey(R.string.setting_key_server_instance), -1)