diff --git a/lib/mixins/tab_navigation_mixin.dart b/lib/mixins/tab_navigation_mixin.dart index 69345c81..7359ba8e 100644 --- a/lib/mixins/tab_navigation_mixin.dart +++ b/lib/mixins/tab_navigation_mixin.dart @@ -29,22 +29,25 @@ mixin TabNavigationMixin on State, TickerProviderSt /// Number of tabs — derived from [tabChipFocusNodes]. int get tabCount => tabChipFocusNodes.length; - /// Initialise the [TabController] and register gamepad callbacks. + /// Initialise the [TabController] and register owner-scoped gamepad callbacks. /// Call from [initState]. void initTabNavigation() { tabController = TabController(length: tabCount, vsync: this); tabController.addListener(onTabChanged); - GamepadService.onL1Pressed = goToPreviousTab; - GamepadService.onR1Pressed = goToNextTab; + GamepadService.registerTabNavigation( + this, + previous: goToPreviousTab, + next: goToNextTab, + isActive: () => mounted && TickerMode.getValuesNotifier(context).value.enabled, + ); } - /// Dispose the [TabController] and clear gamepad callbacks. + /// Dispose the [TabController] and remove only this screen's callbacks. /// Call from [dispose]. void disposeTabNavigation() { + GamepadService.unregisterTabNavigation(this); tabController.removeListener(onTabChanged); tabController.dispose(); - GamepadService.onL1Pressed = null; - GamepadService.onR1Pressed = null; } void goToPreviousTab() { diff --git a/lib/screens/main_screen.dart b/lib/screens/main_screen.dart index 31bf47cb..1b9409bc 100644 --- a/lib/screens/main_screen.dart +++ b/lib/screens/main_screen.dart @@ -681,6 +681,7 @@ class _MainScreenState extends State appLogger.d('System shelf tap: $contentId'); _handleShelfContentId(contentId); }; + _systemShelfTapCallback = systemShelf.onShelfItemTap; // Check for pending deep link from cold start WidgetsBinding.instance.addPostFrameCallback((_) async { @@ -738,6 +739,7 @@ class _MainScreenState extends State } bool _companionRemoteSetup = false; + ValueChanged? _systemShelfTapCallback; @override void didChangeDependencies() { @@ -803,7 +805,7 @@ class _MainScreenState extends State }; final receiver = CompanionRemoteReceiver.instance; - + receiver.navigationOwner = this; receiver.onTabNext = () { final tabs = _getVisibleTabs(_isOffline); final idx = tabs.indexWhere((t) => t.id == _currentTab); @@ -877,18 +879,27 @@ class _MainScreenState extends State _contentFocusScope.dispose(); _setTvosMenuPassthrough(false); - // Clean up companion remote callbacks + // Clean up only callbacks still owned by this screen. A replacement + // MainScreen may already have installed its callbacks this frame. if (_companionRemoteSetup) { final receiver = CompanionRemoteReceiver.instance; - receiver.onTabNext = null; - receiver.onTabPrevious = null; - receiver.onTabDiscover = null; - receiver.onTabLibraries = null; - receiver.onTabSearch = null; - receiver.onTabDownloads = null; - receiver.onTabSettings = null; - receiver.onHome = null; - receiver.onSearchAction = null; + if (identical(receiver.navigationOwner, this)) { + receiver.onTabNext = null; + receiver.onTabPrevious = null; + receiver.onTabDiscover = null; + receiver.onTabLibraries = null; + receiver.onTabSearch = null; + receiver.onTabDownloads = null; + receiver.onTabSettings = null; + receiver.onHome = null; + receiver.onSearchAction = null; + receiver.navigationOwner = null; + } + } + final shelfCallback = _systemShelfTapCallback; + final systemShelf = SystemShelfService(); + if (shelfCallback != null && identical(systemShelf.onShelfItemTap, shelfCallback)) { + systemShelf.onShelfItemTap = null; } super.dispose(); diff --git a/lib/screens/video_player/parts/companion_remote.dart b/lib/screens/video_player/parts/companion_remote.dart index 762b7e7f..657d3229 100644 --- a/lib/screens/video_player/parts/companion_remote.dart +++ b/lib/screens/video_player/parts/companion_remote.dart @@ -3,6 +3,8 @@ part of '../../video_player_screen.dart'; extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState { void _setupCompanionRemoteCallbacks() { final receiver = CompanionRemoteReceiver.instance; + receiver.playerHomeFallback ??= receiver.onHome; + receiver.playerOwner = this; receiver.onStop = () { if (mounted) _handleBackButton(); }; @@ -47,8 +49,9 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState { receiver.onAudioTracks = _cycleAudioTrack; receiver.onFullscreen = _toggleFullscreen; - // Override home to exit the player first (main screen handler runs after pop) - _savedOnHome = receiver.onHome; + // Override home to exit the player first. Replacements inherit the base + // MainScreen callback rather than chaining through the outgoing player. + _savedOnHome = receiver.playerHomeFallback; receiver.onHome = () { if (mounted) _handleHomeButton(); }; @@ -64,6 +67,10 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState { void _cleanupCompanionRemoteCallbacks() { final receiver = CompanionRemoteReceiver.instance; + if (!identical(receiver.playerOwner, this)) { + _companionRemoteProvider = null; + return; + } receiver.onStop = null; receiver.onNextTrack = null; receiver.onPreviousTrack = null; @@ -75,10 +82,12 @@ extension _VideoPlayerCompanionRemoteMethods on VideoPlayerScreenState { receiver.onSubtitles = null; receiver.onAudioTracks = null; receiver.onFullscreen = null; - receiver.onHome = _savedOnHome; + receiver.onHome = receiver.playerHomeFallback; + receiver.playerHomeFallback = null; + receiver.playerOwner = null; _savedOnHome = null; - // Notify remote that player is no longer active + // Notify only when the active player owner exits. _companionRemoteProvider?.sendCommand(RemoteCommandType.syncState, data: {'playerActive': false}); _companionRemoteProvider = null; } diff --git a/lib/screens/video_player_screen.dart b/lib/screens/video_player_screen.dart index 72dac1fd..a357eb6a 100644 --- a/lib/screens/video_player_screen.dart +++ b/lib/screens/video_player_screen.dart @@ -458,6 +458,7 @@ class VideoPlayerScreenState extends State with WidgetsBindin VideoPIPManager? _videoPIPManager; ShaderService? _shaderService; AmbientLightingService? _ambientLightingService; + bool _fullscreenListenerAttached = false; Size? _lastVideoLayoutSize; Size? _pendingVideoLayoutSize; Player? _lastVideoLayoutPlayer; @@ -760,7 +761,10 @@ class VideoPlayerScreenState extends State with WidgetsBindin _displayModeService = DisplayModeService(settingsService, FullscreenStateManager()); await _displayModeService!.syncWithNative(); if (!mounted) return; - FullscreenStateManager().addListener(_onFullscreenChanged); + if (!_fullscreenListenerAttached) { + FullscreenStateManager().addListener(_onFullscreenChanged); + _fullscreenListenerAttached = true; + } } // One-native-instance rule: a live music session owns the only audio @@ -1297,8 +1301,9 @@ class VideoPlayerScreenState extends State with WidgetsBindin TraktScrobbleService.instance.stopPlayback(); TrackerCoordinator.instance.stopPlayback(); - if (Platform.isWindows && _displayModeService != null) { + if (_fullscreenListenerAttached) { FullscreenStateManager().removeListener(_onFullscreenChanged); + _fullscreenListenerAttached = false; } if (!isReplacingWithVideo && Platform.isWindows && diff --git a/lib/services/companion_remote/companion_remote_receiver.dart b/lib/services/companion_remote/companion_remote_receiver.dart index 1220dd89..3006eef9 100644 --- a/lib/services/companion_remote/companion_remote_receiver.dart +++ b/lib/services/companion_remote/companion_remote_receiver.dart @@ -19,6 +19,12 @@ class CompanionRemoteReceiver { /// Same pattern as [GamepadService.onGamepadInput]. static VoidCallback? onRemoteInput; + /// Owners prevent a disposed screen from clearing callbacks installed by a + /// replacement screen later in the same frame. + Object? navigationOwner; + Object? playerOwner; + VoidCallback? playerHomeFallback; + VoidCallback? onTabNext; VoidCallback? onTabPrevious; VoidCallback? onTabDiscover; diff --git a/lib/services/gamepad_service.dart b/lib/services/gamepad_service.dart index 362556e2..d549f0ab 100644 --- a/lib/services/gamepad_service.dart +++ b/lib/services/gamepad_service.dart @@ -2,7 +2,6 @@ import 'dart:async'; import 'dart:io'; import 'dart:ui' as ui; -import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:universal_gamepad/universal_gamepad.dart'; @@ -146,13 +145,41 @@ class GamepadService with WindowListener { /// Set by InputModeTracker when it initializes. static VoidCallback? onGamepadInput; - /// Callback for L1 bumper press (previous tab). - /// Screens with tabs can listen to this. - static VoidCallback? onL1Pressed; + static final Map + _tabNavigationHandlers = {}; - /// Callback for R1 bumper press (next tab). - /// Screens with tabs can listen to this. - static VoidCallback? onR1Pressed; + /// Registers owner-scoped bumper navigation. Multiple tab screens can stay + /// mounted; only the handler whose screen is currently visible runs. + static void registerTabNavigation( + Object owner, { + required VoidCallback previous, + required VoidCallback next, + required bool Function() isActive, + }) { + _tabNavigationHandlers[owner] = (previous: previous, next: next, isActive: isActive); + } + + static void unregisterTabNavigation(Object owner) { + _tabNavigationHandlers.remove(owner); + } + + static void _dispatchTabNavigation({required bool previous}) { + for (final handler in _tabNavigationHandlers.values) { + if (!handler.isActive()) continue; + previous ? handler.previous() : handler.next(); + return; + } + } + + @visibleForTesting + static void debugDispatchTabNavigation({required bool previous}) { + _dispatchTabNavigation(previous: previous); + } + + @visibleForTesting + static void debugClearTabNavigationHandlers() { + _tabNavigationHandlers.clear(); + } // Deadzone for analog sticks (0.0 to 1.0) static const double _stickDeadzone = 0.5; @@ -408,9 +435,9 @@ class GamepadService with WindowListener { _logGamepadDiag('button simulates key press back ${_describeGamepadButton(event)}'); _simulateKeyPress(LogicalKeyboardKey.gameButtonB); case GamepadButton.leftShoulder: - onL1Pressed?.call(); + _dispatchTabNavigation(previous: true); case GamepadButton.rightShoulder: - onR1Pressed?.call(); + _dispatchTabNavigation(previous: false); default: break; } @@ -458,11 +485,11 @@ class GamepadService with WindowListener { return; } - // Switch to keyboard mode on significant axis input + // Switch to keyboard mode on significant axis input. Navigation itself + // schedules frames only when the stick crosses the real deadzone. if (event.value.abs() > 0.3) { onGamepadInput?.call(); _setTraditionalFocusHighlight(); - SchedulerBinding.instance.ensureVisualUpdate(); } switch (event.axis) { diff --git a/test/mixins/tab_navigation_mixin_test.dart b/test/mixins/tab_navigation_mixin_test.dart index 8dec6e42..3b4d693e 100644 --- a/test/mixins/tab_navigation_mixin_test.dart +++ b/test/mixins/tab_navigation_mixin_test.dart @@ -8,7 +8,7 @@ import 'package:plezy/services/gamepad_service.dart'; /// Tests stage [tabCount] focus nodes and read the resulting controller state /// after [initTabNavigation] runs. class _Probe extends StatefulWidget { - const _Probe({required this.tabCount, required this.onState, this.initialIndex = 0}); + const _Probe({super.key, required this.tabCount, required this.onState, this.initialIndex = 0}); final int tabCount; final int initialIndex; @@ -60,16 +60,9 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); group('TabNavigationMixin', () { - setUp(() { - // Static gamepad callbacks are global state; isolate each test. - GamepadService.onL1Pressed = null; - GamepadService.onR1Pressed = null; - }); + setUp(GamepadService.debugClearTabNavigationHandlers); - tearDown(() { - GamepadService.onL1Pressed = null; - GamepadService.onR1Pressed = null; - }); + tearDown(GamepadService.debugClearTabNavigationHandlers); testWidgets('initTabNavigation creates a TabController with the right length', (tester) async { late _ProbeState state; @@ -83,30 +76,45 @@ void main() { expect(state.suppressAutoFocus, isFalse); }); - testWidgets('initTabNavigation registers L1/R1 gamepad callbacks', (tester) async { + testWidgets('initTabNavigation registers owner-scoped bumper navigation', (tester) async { late _ProbeState state; await tester.pumpWidget(_Probe(tabCount: 3, onState: (s) => state = s)); - // Mixin wires its private goToPreviousTab/goToNextTab to the static - // callbacks; we can only assert non-null wiring (the closures are the - // mixin's bound methods, not directly comparable). - expect(GamepadService.onL1Pressed, isNotNull); - expect(GamepadService.onR1Pressed, isNotNull); - // Sanity: invoking R1 advances the tab via the mixin's goToNextTab. - GamepadService.onR1Pressed!.call(); + GamepadService.debugDispatchTabNavigation(previous: false); await tester.pump(); expect(state.tabController.index, 1); }); - testWidgets('disposeTabNavigation clears the static gamepad callbacks', (tester) async { - await tester.pumpWidget(_Probe(tabCount: 2, onState: (_) {})); - expect(GamepadService.onL1Pressed, isNotNull); - expect(GamepadService.onR1Pressed, isNotNull); + testWidgets('disposing one tab screen preserves another owner registration', (tester) async { + late _ProbeState first; + late _ProbeState second; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Column( + children: [ + _Probe(key: const ValueKey('first'), tabCount: 2, onState: (s) => first = s), + _Probe(key: const ValueKey('second'), tabCount: 2, onState: (s) => second = s), + ], + ), + ), + ); - // Replace the widget tree to fire dispose. - await tester.pumpWidget(const SizedBox.shrink()); - expect(GamepadService.onL1Pressed, isNull); - expect(GamepadService.onR1Pressed, isNull); + GamepadService.debugDispatchTabNavigation(previous: false); + await tester.pump(); + expect(first.tabController.index, 1); + expect(second.tabController.index, 0); + + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: _Probe(key: const ValueKey('second'), tabCount: 2, onState: (s) => second = s), + ), + ); + GamepadService.debugDispatchTabNavigation(previous: false); + await tester.pump(); + + expect(second.tabController.index, 1); }); testWidgets('tabChipFocusNodes drives tabCount; getTabChipFocusNode returns the right node', (tester) async { @@ -183,10 +191,11 @@ void main() { expect(state.tabController.length, 3); expect(state.tabController.index, 0); - // The original is disposed; touching it would throw — but the - // mixin's references all point at the new instance now. - expect(GamepadService.onL1Pressed, isNotNull); - expect(GamepadService.onR1Pressed, isNotNull); + // The original is disposed, while the owner-scoped registry points at + // the newly initialized controller. + GamepadService.debugDispatchTabNavigation(previous: false); + await tester.pump(); + expect(state.tabController.index, 1); }); testWidgets('onTabChanged fires when tabController.index changes', (tester) async {