From 01603f52de6c30f20a20affe61d17f8b2a209d03 Mon Sep 17 00:00:00 2001 From: Joshua Hughes Date: Sat, 31 Jan 2026 15:24:45 -0500 Subject: [PATCH 1/2] fix: initialize VolumeController at app launch to prevent race condition MPVolumeView needs to be in the view hierarchy and laid out before its internal slider subview is accessible. Previously, volume adjustments could fail silently if called before the view was ready. - Add explicit initialize() method called from ShuffledApp.init() - Force layout after adding to hierarchy so subviews are populated - Consolidate increase/decrease into shared adjustVolume(by:) helper - Keep assertionFailure for catching issues during development Co-Authored-By: Claude Opus 4.5 --- Shfl/ShuffledApp.swift | 4 +++ Shfl/Utilities/VolumeController.swift | 38 ++++++++++++++++----------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Shfl/ShuffledApp.swift b/Shfl/ShuffledApp.swift index 9f37964..d6d510d 100644 --- a/Shfl/ShuffledApp.swift +++ b/Shfl/ShuffledApp.swift @@ -12,6 +12,10 @@ import SwiftData struct ShuffledApp: App { @State private var motionManager = MotionManager() + init() { + VolumeController.initialize() + } + var sharedModelContainer: ModelContainer = { let schema = Schema([PersistedSong.self]) let modelConfiguration = ModelConfiguration(schema: schema, isStoredInMemoryOnly: false) diff --git a/Shfl/Utilities/VolumeController.swift b/Shfl/Utilities/VolumeController.swift index 36f2a63..d5fc73e 100644 --- a/Shfl/Utilities/VolumeController.swift +++ b/Shfl/Utilities/VolumeController.swift @@ -13,6 +13,7 @@ import UIKit /// - The view hierarchy lookup fails for other reasons enum VolumeController { private static let volumeStep: Float = 0.0625 // 1/16, matches iOS default + private static var isInitialized = false private static var volumeView: MPVolumeView = { let view = MPVolumeView(frame: .zero) @@ -24,32 +25,37 @@ enum VolumeController { volumeView.subviews.compactMap { $0 as? UISlider }.first } - private static func ensureVolumeViewInHierarchy() { - guard volumeView.superview == nil, - let window = UIApplication.shared.connectedScenes - .compactMap({ $0 as? UIWindowScene }) - .first?.windows.first else { return } + /// Call this early in app lifecycle (e.g., from AppDelegate or initial view) + /// to ensure the volume view is ready before user interaction. + static func initialize() { + guard !isInitialized else { return } + isInitialized = true + + guard let window = UIApplication.shared.connectedScenes + .compactMap({ $0 as? UIWindowScene }) + .first?.windows.first else { + assertionFailure("VolumeController: No window available during initialization") + return + } window.addSubview(volumeView) + // Force layout so subviews are populated + volumeView.layoutIfNeeded() } static func increaseVolume() { - ensureVolumeViewInHierarchy() - guard let slider = volumeSlider else { - assertionFailure("VolumeController: Could not find volume slider in MPVolumeView hierarchy") - return - } - let newValue = min(slider.value + volumeStep, 1.0) - slider.value = newValue - slider.sendActions(for: .touchUpInside) + adjustVolume(by: volumeStep) } static func decreaseVolume() { - ensureVolumeViewInHierarchy() + adjustVolume(by: -volumeStep) + } + + private static func adjustVolume(by delta: Float) { guard let slider = volumeSlider else { - assertionFailure("VolumeController: Could not find volume slider in MPVolumeView hierarchy") + assertionFailure("VolumeController: Slider unavailable. Ensure initialize() is called on app launch.") return } - let newValue = max(slider.value - volumeStep, 0.0) + let newValue = max(0.0, min(slider.value + delta, 1.0)) slider.value = newValue slider.sendActions(for: .touchUpInside) } From 4fc840834a05648dfa848b9d063cd9c541445804 Mon Sep 17 00:00:00 2001 From: Joshua Hughes Date: Sat, 31 Jan 2026 16:16:24 -0500 Subject: [PATCH 2/2] refactor: migrate to @Observable and fix VolumeController initialization ## @Observable Migration - Migrate 6 classes from ObservableObject to @Observable macro: - ShufflePlayer, AppViewModel, LibraryBrowserViewModel - SongUndoManager, ArtworkLoader, AlbumArtColorExtractor - Replace @StateObject/@ObservedObject with @State/var in views - Use @Bindable for bindings from @Observable objects - Use @ObservationIgnored for non-observed internal state ## Environment-Based Settings - Create AppSettings (@Observable) to centralize app settings - Add Environment keys for appSettings and shufflePlayer - Replace NotificationCenter-based communication with direct observation - Settings views now modify appSettings directly ## VolumeController Fix - Move initialization from App.init() to MainView.onAppear - Remove assertionFailure that caused crash when no window available - Add lazy initialization fallback in adjustVolume() ## Performance Fix - Make ShufflePlayer's playback observation Task @MainActor - Simplify AppViewModel's scrobble observation to avoid actor-hopping - Fixes multi-second lag on play/pause UI updates Co-Authored-By: Claude Opus 4.5 --- Shfl/Domain/AppSettings.swift | 52 +++++++++ Shfl/Domain/ShufflePlayer.swift | 50 +++----- Shfl/Services/ArtworkLoader.swift | 17 +-- Shfl/ShuffledApp.swift | 4 - Shfl/Utilities/AlbumArtColorExtractor.swift | 12 +- Shfl/Utilities/VolumeController.swift | 16 ++- Shfl/ViewModels/AppViewModel.swift | 63 +++++++---- Shfl/ViewModels/LibraryBrowserViewModel.swift | 107 ++++++++---------- Shfl/ViewModels/SongUndoManager.swift | 6 +- Shfl/Views/Components/SongDisplay.swift | 4 +- Shfl/Views/MainView.swift | 22 +++- Shfl/Views/ManageView.swift | 2 +- Shfl/Views/PlayerView.swift | 4 +- Shfl/Views/Settings/DebugQueueView.swift | 90 ++++++++------- .../Settings/LibrarySortingSettingsView.swift | 17 +-- .../ShuffleAlgorithmSettingsView.swift | 21 ++-- Shfl/Views/SongPickerView.swift | 17 ++- 17 files changed, 284 insertions(+), 220 deletions(-) create mode 100644 Shfl/Domain/AppSettings.swift diff --git a/Shfl/Domain/AppSettings.swift b/Shfl/Domain/AppSettings.swift new file mode 100644 index 0000000..c45c6f7 --- /dev/null +++ b/Shfl/Domain/AppSettings.swift @@ -0,0 +1,52 @@ +import Foundation +import SwiftUI + +/// Centralized app settings using @Observable for automatic SwiftUI updates. +/// Replaces NotificationCenter-based communication for settings changes. +@Observable +@MainActor +final class AppSettings { + var shuffleAlgorithm: ShuffleAlgorithm { + didSet { + guard shuffleAlgorithm != oldValue else { return } + UserDefaults.standard.set(shuffleAlgorithm.rawValue, forKey: "shuffleAlgorithm") + } + } + + var librarySortOption: SortOption { + didSet { + guard librarySortOption != oldValue else { return } + UserDefaults.standard.set(librarySortOption.rawValue, forKey: "librarySortOption") + } + } + + init() { + let algorithmRaw = UserDefaults.standard.string(forKey: "shuffleAlgorithm") ?? ShuffleAlgorithm.noRepeat.rawValue + self.shuffleAlgorithm = ShuffleAlgorithm(rawValue: algorithmRaw) ?? .noRepeat + + let sortRaw = UserDefaults.standard.string(forKey: "librarySortOption") ?? SortOption.mostPlayed.rawValue + self.librarySortOption = SortOption(rawValue: sortRaw) ?? .mostPlayed + } +} + +// MARK: - Environment Keys + +private struct AppSettingsKey: EnvironmentKey { + static let defaultValue: AppSettings? = nil +} + +private struct ShufflePlayerKey: EnvironmentKey { + static let defaultValue: ShufflePlayer? = nil +} + +extension EnvironmentValues { + var appSettings: AppSettings? { + get { self[AppSettingsKey.self] } + set { self[AppSettingsKey.self] = newValue } + } + + var shufflePlayer: ShufflePlayer? { + get { self[ShufflePlayerKey.self] } + set { self[ShufflePlayerKey.self] = newValue } + } +} diff --git a/Shfl/Domain/ShufflePlayer.swift b/Shfl/Domain/ShufflePlayer.swift index 5f35022..866ec50 100644 --- a/Shfl/Domain/ShufflePlayer.swift +++ b/Shfl/Domain/ShufflePlayer.swift @@ -1,4 +1,3 @@ -import Combine import Foundation enum ShufflePlayerError: Error, Equatable { @@ -7,24 +6,25 @@ enum ShufflePlayerError: Error, Equatable { case playbackFailed(String) } +@Observable @MainActor -final class ShufflePlayer: ObservableObject { +final class ShufflePlayer { static let maxSongs = 120 - private let musicService: MusicService - @Published private(set) var songs: [Song] = [] - private var stateTask: Task? + @ObservationIgnored private let musicService: MusicService + private(set) var songs: [Song] = [] + @ObservationIgnored private var stateTask: Task? - @Published private(set) var playbackState: PlaybackState = .empty + private(set) var playbackState: PlaybackState = .empty /// Debug: The last shuffled queue order (for verifying shuffle algorithms) - @Published private(set) var lastShuffledQueue: [Song] = [] + private(set) var lastShuffledQueue: [Song] = [] /// Debug: The algorithm used for the last shuffle - @Published private(set) var lastUsedAlgorithm: ShuffleAlgorithm = .noRepeat + private(set) var lastUsedAlgorithm: ShuffleAlgorithm = .noRepeat - private var playedSongIds: Set = [] - private var lastObservedSongId: String? - private var preparedSongIds: Set = [] + @ObservationIgnored private var playedSongIds: Set = [] + @ObservationIgnored private var lastObservedSongId: String? + @ObservationIgnored private var preparedSongIds: Set = [] private var isQueuePrepared: Bool { Set(songs.map(\.id)) == preparedSongIds @@ -38,32 +38,15 @@ final class ShufflePlayer: ObservableObject { /// Exposed for testing only var playedSongIdsForTesting: Set { playedSongIds } - private var algorithmObserver: NSObjectProtocol? - init(musicService: MusicService) { self.musicService = musicService observePlaybackState() - observeAlgorithmChanges() - } - - private func observeAlgorithmChanges() { - algorithmObserver = NotificationCenter.default.addObserver( - forName: .shuffleAlgorithmChanged, - object: nil, - queue: .main - ) { [weak self] _ in - Task { @MainActor [weak self] in - await self?.reshuffleWithNewAlgorithm() - } - } } - private func reshuffleWithNewAlgorithm() async { + /// Called when shuffle algorithm changes. Views should call this via onChange(of: appSettings.shuffleAlgorithm). + func reshuffleWithNewAlgorithm(_ algorithm: ShuffleAlgorithm) async { guard !songs.isEmpty, playbackState.isActive else { return } - let algorithmRaw = UserDefaults.standard.string(forKey: "shuffleAlgorithm") ?? ShuffleAlgorithm.noRepeat.rawValue - let algorithm = ShuffleAlgorithm(rawValue: algorithmRaw) ?? .noRepeat - print("🎲 Algorithm changed to \(algorithm.displayName), reshuffling...") // Get currently playing song @@ -101,15 +84,12 @@ final class ShufflePlayer: ObservableObject { deinit { stateTask?.cancel() - if let observer = algorithmObserver { - NotificationCenter.default.removeObserver(observer) - } } private func observePlaybackState() { - stateTask = Task { [weak self] in + stateTask = Task { @MainActor [weak self] in guard let self else { return } - for await state in musicService.playbackStateStream { + for await state in self.musicService.playbackStateStream { self.handlePlaybackStateChange(state) } } diff --git a/Shfl/Services/ArtworkLoader.swift b/Shfl/Services/ArtworkLoader.swift index 28394e6..2d4995d 100644 --- a/Shfl/Services/ArtworkLoader.swift +++ b/Shfl/Services/ArtworkLoader.swift @@ -1,16 +1,19 @@ -import Combine import MusicKit import SwiftUI /// Lazy artwork loader with rate limiting to avoid overwhelming MusicKit +@Observable @MainActor -final class ArtworkLoader: ObservableObject { +final class ArtworkLoader { static let shared = ArtworkLoader() - private var cache: [String: Artwork] = [:] - private var pending: Set = [] - private var loadQueue: [String] = [] - private var isProcessing = false + @ObservationIgnored private var cache: [String: Artwork] = [:] + @ObservationIgnored private var pending: Set = [] + @ObservationIgnored private var loadQueue: [String] = [] + @ObservationIgnored private var isProcessing = false + + /// Triggers view updates when artwork is loaded + private(set) var lastUpdateTimestamp = Date() private init() {} @@ -62,7 +65,7 @@ final class ArtworkLoader: ObservableObject { pending.remove(song.id.rawValue) } // Trigger UI update - objectWillChange.send() + lastUpdateTimestamp = Date() } catch { // Remove from pending on error so they can retry for id in songIds { diff --git a/Shfl/ShuffledApp.swift b/Shfl/ShuffledApp.swift index d6d510d..9f37964 100644 --- a/Shfl/ShuffledApp.swift +++ b/Shfl/ShuffledApp.swift @@ -12,10 +12,6 @@ import SwiftData struct ShuffledApp: App { @State private var motionManager = MotionManager() - init() { - VolumeController.initialize() - } - var sharedModelContainer: ModelContainer = { let schema = Schema([PersistedSong.self]) let modelConfiguration = ModelConfiguration(schema: schema, isStoredInMemoryOnly: false) diff --git a/Shfl/Utilities/AlbumArtColorExtractor.swift b/Shfl/Utilities/AlbumArtColorExtractor.swift index b4f0fb1..17eeda7 100644 --- a/Shfl/Utilities/AlbumArtColorExtractor.swift +++ b/Shfl/Utilities/AlbumArtColorExtractor.swift @@ -1,16 +1,16 @@ -import Combine import MusicKit import SwiftUI import UIKit /// Extracts colors from album artwork using MusicKit's catalog data +@Observable @MainActor -final class AlbumArtColorExtractor: ObservableObject { - @Published private(set) var extractedColor: Color? +final class AlbumArtColorExtractor { + private(set) var extractedColor: Color? - private var currentSongId: String? - private var currentTask: Task? - private var colorCache: [String: Color] = [:] + @ObservationIgnored private var currentSongId: String? + @ObservationIgnored private var currentTask: Task? + @ObservationIgnored private var colorCache: [String: Color] = [:] /// Updates the extracted color for the given song by fetching from Apple Music catalog func updateColor(for songId: String) { diff --git a/Shfl/Utilities/VolumeController.swift b/Shfl/Utilities/VolumeController.swift index d5fc73e..dbf1c46 100644 --- a/Shfl/Utilities/VolumeController.swift +++ b/Shfl/Utilities/VolumeController.swift @@ -25,18 +25,21 @@ enum VolumeController { volumeView.subviews.compactMap { $0 as? UISlider }.first } - /// Call this early in app lifecycle (e.g., from AppDelegate or initial view) + /// Call this early in app lifecycle (e.g., from a view's onAppear) /// to ensure the volume view is ready before user interaction. + /// Safe to call multiple times - will only initialize once when a window is available. static func initialize() { guard !isInitialized else { return } - isInitialized = true guard let window = UIApplication.shared.connectedScenes .compactMap({ $0 as? UIWindowScene }) .first?.windows.first else { - assertionFailure("VolumeController: No window available during initialization") + // Window not available yet - this is expected if called too early. + // Volume control will attempt to initialize lazily when first used. return } + + isInitialized = true window.addSubview(volumeView) // Force layout so subviews are populated volumeView.layoutIfNeeded() @@ -51,8 +54,13 @@ enum VolumeController { } private static func adjustVolume(by delta: Float) { + // Attempt lazy initialization if not done yet + if !isInitialized { + initialize() + } + guard let slider = volumeSlider else { - assertionFailure("VolumeController: Slider unavailable. Ensure initialize() is called on app launch.") + // Slider may be unavailable if window isn't ready or MPVolumeView structure changed return } let newValue = max(0.0, min(slider.value + delta, 1.0)) diff --git a/Shfl/ViewModels/AppViewModel.swift b/Shfl/ViewModels/AppViewModel.swift index 612f462..f423af4 100644 --- a/Shfl/ViewModels/AppViewModel.swift +++ b/Shfl/ViewModels/AppViewModel.swift @@ -1,23 +1,22 @@ -import Combine import SwiftData import SwiftUI +@Observable @MainActor -final class AppViewModel: ObservableObject { +final class AppViewModel { let player: ShufflePlayer - let musicService: MusicService - private let repository: SongRepository - private let scrobbleTracker: ScrobbleTracker - private var cancellables = Set() - - @Published var isAuthorized = false - @Published var isLoading = true - @Published var loadingMessage = "Loading..." - @Published var showingManage = false - @Published var showingPicker = false - @Published var showingPickerDirect = false - @Published var showingSettings = false - @Published var authorizationError: String? + @ObservationIgnored let musicService: MusicService + @ObservationIgnored private let repository: SongRepository + @ObservationIgnored private let scrobbleTracker: ScrobbleTracker + + var isAuthorized = false + var isLoading = true + var loadingMessage = "Loading..." + var showingManage = false + var showingPicker = false + var showingPickerDirect = false + var showingSettings = false + var authorizationError: String? init(musicService: MusicService, modelContext: ModelContext) { self.musicService = musicService @@ -32,14 +31,38 @@ final class AppViewModel: ObservableObject { let scrobbleManager = ScrobbleManager(transports: [lastFMTransport]) self.scrobbleTracker = ScrobbleTracker(scrobbleManager: scrobbleManager, musicService: musicService) - // Forward playback state to scrobble tracker - player.$playbackState - .sink { [weak self] state in - self?.scrobbleTracker.onPlaybackStateChanged(state) + // Start observing playback state for scrobbling + startObservingPlaybackState() + } + + @ObservationIgnored private var scrobbleObservationTask: Task? + + deinit { + scrobbleObservationTask?.cancel() + } + + private func startObservingPlaybackState() { + // Track last state to avoid duplicate notifications + var lastState: PlaybackState? + + scrobbleObservationTask = Task { @MainActor [weak self] in + while !Task.isCancelled { + guard let self else { return } + + // Check if state changed + let currentState = self.player.playbackState + if currentState != lastState { + lastState = currentState + self.scrobbleTracker.onPlaybackStateChanged(currentState) + } + + // Poll at reasonable interval - scrobbling doesn't need instant updates + try? await Task.sleep(for: .milliseconds(250)) } - .store(in: &cancellables) + } } + func onAppear() async { // Check authorization in parallel with song loading loadingMessage = "Loading your music..." diff --git a/Shfl/ViewModels/LibraryBrowserViewModel.swift b/Shfl/ViewModels/LibraryBrowserViewModel.swift index 313336d..069e2c8 100644 --- a/Shfl/ViewModels/LibraryBrowserViewModel.swift +++ b/Shfl/ViewModels/LibraryBrowserViewModel.swift @@ -1,8 +1,8 @@ -import Combine import Foundation +@Observable @MainActor -final class LibraryBrowserViewModel: ObservableObject { +final class LibraryBrowserViewModel { enum Mode: Equatable { case browse case search @@ -16,22 +16,27 @@ final class LibraryBrowserViewModel: ObservableObject { } // Autofill state - @Published private(set) var autofillState: AutofillState = .idle + private(set) var autofillState: AutofillState = .idle // Browse state - @Published private(set) var browseSongs: [Song] = [] - @Published private(set) var browseLoading = true // Start true to show skeleton - @Published private(set) var hasMorePages = true - @Published private(set) var hasLoadedOnce = false - @Published var sortOption: SortOption + private(set) var browseSongs: [Song] = [] + private(set) var browseLoading = true // Start true to show skeleton + private(set) var hasMorePages = true + private(set) var hasLoadedOnce = false + var sortOption: SortOption // Search state - @Published private(set) var searchResults: [Song] = [] - @Published private(set) var searchLoading = false + private(set) var searchResults: [Song] = [] + private(set) var searchLoading = false // Shared state - @Published var searchText = "" - @Published private(set) var errorMessage: String? + var searchText = "" { + didSet { + guard searchText != oldValue else { return } + handleSearchTextChanged() + } + } + private(set) var errorMessage: String? var currentMode: Mode { searchText.isEmpty ? .browse : .search @@ -46,15 +51,14 @@ final class LibraryBrowserViewModel: ObservableObject { } // Pagination - private let pageSize = 50 - private var currentOffset = 0 - private var isLoadingMore = false + @ObservationIgnored private let pageSize = 50 + @ObservationIgnored private var currentOffset = 0 + @ObservationIgnored private var isLoadingMore = false // Dependencies - private let musicService: MusicService - private var searchCancellable: AnyCancellable? - private var searchTask: Task? - private var sortingChangedObserver: NSObjectProtocol? + @ObservationIgnored private let musicService: MusicService + @ObservationIgnored private var searchTask: Task? + @ObservationIgnored private var debounceTask: Task? init(musicService: MusicService) { self.musicService = musicService @@ -62,27 +66,10 @@ final class LibraryBrowserViewModel: ObservableObject { // Read sort option from UserDefaults let savedRaw = UserDefaults.standard.string(forKey: "librarySortOption") ?? SortOption.mostPlayed.rawValue self.sortOption = SortOption(rawValue: savedRaw) ?? .mostPlayed - - setupSearchSubscription() - setupSortingObserver() } - private func setupSortingObserver() { - sortingChangedObserver = NotificationCenter.default.addObserver( - forName: Notification.Name("librarySortingChanged"), - object: nil, - queue: .main - ) { [weak self] _ in - Task { @MainActor in - self?.handleSortingChanged() - } - } - } - - private func handleSortingChanged() { - let savedRaw = UserDefaults.standard.string(forKey: "librarySortOption") ?? SortOption.mostPlayed.rawValue - let newOption = SortOption(rawValue: savedRaw) ?? .mostPlayed - + /// Called when sort option changes. Views should call this via onChange(of: appSettings.librarySortOption). + func handleSortOptionChanged(_ newOption: SortOption) { guard newOption != sortOption else { return } sortOption = newOption @@ -96,33 +83,31 @@ final class LibraryBrowserViewModel: ObservableObject { } } - deinit { - if let observer = sortingChangedObserver { - NotificationCenter.default.removeObserver(observer) + private func handleSearchTextChanged() { + // Cancel previous debounce + debounceTask?.cancel() + + let query = searchText + + // Clear results immediately when search is cleared + if query.isEmpty { + searchResults = [] + searchTask?.cancel() + return } - } - private func setupSearchSubscription() { - searchCancellable = $searchText - .debounce(for: .milliseconds(300), scheduler: DispatchQueue.main) - .removeDuplicates() - .sink { [weak self] query in - print("🔎 Debounced search triggered for: '\(query)'") - guard let self else { return } - - // Clear results immediately when search is cleared - if query.isEmpty { - Task { @MainActor in - self.searchResults = [] - } - return - } + // Debounce search + debounceTask = Task { + try? await Task.sleep(nanoseconds: 300_000_000) // 300ms + guard !Task.isCancelled else { return } - self.searchTask?.cancel() - self.searchTask = Task { - await self.performSearch(query: query) - } + print("🔎 Debounced search triggered for: '\(query)'") + + searchTask?.cancel() + searchTask = Task { + await performSearch(query: query) } + } } // MARK: - Browse Methods diff --git a/Shfl/ViewModels/SongUndoManager.swift b/Shfl/ViewModels/SongUndoManager.swift index b3628d2..388041c 100644 --- a/Shfl/ViewModels/SongUndoManager.swift +++ b/Shfl/ViewModels/SongUndoManager.swift @@ -1,9 +1,9 @@ -import Combine import SwiftUI +@Observable @MainActor -final class SongUndoManager: ObservableObject { - @Published private(set) var currentState: UndoState? +final class SongUndoManager { + private(set) var currentState: UndoState? private var dismissTask: Task? func recordAction(_ action: UndoAction, song: Song, autoHideDelay: TimeInterval = 3.0) { diff --git a/Shfl/Views/Components/SongDisplay.swift b/Shfl/Views/Components/SongDisplay.swift index cafe42c..28ec071 100644 --- a/Shfl/Views/Components/SongDisplay.swift +++ b/Shfl/Views/Components/SongDisplay.swift @@ -25,7 +25,7 @@ struct SongDisplay: View { struct SongArtwork: View { let songId: String - @ObservedObject private var loader = ArtworkLoader.shared + private var loader: ArtworkLoader { ArtworkLoader.shared } var body: some View { RoundedRectangle(cornerRadius: 4) @@ -43,6 +43,8 @@ struct SongArtwork: View { .onAppear { loader.requestArtwork(for: songId) } + // Force view updates when artwork is loaded + .id(loader.lastUpdateTimestamp) } } diff --git a/Shfl/Views/MainView.swift b/Shfl/Views/MainView.swift index ead17ba..8148360 100644 --- a/Shfl/Views/MainView.swift +++ b/Shfl/Views/MainView.swift @@ -2,16 +2,19 @@ import SwiftUI import SwiftData struct MainView: View { - @StateObject private var viewModel: AppViewModel + @State private var viewModel: AppViewModel + @State private var appSettings = AppSettings() init(musicService: MusicService, modelContext: ModelContext) { - _viewModel = StateObject(wrappedValue: AppViewModel( + _viewModel = State(wrappedValue: AppViewModel( musicService: musicService, modelContext: modelContext )) } var body: some View { + @Bindable var viewModel = viewModel + Group { if viewModel.isLoading { loadingView @@ -27,21 +30,32 @@ struct MainView: View { authorizationView } } + .environment(\.appSettings, appSettings) + .onAppear { + VolumeController.initialize() + } .task { await viewModel.onAppear() } + .onChange(of: appSettings.shuffleAlgorithm) { _, newAlgorithm in + Task { + await viewModel.player.reshuffleWithNewAlgorithm(newAlgorithm) + } + } .sheet(isPresented: $viewModel.showingManage) { ManageView( player: viewModel.player, onAddTapped: { viewModel.openPicker() }, onDismiss: { viewModel.closeManage() } ) + .environment(\.appSettings, appSettings) .sheet(isPresented: $viewModel.showingPicker, onDismiss: { viewModel.closePicker() }) { SongPickerView( player: viewModel.player, musicService: viewModel.musicService, onDismiss: { viewModel.closePicker() } ) + .environment(\.appSettings, appSettings) } } .sheet(isPresented: $viewModel.showingPickerDirect, onDismiss: { viewModel.closePickerDirect() }) { @@ -50,10 +64,12 @@ struct MainView: View { musicService: viewModel.musicService, onDismiss: { viewModel.closePickerDirect() } ) + .environment(\.appSettings, appSettings) } .sheet(isPresented: $viewModel.showingSettings) { SettingsView() - .environmentObject(viewModel.player) + .environment(\.appSettings, appSettings) + .environment(\.shufflePlayer, viewModel.player) } .alert("Authorization Required", isPresented: .init( get: { viewModel.authorizationError != nil }, diff --git a/Shfl/Views/ManageView.swift b/Shfl/Views/ManageView.swift index f331701..ed94bd0 100644 --- a/Shfl/Views/ManageView.swift +++ b/Shfl/Views/ManageView.swift @@ -1,7 +1,7 @@ import SwiftUI struct ManageView: View { - @ObservedObject var player: ShufflePlayer + var player: ShufflePlayer let onAddTapped: () -> Void let onDismiss: () -> Void diff --git a/Shfl/Views/PlayerView.swift b/Shfl/Views/PlayerView.swift index dc8e813..5e99c9d 100644 --- a/Shfl/Views/PlayerView.swift +++ b/Shfl/Views/PlayerView.swift @@ -1,14 +1,14 @@ import SwiftUI struct PlayerView: View { - @ObservedObject var player: ShufflePlayer + var player: ShufflePlayer let musicService: MusicService let onManageTapped: () -> Void let onAddTapped: () -> Void let onSettingsTapped: () -> Void @Environment(\.motionManager) private var motionManager - @StateObject private var colorExtractor = AlbumArtColorExtractor() + @State private var colorExtractor = AlbumArtColorExtractor() @State private var highlightOffset: CGPoint = .zero @State private var showError = false @State private var errorMessage = "" diff --git a/Shfl/Views/Settings/DebugQueueView.swift b/Shfl/Views/Settings/DebugQueueView.swift index 2e6b2b4..14e9668 100644 --- a/Shfl/Views/Settings/DebugQueueView.swift +++ b/Shfl/Views/Settings/DebugQueueView.swift @@ -1,11 +1,11 @@ import SwiftUI struct DebugQueueView: View { - @EnvironmentObject private var player: ShufflePlayer - @AppStorage("shuffleAlgorithm") private var algorithmRaw: String = ShuffleAlgorithm.noRepeat.rawValue + @Environment(\.shufflePlayer) private var player + @Environment(\.appSettings) private var appSettings private var algorithm: ShuffleAlgorithm { - ShuffleAlgorithm(rawValue: algorithmRaw) ?? .noRepeat + appSettings?.shuffleAlgorithm ?? .noRepeat } var body: some View { @@ -18,60 +18,67 @@ struct DebugQueueView: View { .foregroundStyle(.secondary) } - HStack { - Text("Algorithm Used") - Spacer() - Text(player.lastUsedAlgorithm.displayName) - .foregroundStyle(player.lastUsedAlgorithm == algorithm ? Color.secondary : Color.red) - } + if let player { + HStack { + Text("Algorithm Used") + Spacer() + Text(player.lastUsedAlgorithm.displayName) + .foregroundStyle(player.lastUsedAlgorithm == algorithm ? Color.secondary : Color.red) + } - HStack { - Text("Queue Size") - Spacer() - Text("\(player.lastShuffledQueue.count) songs") - .foregroundStyle(.secondary) + HStack { + Text("Queue Size") + Spacer() + Text("\(player.lastShuffledQueue.count) songs") + .foregroundStyle(.secondary) + } } } footer: { - if player.lastUsedAlgorithm != algorithm { + if let player, player.lastUsedAlgorithm != algorithm { Text("⚠️ Press play again to apply the new algorithm") } } Section("Shuffled Queue Order") { - if player.lastShuffledQueue.isEmpty { - Text("No queue yet. Add songs and press play.") - .foregroundStyle(.secondary) - } else { - ForEach(Array(player.lastShuffledQueue.enumerated()), id: \.element.id) { index, song in - HStack(alignment: .top) { - Text("\(index + 1)") - .font(.caption) - .foregroundStyle(.secondary) - .frame(width: 24, alignment: .trailing) - - VStack(alignment: .leading) { - Text(song.title) - .lineLimit(1) - Text(song.artist) + if let player { + if player.lastShuffledQueue.isEmpty { + Text("No queue yet. Add songs and press play.") + .foregroundStyle(.secondary) + } else { + ForEach(Array(player.lastShuffledQueue.enumerated()), id: \.element.id) { index, song in + HStack(alignment: .top) { + Text("\(index + 1)") .font(.caption) .foregroundStyle(.secondary) - .lineLimit(1) + .frame(width: 24, alignment: .trailing) - if algorithm == .weightedByPlayCount || algorithm == .weightedByRecency { - HStack(spacing: 8) { - Text("Plays: \(song.playCount)") - if let date = song.lastPlayedDate { - Text("Last: \(date, style: .relative)") - } else { - Text("Never played") + VStack(alignment: .leading) { + Text(song.title) + .lineLimit(1) + Text(song.artist) + .font(.caption) + .foregroundStyle(.secondary) + .lineLimit(1) + + if algorithm == .weightedByPlayCount || algorithm == .weightedByRecency { + HStack(spacing: 8) { + Text("Plays: \(song.playCount)") + if let date = song.lastPlayedDate { + Text("Last: \(date, style: .relative)") + } else { + Text("Never played") + } } + .font(.caption2) + .foregroundStyle(.tertiary) } - .font(.caption2) - .foregroundStyle(.tertiary) } } } } + } else { + Text("Player not available") + .foregroundStyle(.secondary) } } } @@ -82,6 +89,7 @@ struct DebugQueueView: View { #Preview { NavigationStack { DebugQueueView() - .environmentObject(ShufflePlayer(musicService: MockMusicService())) + .environment(\.shufflePlayer, ShufflePlayer(musicService: MockMusicService())) + .environment(\.appSettings, AppSettings()) } } diff --git a/Shfl/Views/Settings/LibrarySortingSettingsView.swift b/Shfl/Views/Settings/LibrarySortingSettingsView.swift index 26939d2..db3d0c2 100644 --- a/Shfl/Views/Settings/LibrarySortingSettingsView.swift +++ b/Shfl/Views/Settings/LibrarySortingSettingsView.swift @@ -1,30 +1,20 @@ import SwiftUI -extension Notification.Name { - static let librarySortingChanged = Notification.Name("librarySortingChanged") -} - struct LibrarySortingSettingsView: View { - @AppStorage("librarySortOption") private var sortOptionRaw: String = SortOption.mostPlayed.rawValue - - private var sortOption: SortOption { - SortOption(rawValue: sortOptionRaw) ?? .mostPlayed - } + @Environment(\.appSettings) private var appSettings var body: some View { Form { Section { ForEach(SortOption.allCases, id: \.self) { option in Button { - guard sortOptionRaw != option.rawValue else { return } - sortOptionRaw = option.rawValue - NotificationCenter.default.post(name: .librarySortingChanged, object: nil) + appSettings?.librarySortOption = option } label: { HStack { Text(option.displayName) .foregroundStyle(.primary) Spacer() - if sortOption == option { + if appSettings?.librarySortOption == option { Image(systemName: "checkmark") .foregroundStyle(Color.accentColor) } @@ -41,4 +31,5 @@ struct LibrarySortingSettingsView: View { NavigationStack { LibrarySortingSettingsView() } + .environment(\.appSettings, AppSettings()) } diff --git a/Shfl/Views/Settings/ShuffleAlgorithmSettingsView.swift b/Shfl/Views/Settings/ShuffleAlgorithmSettingsView.swift index 837bc5a..d190046 100644 --- a/Shfl/Views/Settings/ShuffleAlgorithmSettingsView.swift +++ b/Shfl/Views/Settings/ShuffleAlgorithmSettingsView.swift @@ -1,30 +1,20 @@ import SwiftUI -extension Notification.Name { - static let shuffleAlgorithmChanged = Notification.Name("shuffleAlgorithmChanged") -} - struct ShuffleAlgorithmSettingsView: View { - @AppStorage("shuffleAlgorithm") private var algorithmRaw: String = ShuffleAlgorithm.noRepeat.rawValue - - private var algorithm: ShuffleAlgorithm { - ShuffleAlgorithm(rawValue: algorithmRaw) ?? .noRepeat - } + @Environment(\.appSettings) private var appSettings var body: some View { Form { Section { ForEach(ShuffleAlgorithm.allCases, id: \.self) { algo in Button { - guard algorithmRaw != algo.rawValue else { return } - algorithmRaw = algo.rawValue - NotificationCenter.default.post(name: .shuffleAlgorithmChanged, object: nil) + appSettings?.shuffleAlgorithm = algo } label: { HStack { Text(algo.displayName) .foregroundStyle(.primary) Spacer() - if algorithm == algo { + if appSettings?.shuffleAlgorithm == algo { Image(systemName: "checkmark") .foregroundStyle(Color.accentColor) } @@ -32,7 +22,9 @@ struct ShuffleAlgorithmSettingsView: View { } } } footer: { - Text(algorithm.description) + if let algorithm = appSettings?.shuffleAlgorithm { + Text(algorithm.description) + } } } .navigationTitle("Shuffle Algorithm") @@ -43,4 +35,5 @@ struct ShuffleAlgorithmSettingsView: View { NavigationStack { ShuffleAlgorithmSettingsView() } + .environment(\.appSettings, AppSettings()) } diff --git a/Shfl/Views/SongPickerView.swift b/Shfl/Views/SongPickerView.swift index 3b0cef4..7c8f364 100644 --- a/Shfl/Views/SongPickerView.swift +++ b/Shfl/Views/SongPickerView.swift @@ -1,12 +1,14 @@ import SwiftUI struct SongPickerView: View { - @ObservedObject var player: ShufflePlayer + var player: ShufflePlayer let musicService: MusicService let onDismiss: () -> Void - @StateObject private var viewModel: LibraryBrowserViewModel - @StateObject private var undoManager = SongUndoManager() + @State private var viewModel: LibraryBrowserViewModel + @State private var undoManager = SongUndoManager() + + @Environment(\.appSettings) private var appSettings init( player: ShufflePlayer, @@ -16,7 +18,7 @@ struct SongPickerView: View { self.player = player self.musicService = musicService self.onDismiss = onDismiss - self._viewModel = StateObject(wrappedValue: LibraryBrowserViewModel(musicService: musicService)) + self._viewModel = State(wrappedValue: LibraryBrowserViewModel(musicService: musicService)) } var body: some View { @@ -89,10 +91,15 @@ struct SongPickerView: View { .disabled(player.songCount == 0) } } - .searchable(text: $viewModel.searchText, prompt: "Search your library") + .searchable(text: Bindable(viewModel).searchText, prompt: "Search your library") .task { await viewModel.loadInitialPage() } + .onChange(of: appSettings?.librarySortOption) { _, newOption in + if let newOption { + viewModel.handleSortOptionChanged(newOption) + } + } .alert("Error", isPresented: .init( get: { viewModel.errorMessage != nil }, set: { if !$0 { viewModel.clearError() } }