From a1919d48a637339f66f38b12292f26629243ffad Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 14:04:33 -0400 Subject: [PATCH 1/2] chore(sports): drop the dead second copy of the fallback scroll classes hockey, basketball and lacrosse shipped as the pre-adoption file and the adopted file CONCATENATED rather than one replacing the other. Each carried a full copy of the bundled classes at module level that nothing referenced -- the fallback branch imports the real ones from `scroll_display_legacy`. The concatenation also duplicated the GameRenderer import and the logger assignment. hockey 991 -> 334 (-657) basketball 1058 -> 363 (-695) lacrosse 959 -> 321 (-638) -1990 The dead copy was stale as well: it still annotated `Dict[str, ScrollDisplay]` where the live file had been corrected to `Dict[str, 'LegacyScrollDisplay']`. That one line is the only meaningful difference between them, which is what makes removing the inline block safe. Not cosmetic ------------ The separator-icon constants whose absence broke scroll mode on a 3.2.0 core were sitting in this dead block. That is why the file read as correct -- to a reviewer, and to an AST checker that only asked whether the names were defined somewhere in the module. One implementation per file is what makes the next such miss visible rather than camouflaged. `scripts/check_scroll_adoption.py` now fails any plugin whose `scroll_display.py` defines a module-level `Legacy*` class, and runs in CI beside the collision check. It fails on the three files as they were and passes on all ten now. Verification ------------ * 8/8 fallback suites pass; the 5 remaining failures are the known pre-existing stale-test tranche, unchanged. * 168/168 harness renders byte-identical to before the strip. * Scroll strips rendered from identical games are pixel-identical between the core and fallback paths in all 8 plugins, with the same SHAs as before. * Both structural guards pass across all 42 plugins. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .github/workflows/module-collisions.yml | 28 +- plugins.json | 6 +- plugins/basketball-scoreboard/manifest.json | 8 +- .../basketball-scoreboard/scroll_display.py | 695 ------------------ plugins/hockey-scoreboard/manifest.json | 8 +- plugins/hockey-scoreboard/scroll_display.py | 657 ----------------- plugins/lacrosse-scoreboard/manifest.json | 8 +- plugins/lacrosse-scoreboard/scroll_display.py | 638 ---------------- scripts/check_scroll_adoption.py | 81 ++ 9 files changed, 126 insertions(+), 2003 deletions(-) create mode 100644 scripts/check_scroll_adoption.py diff --git a/.github/workflows/module-collisions.yml b/.github/workflows/module-collisions.yml index 74dfa5b6..8ed9a282 100644 --- a/.github/workflows/module-collisions.yml +++ b/.github/workflows/module-collisions.yml @@ -1,17 +1,27 @@ -name: Module Collisions +name: Plugin Structure -# Fails when a plugin's deferred import (subpackage file or function-scoped -# import) targets a sibling top-level module whose name is also shipped by -# another plugin. The core loads plugin modules by bare name on sys.path and -# isolates them after the entry point loads, so such a deferred import can bind -# a different plugin's same-named module and fail to load. Scans ALL plugins -# because a newly added plugin can collide with an existing one. +# Two structural checks that both scan ALL plugins, because either problem can +# arrive with a newly added plugin rather than a changed one. +# +# 1. Module collisions — fails when a plugin's deferred import (subpackage file +# or function-scoped import) targets a sibling top-level module whose name is +# also shipped by another plugin. The core loads plugin modules by bare name +# on sys.path and isolates them after the entry point loads, so such a +# deferred import can bind a different plugin's same-named module and fail +# to load. +# +# 2. Scroll adoption — fails when a plugin's scroll_display.py also defines the +# fallback implementation it is supposed to import from +# scroll_display_legacy.py. Three plugins shipped as those two files +# concatenated, carrying ~2,000 lines nothing referenced, and that dead copy +# is what hid the missing separator-icon constants that broke scroll mode. on: pull_request: paths: - 'plugins/**' - 'scripts/check_module_collisions.py' + - 'scripts/check_scroll_adoption.py' workflow_dispatch: jobs: @@ -24,3 +34,7 @@ jobs: python-version: '3.12' - name: Check for cross-plugin module collisions run: python scripts/check_module_collisions.py + # Runs even when the collision check fails, so one PR surfaces both. + - name: Check scroll adoption does not inline the fallback + if: always() + run: python scripts/check_scroll_adoption.py diff --git a/plugins.json b/plugins.json index c9d216bf..62e9eb53 100644 --- a/plugins.json +++ b/plugins.json @@ -101,7 +101,7 @@ "last_updated": "2026-07-31", "verified": true, "screenshot": "", - "latest_version": "1.10.1" + "latest_version": "1.10.2" }, { "id": "calendar", @@ -335,7 +335,7 @@ "last_updated": "2026-07-31", "verified": true, "screenshot": "", - "latest_version": "1.7.1", + "latest_version": "1.7.2", "icon": "fas fa-hockey-puck" }, { @@ -359,7 +359,7 @@ "last_updated": "2026-07-31", "verified": true, "screenshot": "", - "latest_version": "1.7.1", + "latest_version": "1.7.2", "icon": "fas fa-baseball-ball" }, { diff --git a/plugins/basketball-scoreboard/manifest.json b/plugins/basketball-scoreboard/manifest.json index 054afd05..4b95cd20 100644 --- a/plugins/basketball-scoreboard/manifest.json +++ b/plugins/basketball-scoreboard/manifest.json @@ -1,7 +1,7 @@ { "id": "basketball-scoreboard", "name": "Basketball Scoreboard", - "version": "1.10.1", + "version": "1.10.2", "description": "Live, recent, and upcoming basketball games across NBA, NCAA Men's, NCAA Women's, and WNBA with real-time scores, schedules, and March Madness tournament support", "author": "ChuckBuilds", "category": "sports", @@ -18,6 +18,12 @@ "branch": "main", "plugin_path": "plugins/basketball-scoreboard", "versions": [ + { + "version": "1.10.2", + "released": "2026-08-05", + "notes": "Housekeeping, no behaviour change: scroll_display.py carried a second, unreferenced copy of the bundled fallback classes at module level -- the file was the pre-adoption and adopted versions concatenated rather than one replacing the other. The live fallback in scroll_display_legacy.py is untouched. Removing the dead copy is what makes a missing constant visible instead of appearing defined; that dead block is where the ones that broke scroll mode were hiding.", + "ledmatrix_min_version": "2.0.0" + }, { "version": "1.10.1", "released": "2026-08-05", diff --git a/plugins/basketball-scoreboard/scroll_display.py b/plugins/basketball-scoreboard/scroll_display.py index 14a39bc4..4ab0569d 100644 --- a/plugins/basketball-scoreboard/scroll_display.py +++ b/plugins/basketball-scoreboard/scroll_display.py @@ -36,10 +36,6 @@ except ImportError: ScrollHelper = None -try: - from game_renderer import GameRenderer -except ImportError: - GameRenderer = None logger = logging.getLogger(__name__) @@ -51,697 +47,6 @@ RESAMPLE_FILTER = Image.LANCZOS -class LegacyScrollDisplay: - """ - Handles scroll display mode for the basketball scoreboard plugin. - - This class: - - Collects all games matching criteria (respecting live priority) - - Pre-renders each game using GameRenderer - - Adds league separator icons between different leagues - - Composes a single wide image using ScrollHelper - - Implements dynamic duration based on total content width - - Logs FPS and game count during scrolling - """ - - # Paths to league separator icons - NBA_SEPARATOR_ICON = "assets/sports/nba_logos/NBA.png" - WNBA_SEPARATOR_ICON = "assets/sports/wnba_logos/WNBA.png" - NCAA_SEPARATOR_ICON = "assets/sports/ncaa_logos/NCAA.png" # Generic NCAA logo, or use league-specific if available - MARCH_MADNESS_SEPARATOR_ICON = "assets/sports/ncaa_logos/MARCH_MADNESS.png" - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplay handler. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Get display dimensions - if hasattr(display_manager, 'matrix') and display_manager.matrix is not None: - self.display_width = display_manager.matrix.width - self.display_height = display_manager.matrix.height - else: - self.display_width = getattr(display_manager, "width", 128) - self.display_height = getattr(display_manager, "height", 32) - - # Initialize ScrollHelper - if ScrollHelper: - self.scroll_helper = ScrollHelper( - self.display_width, - self.display_height, - self.logger - ) - # Configure scroll settings - self._configure_scroll_helper() - else: - self.scroll_helper = None - self.logger.error("ScrollHelper not available - scroll mode will not work") - - # Shared logo cache for game renderer - self._logo_cache: Dict[str, Image.Image] = {} - - # League separator icons cache - self._separator_icons: Dict[str, Image.Image] = {} - self._load_separator_icons() - - # Tracking state - self._current_games: List[Dict] = [] - self._current_game_type: str = "" - self._current_leagues: List[str] = [] - self._vegas_content_items: List[Image.Image] = [] - self._is_scrolling = False - self._scroll_start_time: Optional[float] = None - self._last_log_time: float = 0 - self._log_interval: float = 5.0 # Log every 5 seconds - - # Performance tracking - self._frame_count: int = 0 - self._fps_sample_start: float = time.time() - - def _configure_scroll_helper(self) -> None: - """Configure scroll helper with settings from config.""" - if not self.scroll_helper: - return - - # Get global scroll settings, then per-league overrides - # For now, use global settings - scroll_settings = self._get_scroll_settings() - - # Set scroll speed (pixels per second in time-based mode) - scroll_speed = scroll_settings.get("scroll_speed", 50.0) - self.scroll_helper.set_scroll_speed(scroll_speed) - - # Set scroll delay - scroll_delay = scroll_settings.get("scroll_delay", 0.01) - self.scroll_helper.set_scroll_delay(scroll_delay) - - # Enable dynamic duration - dynamic_duration = scroll_settings.get("dynamic_duration", True) - self.scroll_helper.set_dynamic_duration_settings( - enabled=dynamic_duration, - min_duration=30, - max_duration=600, # 10 minutes max - buffer=0.2 # 20% buffer to ensure scroll completes fully off screen - ) - - # Use frame-based scrolling for better FPS control - # In frame-based mode: scroll_speed is pixels per frame, scroll_delay controls frame rate - # This allows precise control: 1 px/frame at 0.01s delay = 100 FPS - self.scroll_helper.set_frame_based_scrolling(True) - - # Convert scroll_speed from pixels/second to pixels/frame for frame-based mode - # If scroll_speed is very low (like 1.0 px/s), treat it as pixels per frame directly - # Otherwise, calculate pixels per frame based on scroll_delay - if scroll_speed < 10.0: - # Low values are likely intended as pixels per frame - pixels_per_frame = scroll_speed - else: - # Higher values are pixels/second, convert to pixels/frame - pixels_per_frame = scroll_speed * scroll_delay - - # Clamp to reasonable range (0.1 to 5 pixels per frame for smooth scrolling) - pixels_per_frame = max(0.1, min(5.0, pixels_per_frame)) - self.scroll_helper.set_scroll_speed(pixels_per_frame) - - # Calculate effective pixels per second for logging - effective_pps = pixels_per_frame / scroll_delay if scroll_delay > 0 else pixels_per_frame * 100 - - self.logger.info( - f"ScrollHelper configured: {pixels_per_frame:.2f} px/frame, delay={scroll_delay}s " - f"(effective {effective_pps:.1f} px/s), dynamic_duration={dynamic_duration}" - ) - - # Honor the global smooth-scrolling FPS target (older cores lack the setter) - target_fps = self.global_config.get('target_fps') or self.global_config.get('scroll_target_fps') - try: - # Coerce before comparing: a malformed global config value - # must degrade to today's scroll_delay pacing, not raise. - target_fps = float(target_fps) if target_fps is not None else None - except (TypeError, ValueError): - target_fps = None - if target_fps: - if hasattr(self.scroll_helper, 'set_target_fps'): - self.scroll_helper.set_target_fps(target_fps) - else: - self.scroll_helper.target_fps = max(30.0, min(200.0, target_fps)) - self.scroll_helper.frame_time_target = 1.0 / self.scroll_helper.target_fps - - def _get_scroll_settings(self, league: str = None) -> Dict[str, Any]: - """Get scroll settings, optionally for a specific league.""" - # Default scroll settings - defaults = { - "scroll_speed": 50.0, - "scroll_delay": 0.01, - "gap_between_games": 48, - "show_league_separators": True, - "dynamic_duration": True, - "game_card_width": 128, - } - - # Try to get league-specific settings first - if league: - league_config = self.config.get(league, {}) - league_scroll = league_config.get("scroll_settings", {}) - if league_scroll: - return {**defaults, **league_scroll} - - # Fall back to NBA settings (usually first enabled) - nba_config = self.config.get("nba", {}) - nba_scroll = nba_config.get("scroll_settings", {}) - if nba_scroll: - return {**defaults, **nba_scroll} - - # Fall back to WNBA settings - wnba_config = self.config.get("wnba", {}) - wnba_scroll = wnba_config.get("scroll_settings", {}) - if wnba_scroll: - return {**defaults, **wnba_scroll} - - # Fall back to NCAA Men's settings - ncaam_config = self.config.get("ncaam", {}) - ncaam_scroll = ncaam_config.get("scroll_settings", {}) - if ncaam_scroll: - return {**defaults, **ncaam_scroll} - - # Fall back to NCAA Women's settings - ncaaw_config = self.config.get("ncaaw", {}) - ncaaw_scroll = ncaaw_config.get("scroll_settings", {}) - if ncaaw_scroll: - return {**defaults, **ncaaw_scroll} - - return defaults - - def _load_separator_icon( - self, - icon_path: str, - league_keys: List[str], - separator_height: int, - display_name: str - ) -> None: - """ - Load and resize a single separator icon. - - Args: - icon_path: Path to the icon file - league_keys: List of league keys to associate with this icon - separator_height: Target height for the icon - display_name: Name for logging purposes - """ - if not os.path.exists(icon_path): - self.logger.warning(f"{display_name} separator icon not found at {icon_path}") - return - - try: - with Image.open(icon_path) as icon: - if icon.mode != "RGBA": - icon = icon.convert("RGBA") - # Resize to fit height while maintaining aspect ratio - aspect = icon.width / icon.height - new_width = int(separator_height * aspect) - resized_icon = icon.resize((new_width, separator_height), resample=RESAMPLE_FILTER) - # Store for each league key - for key in league_keys: - self._separator_icons[key] = resized_icon - self.logger.debug(f"Loaded {display_name} separator icon: {new_width}x{separator_height}") - except Exception: - self.logger.exception(f"Error loading {display_name} separator icon") - - def _load_separator_icons(self) -> None: - """Load and resize league separator icons.""" - separator_height = self.display_height - 4 # Leave some padding - - # Load all separator icons using helper - self._load_separator_icon( - self.NBA_SEPARATOR_ICON, ["nba"], separator_height, "NBA" - ) - self._load_separator_icon( - self.WNBA_SEPARATOR_ICON, ["wnba"], separator_height, "WNBA" - ) - self._load_separator_icon( - self.NCAA_SEPARATOR_ICON, ["ncaam", "ncaaw"], separator_height, "NCAA" - ) - # March Madness tournament separator (used when tournament games are detected) - self._load_separator_icon( - self.MARCH_MADNESS_SEPARATOR_ICON, - ["ncaam_tournament", "ncaaw_tournament"], - separator_height, - "March Madness", - ) - - def _determine_game_type(self, game: Dict) -> str: - """ - Determine the game type from the game's status. - - Args: - game: Game dictionary (flat format from sports.py) - - Returns: - Game type: 'live', 'recent', or 'upcoming' - """ - # Use flat game dict flags from sports.py - if game.get('is_live'): - return 'live' - elif game.get('is_final'): - return 'recent' - elif game.get('is_upcoming'): - return 'upcoming' - else: - # Default to upcoming if state is unknown - return 'upcoming' - - def prepare_scroll_content( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Dict[str, int] = None - ) -> bool: - """ - Prepare scrolling content from a list of games. - - Args: - games: List of game dictionaries with league info - game_type: Type hint ('live', 'recent', 'upcoming', or 'mixed' for mixed types) - leagues: List of leagues in order (e.g., ['nba', 'wnba', 'ncaam']) - rankings_cache: Optional team rankings cache - - Returns: - True if content was prepared successfully, False otherwise - """ - if not self.scroll_helper: - self.logger.error("ScrollHelper not available") - return False - - if not games: - self.logger.debug("No games to prepare for scrolling") - self.scroll_helper.clear_cache() - self._vegas_content_items = [] - return False - - self._current_games = games - self._current_game_type = game_type - self._current_leagues = leagues - - # Get scroll settings - scroll_settings = self._get_scroll_settings() - gap_between_games = scroll_settings.get("gap_between_games", 24) - show_separators = scroll_settings.get("show_league_separators", True) - game_card_width = scroll_settings.get("game_card_width", 128) - - # Verify GameRenderer is available - if GameRenderer is None: - self.logger.error("GameRenderer not available - cannot prepare scroll content") - return False - - # Create game renderer using game_card_width so cards are a fixed size - # regardless of the full chain width (display_width may span multiple panels) - renderer = GameRenderer( - game_card_width, - self.display_height, - self.config, - logo_cache=self._logo_cache, - custom_logger=self.logger - ) - if rankings_cache: - renderer.set_rankings_cache(rankings_cache) - - # Pre-render all game cards - content_items: List[Image.Image] = [] - current_league = None - game_count = 0 - league_counts: Dict[str, int] = {} - - for game in games: - game_league = game.get("league", "nba") # Default to NBA if not specified - - # Use March Madness separator for tournament games - separator_key = game_league - if game.get("is_tournament") and game_league in ("ncaam", "ncaaw"): - tournament_key = f"{game_league}_tournament" - if tournament_key in self._separator_icons: - separator_key = tournament_key - - # Add league separator if switching leagues OR if this is the first league - if show_separators: - if current_league is None: - # First league - add separator at the start - separator = self._separator_icons.get(separator_key) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {separator_key} separator icon at start") - elif separator_key != current_league: - # Switching leagues or switching between regular/tournament - add separator - separator = self._separator_icons.get(separator_key) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {separator_key} separator icon") - - current_league = separator_key - - # Render game card - determine type from game state - try: - individual_game_type = self._determine_game_type(game) - game_img = renderer.render_game_card(game, individual_game_type) - - # Add horizontal padding to prevent logos from being cut off at edges - # Logos are positioned at -10 and display_width+10, so we need padding - padding = 12 # Padding on each side to ensure logos aren't cut off - padded_width = game_img.width + (padding * 2) - padded_img = Image.new('RGB', (padded_width, game_img.height), (0, 0, 0)) - padded_img.paste(game_img, (padding, 0)) - - content_items.append(padded_img) - game_count += 1 - league_counts[game_league] = league_counts.get(game_league, 0) + 1 - except Exception: - self.logger.exception("Error rendering game card") - continue - - if not content_items: - self.logger.warning("No game cards rendered") - return False - - # Store individual items for Vegas mode (avoids scroll_helper padding) - self._vegas_content_items = list(content_items) - - # Create scrolling image using ScrollHelper - self.scroll_helper.create_scrolling_image( - content_items, - item_gap=gap_between_games, - element_gap=0 # No element gap - each item is a complete game card - ) - - # Log what we loaded - league_summary = ", ".join([f"{league.upper()}({count})" for league, count in league_counts.items()]) - self.logger.info( - f"[Basketball Scroll] Prepared {game_count} games for scrolling: {league_summary}" - ) - self.logger.info( - f"[Basketball Scroll] Total scroll width: {self.scroll_helper.total_scroll_width}px, " - f"Dynamic duration: {self.scroll_helper.calculated_duration}s" - ) - - # Reset tracking state - self._is_scrolling = True - self._scroll_start_time = time.time() - self._frame_count = 0 - self._fps_sample_start = time.time() - - return True - - def display_scroll_frame(self) -> bool: - """ - Display the next frame of the scrolling content. - - Returns: - True if a frame was displayed, False if scroll is complete or no content - """ - if not self.scroll_helper or not self.scroll_helper.cached_image: - return False - - # Update scroll position - self.scroll_helper.update_scroll_position() - - # Get visible portion - visible = self.scroll_helper.get_visible_portion() - if not visible: - return False - - # Display the visible portion - try: - self.display_manager.image = visible - self.display_manager.update_display() - - # Track frame rate - self._frame_count += 1 - self.scroll_helper.log_frame_rate() - - # Periodic logging - self._log_scroll_progress() - - return True - except Exception: - self.logger.exception("Error displaying scroll frame") - return False - - def _log_scroll_progress(self) -> None: - """Log scroll progress and FPS periodically.""" - current_time = time.time() - - if current_time - self._last_log_time >= self._log_interval: - # Calculate FPS - elapsed = current_time - self._fps_sample_start - if elapsed > 0: - fps = self._frame_count / elapsed - - # Get scroll info - scroll_info = self.scroll_helper.get_scroll_info() - - self.logger.info( - f"[Basketball Scroll] FPS: {fps:.1f}, " - f"Position: {scroll_info['scroll_position']:.0f}/{scroll_info['total_width']}px, " - f"Elapsed: {scroll_info.get('elapsed_time', 0):.1f}s/{scroll_info['dynamic_duration']}s" - ) - - # Reset FPS tracking - self._frame_count = 0 - self._fps_sample_start = current_time - self._last_log_time = current_time - - def is_scroll_complete(self) -> bool: - """Check if the scroll cycle is complete.""" - if not self.scroll_helper: - return True - return self.scroll_helper.is_scroll_complete() - - def reset_scroll(self) -> None: - """Reset the scroll position to the beginning.""" - if self.scroll_helper: - self.scroll_helper.reset_scroll() - self._frame_count = 0 - self._fps_sample_start = time.time() - self.logger.debug("Scroll position reset") - - def get_scroll_info(self) -> Dict[str, Any]: - """Get current scroll state information.""" - if not self.scroll_helper: - return {"error": "ScrollHelper not available"} - - info = self.scroll_helper.get_scroll_info() - info.update({ - "game_count": len(self._current_games), - "game_type": self._current_game_type, - "leagues": self._current_leagues, - "is_scrolling": self._is_scrolling - }) - return info - - def get_dynamic_duration(self) -> int: - """Get the calculated dynamic duration for this scroll content.""" - if self.scroll_helper: - return self.scroll_helper.get_dynamic_duration() - return 60 # Default fallback - - def clear(self) -> None: - """Clear scroll content and reset state.""" - if self.scroll_helper: - self.scroll_helper.clear_cache() - self._current_games = [] - self._current_game_type = "" - self._current_leagues = [] - self._vegas_content_items = [] - self._is_scrolling = False - self._scroll_start_time = None - self.logger.debug("Scroll display cleared") - - -class LegacyScrollDisplayManager: - """ - Manages scroll display instances for different game types. - - This class provides a higher-level interface for the basketball plugin - to manage scroll displays for live, recent, and upcoming games. - """ - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplayManager. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Create scroll displays for each game type - self._scroll_displays: Dict[str, ScrollDisplay] = {} - self._current_game_type: Optional[str] = None - - def get_scroll_display(self, game_type: str) -> 'LegacyScrollDisplay': - """ - Get or create a scroll display for a game type. - - Args: - game_type: Type of games ('live', 'recent', 'upcoming') - - Returns: - ScrollDisplay instance for the game type - """ - if game_type not in self._scroll_displays: - self._scroll_displays[game_type] = LegacyScrollDisplay( - self.display_manager, - self.config, - self.logger, - global_config=self.global_config - ) - return self._scroll_displays[game_type] - - def prepare_and_display( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Dict[str, int] = None - ) -> bool: - """ - Prepare content and start displaying scroll. - - Args: - games: List of game dictionaries - game_type: Type of games - leagues: List of leagues - rankings_cache: Optional team rankings cache - - Returns: - True if scroll was started successfully - """ - scroll_display = self.get_scroll_display(game_type) - - success = scroll_display.prepare_scroll_content( - games, game_type, leagues, rankings_cache - ) - - if success: - self._current_game_type = game_type - - return success - - def display_frame(self, game_type: str = None) -> bool: - """ - Display the next frame of the current scroll. - - Args: - game_type: Optional game type (uses current if not specified) - - Returns: - True if a frame was displayed - """ - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return False - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return False - - return scroll_display.display_scroll_frame() - - def is_complete(self, game_type: str = None) -> bool: - """Check if the current scroll is complete.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return True - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return True - - return scroll_display.is_scroll_complete() - - def get_dynamic_duration(self, game_type: str = None) -> int: - """Get the dynamic duration for the current scroll.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return 60 - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return 60 - - return scroll_display.get_dynamic_duration() - - def has_cached_content(self) -> bool: - """ - Check if any scroll display has cached content. - - Returns: - True if any scroll display has a cached image ready for display - """ - for scroll_display in self._scroll_displays.values(): - if hasattr(scroll_display, 'scroll_helper') and scroll_display.scroll_helper: - if scroll_display.scroll_helper.cached_image is not None: - return True - return False - - def get_all_vegas_content_items(self) -> list: - """Collect _vegas_content_items from all scroll displays.""" - items = [] - for sd in self._scroll_displays.values(): - vegas_items = getattr(sd, '_vegas_content_items', None) - if vegas_items: - items.extend(vegas_items) - return items - - def clear_all(self) -> None: - """Clear all scroll displays.""" - for scroll_display in self._scroll_displays.values(): - scroll_display.clear() - self._current_game_type = None - -logger = logging.getLogger(__name__) - _USING_CORE_SCROLL = False try: from src.common.sports_scroll import ( diff --git a/plugins/hockey-scoreboard/manifest.json b/plugins/hockey-scoreboard/manifest.json index 676d51cf..5ceab7a6 100644 --- a/plugins/hockey-scoreboard/manifest.json +++ b/plugins/hockey-scoreboard/manifest.json @@ -1,7 +1,7 @@ { "id": "hockey-scoreboard", "name": "Hockey Scoreboard", - "version": "1.7.1", + "version": "1.7.2", "author": "ChuckBuilds", "description": "Live, recent, and upcoming hockey games across NHL, NCAA Men's, and NCAA Women's hockey with real-time scores and schedules", "homepage": "https://github.com/ChuckBuilds/ledmatrix-plugins/tree/main/plugins/hockey-scoreboard", @@ -54,6 +54,12 @@ } ], "versions": [ + { + "version": "1.7.2", + "released": "2026-08-05", + "notes": "Housekeeping, no behaviour change: scroll_display.py carried a second, unreferenced copy of the bundled fallback classes at module level -- the file was the pre-adoption and adopted versions concatenated rather than one replacing the other. The live fallback in scroll_display_legacy.py is untouched. Removing the dead copy is what makes a missing constant visible instead of appearing defined; that dead block is where the ones that broke scroll mode were hiding.", + "ledmatrix_min_version": "2.0.0" + }, { "version": "1.7.1", "released": "2026-08-05", diff --git a/plugins/hockey-scoreboard/scroll_display.py b/plugins/hockey-scoreboard/scroll_display.py index cc6db85f..5a16a58b 100644 --- a/plugins/hockey-scoreboard/scroll_display.py +++ b/plugins/hockey-scoreboard/scroll_display.py @@ -33,7 +33,6 @@ except ImportError: ScrollHelper = None -from game_renderer import GameRenderer logger = logging.getLogger(__name__) @@ -45,662 +44,6 @@ RESAMPLE_FILTER = Image.LANCZOS -class LegacyScrollDisplay: - """ - Handles scroll display mode for the hockey scoreboard plugin. - - This class: - - Collects all games matching criteria (respecting live priority) - - Pre-renders each game using GameRenderer - - Adds league separator icons between different leagues - - Composes a single wide image using ScrollHelper - - Implements dynamic duration based on total content width - - Logs FPS and game count during scrolling - """ - - # Paths to league separator icons - NHL_SEPARATOR_ICON = "assets/sports/nhl_logos/NHL.png" - NCAA_SEPARATOR_ICON = "assets/sports/ncaa_logos/NCAA.png" - NCAAM_HOCKEY_SEPARATOR_ICON = "assets/sports/ncaa_logos/ncaa_hockey.png" - NCAAW_HOCKEY_SEPARATOR_ICON = "assets/sports/ncaa_logos/ncaa_hockey.png" - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplay handler. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Get display dimensions - if hasattr(display_manager, 'matrix') and display_manager.matrix is not None: - self.display_width = display_manager.matrix.width - self.display_height = display_manager.matrix.height - else: - self.display_width = getattr(display_manager, "width", 128) - self.display_height = getattr(display_manager, "height", 32) - - # Initialize ScrollHelper - if ScrollHelper: - self.scroll_helper = ScrollHelper( - self.display_width, - self.display_height, - self.logger - ) - # Configure scroll settings - self._configure_scroll_helper() - else: - self.scroll_helper = None - self.logger.error("ScrollHelper not available - scroll mode will not work") - - # Shared logo cache for game renderer - self._logo_cache: Dict[str, Image.Image] = {} - - # League separator icons cache - self._separator_icons: Dict[str, Image.Image] = {} - self._load_separator_icons() - - # Tracking state - self._current_games: List[Dict] = [] - self._current_game_type: str = "" - self._current_leagues: List[str] = [] - self._vegas_content_items: List[Image.Image] = [] - self._is_scrolling = False - self._scroll_start_time: Optional[float] = None - self._last_log_time: float = 0 - self._log_interval: float = 5.0 # Log every 5 seconds - - # Performance tracking - self._frame_count: int = 0 - self._fps_sample_start: float = time.time() - - def _configure_scroll_helper(self) -> None: - """Configure scroll helper with settings from config.""" - if not self.scroll_helper: - return - - # Get global scroll settings, then per-league overrides - scroll_settings = self._get_scroll_settings() - - # Set scroll speed (pixels per second in time-based mode) - scroll_speed = scroll_settings.get("scroll_speed", 50.0) - self.scroll_helper.set_scroll_speed(scroll_speed) - - # Set scroll delay - scroll_delay = scroll_settings.get("scroll_delay", 0.01) - self.scroll_helper.set_scroll_delay(scroll_delay) - - # Enable dynamic duration - dynamic_duration = scroll_settings.get("dynamic_duration", True) - self.scroll_helper.set_dynamic_duration_settings( - enabled=dynamic_duration, - min_duration=30, - max_duration=600, # 10 minutes max - buffer=0.2 # 20% buffer to ensure scroll completes fully off screen - ) - - # Use frame-based scrolling for better FPS control - self.scroll_helper.set_frame_based_scrolling(True) - - # Convert scroll_speed from pixels/second to pixels/frame for frame-based mode - # Formula: pixels_per_frame = (pixels/second) * (seconds/frame) - if scroll_delay > 0: - pixels_per_frame = scroll_speed * scroll_delay - else: - # Fallback: assume 100 FPS if delay is 0 - pixels_per_frame = scroll_speed / 100.0 - - # Clamp to reasonable range (0.1 to 5 pixels per frame for smooth scrolling) - pixels_per_frame = max(0.1, min(5.0, pixels_per_frame)) - self.scroll_helper.set_scroll_speed(pixels_per_frame) - - # Calculate effective pixels per second for logging - effective_pps = pixels_per_frame / scroll_delay if scroll_delay > 0 else pixels_per_frame * 100 - - self.logger.info( - f"ScrollHelper configured: {pixels_per_frame:.2f} px/frame, delay={scroll_delay}s " - f"(effective {effective_pps:.1f} px/s from {scroll_speed} px/s config), dynamic_duration={dynamic_duration}" - ) - - # Honor the global smooth-scrolling FPS target (older cores lack the setter) - target_fps = self.global_config.get('target_fps') or self.global_config.get('scroll_target_fps') - try: - # Coerce before comparing: a malformed global config value - # must degrade to today's scroll_delay pacing, not raise. - target_fps = float(target_fps) if target_fps is not None else None - except (TypeError, ValueError): - target_fps = None - if target_fps: - if hasattr(self.scroll_helper, 'set_target_fps'): - self.scroll_helper.set_target_fps(target_fps) - else: - self.scroll_helper.target_fps = max(30.0, min(200.0, target_fps)) - self.scroll_helper.frame_time_target = 1.0 / self.scroll_helper.target_fps - - def _get_scroll_settings(self, league: Optional[str] = None) -> Dict[str, Any]: - """Get scroll settings, optionally for a specific league.""" - # Default scroll settings - defaults = { - "scroll_speed": 50.0, - "scroll_delay": 0.01, - "gap_between_games": 48, - "show_league_separators": True, - "dynamic_duration": True, - "game_card_width": 128, - } - - # Try to get league-specific settings first - if league: - league_config = self.config.get(league, {}) - league_scroll = league_config.get("scroll_settings", {}) - if league_scroll: - return {**defaults, **league_scroll} - - # Fall back to NHL settings (usually first enabled) - nhl_config = self.config.get("nhl", {}) - nhl_scroll = nhl_config.get("scroll_settings", {}) - if nhl_scroll: - return {**defaults, **nhl_scroll} - - # Fall back to NCAA Men's settings (try both naming conventions) - for league_key in ["ncaa_mens", "ncaam_hockey"]: - ncaa_config = self.config.get(league_key, {}) - ncaa_scroll = ncaa_config.get("scroll_settings", {}) - if ncaa_scroll: - return {**defaults, **ncaa_scroll} - - # Fall back to NCAA Women's settings (try both naming conventions) - for league_key in ["ncaa_womens", "ncaaw_hockey"]: - ncaa_config = self.config.get(league_key, {}) - ncaa_scroll = ncaa_config.get("scroll_settings", {}) - if ncaa_scroll: - return {**defaults, **ncaa_scroll} - - return defaults - - def _load_separator_icons(self) -> None: - """Load and resize league separator icons.""" - separator_height = self.display_height - 4 # Leave some padding - - # Load NHL icon - if os.path.exists(self.NHL_SEPARATOR_ICON): - try: - # Use context manager to ensure file handle is closed - with Image.open(self.NHL_SEPARATOR_ICON) as nhl_file: - # Convert creates a copy; if already RGBA, use copy() to detach from file - if nhl_file.mode != "RGBA": - nhl_icon = nhl_file.convert("RGBA") - else: - nhl_icon = nhl_file.copy() - # Resize to fit height while maintaining aspect ratio (after file is closed) - aspect = nhl_icon.width / nhl_icon.height - new_width = int(separator_height * aspect) - nhl_icon = nhl_icon.resize((new_width, separator_height), resample=RESAMPLE_FILTER) - self._separator_icons["nhl"] = nhl_icon - self.logger.debug(f"Loaded NHL separator icon: {new_width}x{separator_height}") - except Exception: - self.logger.exception("Error loading NHL separator icon") - else: - self.logger.warning(f"NHL separator icon not found at {self.NHL_SEPARATOR_ICON}") - - # Load NCAA icon (try sport-specific first, then generic) - ncaa_icon_paths = [ - (self.NCAAM_HOCKEY_SEPARATOR_ICON, ["ncaam_hockey", "ncaa_mens"]), - (self.NCAAW_HOCKEY_SEPARATOR_ICON, ["ncaaw_hockey", "ncaa_womens"]), - (self.NCAA_SEPARATOR_ICON, ["ncaa"]), - ] - - for icon_path, league_keys in ncaa_icon_paths: - if os.path.exists(icon_path): - try: - # Use context manager to ensure file handle is closed - with Image.open(icon_path) as ncaa_file: - # Convert creates a copy; if already RGBA, use copy() to detach from file - if ncaa_file.mode != "RGBA": - ncaa_icon = ncaa_file.convert("RGBA") - else: - ncaa_icon = ncaa_file.copy() - # Resize to fit height while maintaining aspect ratio (after file is closed) - aspect = ncaa_icon.width / ncaa_icon.height - new_width = int(separator_height * aspect) - ncaa_icon = ncaa_icon.resize((new_width, separator_height), resample=RESAMPLE_FILTER) - for key in league_keys: - self._separator_icons[key] = ncaa_icon - self.logger.debug(f"Loaded NCAA separator icon from {icon_path}: {new_width}x{separator_height}") - except Exception: - self.logger.exception(f"Error loading NCAA separator icon from {icon_path}") - - def _determine_game_type(self, game: Dict) -> str: - """ - Determine the game type from the game's status. - - Args: - game: Game dictionary - - Returns: - Game type: 'live', 'recent', or 'upcoming' - """ - state = game.get('status', {}).get('state', '') - if state == 'in': - return 'live' - elif state == 'post': - return 'recent' - elif state == 'pre': - return 'upcoming' - else: - # Default to upcoming if state is unknown - return 'upcoming' - - def prepare_scroll_content( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Optional[Dict[str, int]] = None - ) -> bool: - """ - Prepare scrolling content from a list of games. - - Args: - games: List of game dictionaries with league info - game_type: Type hint ('live', 'recent', 'upcoming', or 'mixed' for mixed types) - leagues: List of leagues in order (e.g., ['nhl', 'ncaam_hockey', 'ncaaw_hockey']) - rankings_cache: Optional team rankings cache - - Returns: - True if content was prepared successfully, False otherwise - """ - if not self.scroll_helper: - self.logger.error("ScrollHelper not available") - return False - - if not games: - self.logger.debug("No games to prepare for scrolling") - self.clear() # Reset all scroll state, not just cache - return False - - self._current_games = games - self._current_game_type = game_type - self._current_leagues = leagues - - # Get scroll settings using primary league from the provided leagues list - primary_league = leagues[0] if leagues else None - scroll_settings = self._get_scroll_settings(primary_league) - gap_between_games = scroll_settings.get("gap_between_games", 24) - show_separators = scroll_settings.get("show_league_separators", True) - game_card_width = scroll_settings.get("game_card_width", 128) - - # Create game renderer using game_card_width so cards are a fixed size - # regardless of the full chain width (display_width may span multiple panels) - renderer = GameRenderer( - game_card_width, - self.display_height, - self.config, - logo_cache=self._logo_cache, - custom_logger=self.logger - ) - if rankings_cache: - renderer.set_rankings_cache(rankings_cache) - - # Pre-render all game cards - content_items: List[Image.Image] = [] - current_league = None - game_count = 0 - league_counts: Dict[str, int] = {} - - for game in games: - game_league = game.get("league", "nhl") # Default to NHL if not specified - - # Add league separator if switching leagues OR if this is the first league - if show_separators: - if current_league is None: - # First league - add separator at the start - separator = self._separator_icons.get(game_league) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {game_league} separator icon at start") - elif game_league != current_league: - # Switching leagues - add separator - separator = self._separator_icons.get(game_league) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {game_league} separator icon") - - current_league = game_league - - # Render game card - # Only determine type from game state when in 'mixed' mode; otherwise use the passed game_type - try: - if game_type == 'mixed': - individual_game_type = self._determine_game_type(game) - else: - individual_game_type = game_type - game_img = renderer.render_game_card(game, individual_game_type) - - # Add horizontal padding to prevent logos from being cut off at edges - # Logos are positioned at -10 and display_width+10, so we need padding - padding = 12 # Padding on each side to ensure logos aren't cut off - padded_width = game_img.width + (padding * 2) - padded_img = Image.new('RGB', (padded_width, game_img.height), (0, 0, 0)) - padded_img.paste(game_img, (padding, 0)) - - content_items.append(padded_img) - game_count += 1 - league_counts[game_league] = league_counts.get(game_league, 0) + 1 - except Exception: - self.logger.exception("Error rendering game card") - continue - - if not content_items: - self.logger.warning("No game cards rendered") - return False - - # Store individual items for Vegas mode (avoids scroll_helper padding) - self._vegas_content_items = list(content_items) - - # Create scrolling image using ScrollHelper - self.scroll_helper.create_scrolling_image( - content_items, - item_gap=gap_between_games, - element_gap=0 # No element gap - each item is a complete game card - ) - - # Log what we loaded - league_summary = ", ".join([f"{league.upper()}({count})" for league, count in league_counts.items()]) - self.logger.info( - f"[Hockey Scroll] Prepared {game_count} games for scrolling: {league_summary}" - ) - self.logger.info( - f"[Hockey Scroll] Total scroll width: {self.scroll_helper.total_scroll_width}px, " - f"Dynamic duration: {self.scroll_helper.calculated_duration}s" - ) - - # Reset tracking state - self._is_scrolling = True - self._scroll_start_time = time.time() - self._frame_count = 0 - self._fps_sample_start = time.time() - - return True - - def display_scroll_frame(self) -> bool: - """ - Display the next frame of the scrolling content. - - Returns: - True if a frame was displayed, False if scroll is complete or no content - """ - if not self.scroll_helper or not self.scroll_helper.cached_image: - return False - - # Update scroll position - self.scroll_helper.update_scroll_position() - - # Get visible portion - visible = self.scroll_helper.get_visible_portion() - if not visible: - return False - - # Display the visible portion - try: - self.display_manager.image = visible - self.display_manager.update_display() - - # Track frame rate - self._frame_count += 1 - self.scroll_helper.log_frame_rate() - - # Periodic logging - self._log_scroll_progress() - except Exception: - self.logger.exception("Error displaying scroll frame") - return False - else: - return True - - def _log_scroll_progress(self) -> None: - """Log scroll progress and FPS periodically.""" - current_time = time.time() - - if current_time - self._last_log_time >= self._log_interval: - # Calculate FPS - elapsed = current_time - self._fps_sample_start - if elapsed > 0: - fps = self._frame_count / elapsed - - # Get scroll info - scroll_info = self.scroll_helper.get_scroll_info() - - self.logger.info( - f"[Hockey Scroll] FPS: {fps:.1f}, " - f"Position: {scroll_info['scroll_position']:.0f}/{scroll_info['total_width']}px, " - f"Elapsed: {scroll_info.get('elapsed_time', 0):.1f}s/{scroll_info['dynamic_duration']}s" - ) - - # Reset FPS tracking - self._frame_count = 0 - self._fps_sample_start = current_time - self._last_log_time = current_time - - def is_scroll_complete(self) -> bool: - """Check if the scroll cycle is complete.""" - if not self.scroll_helper: - return True - return self.scroll_helper.is_scroll_complete() - - def reset_scroll(self) -> None: - """Reset the scroll position to the beginning.""" - if self.scroll_helper: - self.scroll_helper.reset_scroll() - self._frame_count = 0 - self._fps_sample_start = time.time() - self.logger.debug("Scroll position reset") - - def get_scroll_info(self) -> Dict[str, Any]: - """Get current scroll state information.""" - if not self.scroll_helper: - return {"error": "ScrollHelper not available"} - - info = self.scroll_helper.get_scroll_info() - info.update({ - "game_count": len(self._current_games), - "game_type": self._current_game_type, - "leagues": self._current_leagues, - "is_scrolling": self._is_scrolling - }) - return info - - def get_dynamic_duration(self) -> int: - """Get the calculated dynamic duration for this scroll content.""" - if self.scroll_helper: - return self.scroll_helper.get_dynamic_duration() - return 60 # Default fallback - - def clear(self) -> None: - """Clear scroll content and reset state.""" - if self.scroll_helper: - self.scroll_helper.clear_cache() - self._current_games = [] - self._current_game_type = "" - self._current_leagues = [] - self._vegas_content_items = [] - self._is_scrolling = False - self._scroll_start_time = None - self.logger.debug("Scroll display cleared") - - -class LegacyScrollDisplayManager: - """ - Manages scroll display instances for different game types. - - This class provides a higher-level interface for the hockey plugin - to manage scroll displays for live, recent, and upcoming games. - """ - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplayManager. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Create scroll displays for each game type - self._scroll_displays: Dict[str, ScrollDisplay] = {} - self._current_game_type: Optional[str] = None - - def get_scroll_display(self, game_type: str) -> 'LegacyScrollDisplay': - """ - Get or create a scroll display for a game type. - - Args: - game_type: Type of games ('live', 'recent', 'upcoming') - - Returns: - ScrollDisplay instance for the game type - """ - if game_type not in self._scroll_displays: - self._scroll_displays[game_type] = LegacyScrollDisplay( - self.display_manager, - self.config, - self.logger, - global_config=self.global_config - ) - return self._scroll_displays[game_type] - - def prepare_and_display( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Optional[Dict[str, int]] = None - ) -> bool: - """ - Prepare content and start displaying scroll. - - Args: - games: List of game dictionaries - game_type: Type of games - leagues: List of leagues - rankings_cache: Optional team rankings cache - - Returns: - True if scroll was started successfully - """ - scroll_display = self.get_scroll_display(game_type) - - success = scroll_display.prepare_scroll_content( - games, game_type, leagues, rankings_cache - ) - - if success: - self._current_game_type = game_type - - return success - - def display_frame(self, game_type: Optional[str] = None) -> bool: - """ - Display the next frame of the current scroll. - - Args: - game_type: Optional game type (uses current if not specified) - - Returns: - True if a frame was displayed - """ - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return False - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return False - - return scroll_display.display_scroll_frame() - - def is_complete(self, game_type: Optional[str] = None) -> bool: - """Check if the current scroll is complete.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return True - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return True - - return scroll_display.is_scroll_complete() - - def get_dynamic_duration(self, game_type: Optional[str] = None) -> int: - """Get the dynamic duration for the current scroll.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return 60 - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return 60 - - return scroll_display.get_dynamic_duration() - - def get_all_vegas_content_items(self) -> list: - """Collect _vegas_content_items from all scroll displays.""" - items = [] - for sd in self._scroll_displays.values(): - vegas_items = getattr(sd, '_vegas_content_items', None) - if vegas_items: - items.extend(vegas_items) - return items - - def clear_all(self) -> None: - """Clear all scroll displays.""" - for scroll_display in self._scroll_displays.values(): - scroll_display.clear() - self._current_game_type = None - -logger = logging.getLogger(__name__) - _USING_CORE_SCROLL = False try: from src.common.sports_scroll import ( diff --git a/plugins/lacrosse-scoreboard/manifest.json b/plugins/lacrosse-scoreboard/manifest.json index ee91a52f..b1365c91 100644 --- a/plugins/lacrosse-scoreboard/manifest.json +++ b/plugins/lacrosse-scoreboard/manifest.json @@ -1,7 +1,7 @@ { "id": "lacrosse-scoreboard", "name": "Lacrosse Scoreboard", - "version": "1.7.1", + "version": "1.7.2", "author": "ChuckBuilds", "description": "Live, recent, and upcoming NCAA men's and women's lacrosse games with real-time scores and schedules", "homepage": "https://github.com/ChuckBuilds/ledmatrix-plugins/tree/main/plugins/lacrosse-scoreboard", @@ -50,6 +50,12 @@ } ], "versions": [ + { + "version": "1.7.2", + "released": "2026-08-05", + "notes": "Housekeeping, no behaviour change: scroll_display.py carried a second, unreferenced copy of the bundled fallback classes at module level -- the file was the pre-adoption and adopted versions concatenated rather than one replacing the other. The live fallback in scroll_display_legacy.py is untouched. Removing the dead copy is what makes a missing constant visible instead of appearing defined; that dead block is where the ones that broke scroll mode were hiding.", + "ledmatrix_min_version": "2.0.0" + }, { "version": "1.7.1", "released": "2026-08-05", diff --git a/plugins/lacrosse-scoreboard/scroll_display.py b/plugins/lacrosse-scoreboard/scroll_display.py index c902ebaa..985d8e62 100644 --- a/plugins/lacrosse-scoreboard/scroll_display.py +++ b/plugins/lacrosse-scoreboard/scroll_display.py @@ -33,7 +33,6 @@ except ImportError: ScrollHelper = None -from game_renderer import GameRenderer logger = logging.getLogger(__name__) @@ -45,643 +44,6 @@ RESAMPLE_FILTER = Image.LANCZOS -class LegacyScrollDisplay: - """ - Handles scroll display mode for the lacrosse scoreboard plugin. - - This class: - - Collects all games matching criteria (respecting live priority) - - Pre-renders each game using GameRenderer - - Adds league separator icons between different leagues - - Composes a single wide image using ScrollHelper - - Implements dynamic duration based on total content width - - Logs FPS and game count during scrolling - """ - - # Paths to league separator icons. Lacrosse uses a single NCAA lacrosse - # logo for both men's and women's since ESPN does not ship separate - # gendered marks for the sport. - NCAA_SEPARATOR_ICON = "assets/sports/ncaa_logos/NCAA.png" - NCAA_LACROSSE_SEPARATOR_ICON = "assets/sports/ncaa_logos/ncaa_lacrosse.png" - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplay handler. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Get display dimensions - if hasattr(display_manager, 'matrix') and display_manager.matrix is not None: - self.display_width = display_manager.matrix.width - self.display_height = display_manager.matrix.height - else: - self.display_width = getattr(display_manager, "width", 128) - self.display_height = getattr(display_manager, "height", 32) - - # Initialize ScrollHelper - if ScrollHelper: - self.scroll_helper = ScrollHelper( - self.display_width, - self.display_height, - self.logger - ) - # Configure scroll settings - self._configure_scroll_helper() - else: - self.scroll_helper = None - self.logger.error("ScrollHelper not available - scroll mode will not work") - - # Shared logo cache for game renderer - self._logo_cache: Dict[str, Image.Image] = {} - - # League separator icons cache - self._separator_icons: Dict[str, Image.Image] = {} - self._load_separator_icons() - - # Tracking state - self._current_games: List[Dict] = [] - self._current_game_type: str = "" - self._current_leagues: List[str] = [] - self._vegas_content_items: List[Image.Image] = [] - self._is_scrolling = False - self._scroll_start_time: Optional[float] = None - self._last_log_time: float = 0 - self._log_interval: float = 5.0 # Log every 5 seconds - - # Performance tracking - self._frame_count: int = 0 - self._fps_sample_start: float = time.time() - - def _configure_scroll_helper(self) -> None: - """Configure scroll helper with settings from config.""" - if not self.scroll_helper: - return - - # Get global scroll settings, then per-league overrides - scroll_settings = self._get_scroll_settings() - - # Set scroll speed (pixels per second in time-based mode) - scroll_speed = scroll_settings.get("scroll_speed", 50.0) - self.scroll_helper.set_scroll_speed(scroll_speed) - - # Set scroll delay - scroll_delay = scroll_settings.get("scroll_delay", 0.01) - self.scroll_helper.set_scroll_delay(scroll_delay) - - # Enable dynamic duration - dynamic_duration = scroll_settings.get("dynamic_duration", True) - self.scroll_helper.set_dynamic_duration_settings( - enabled=dynamic_duration, - min_duration=30, - max_duration=600, # 10 minutes max - buffer=0.2 # 20% buffer to ensure scroll completes fully off screen - ) - - # Use frame-based scrolling for better FPS control - self.scroll_helper.set_frame_based_scrolling(True) - - # Convert scroll_speed from pixels/second to pixels/frame for frame-based mode - # Formula: pixels_per_frame = (pixels/second) * (seconds/frame) - if scroll_delay > 0: - pixels_per_frame = scroll_speed * scroll_delay - else: - # Fallback: assume 100 FPS if delay is 0 - pixels_per_frame = scroll_speed / 100.0 - - # Clamp to reasonable range (0.1 to 5 pixels per frame for smooth scrolling) - pixels_per_frame = max(0.1, min(5.0, pixels_per_frame)) - self.scroll_helper.set_scroll_speed(pixels_per_frame) - - # Calculate effective pixels per second for logging - effective_pps = pixels_per_frame / scroll_delay if scroll_delay > 0 else pixels_per_frame * 100 - - self.logger.info( - f"ScrollHelper configured: {pixels_per_frame:.2f} px/frame, delay={scroll_delay}s " - f"(effective {effective_pps:.1f} px/s from {scroll_speed} px/s config), dynamic_duration={dynamic_duration}" - ) - - # Honor the global smooth-scrolling FPS target (older cores lack the setter) - target_fps = self.global_config.get('target_fps') or self.global_config.get('scroll_target_fps') - try: - # Coerce before comparing: a malformed global config value - # must degrade to today's scroll_delay pacing, not raise. - target_fps = float(target_fps) if target_fps is not None else None - except (TypeError, ValueError): - target_fps = None - if target_fps: - if hasattr(self.scroll_helper, 'set_target_fps'): - self.scroll_helper.set_target_fps(target_fps) - else: - self.scroll_helper.target_fps = max(30.0, min(200.0, target_fps)) - self.scroll_helper.frame_time_target = 1.0 / self.scroll_helper.target_fps - - def _get_scroll_settings(self, league: Optional[str] = None) -> Dict[str, Any]: - """Get scroll settings, optionally for a specific league.""" - # Default scroll settings - defaults = { - "scroll_speed": 50.0, - "scroll_delay": 0.01, - "gap_between_games": 48, - "show_league_separators": True, - "dynamic_duration": True, - "game_card_width": 128, - } - - # Try to get league-specific settings first - if league: - league_config = self.config.get(league, {}) - league_scroll = league_config.get("scroll_settings", {}) - if league_scroll: - return {**defaults, **league_scroll} - - # Fall back to NCAA Men's settings (try both naming conventions) - for league_key in ["ncaa_mens", "ncaam_lacrosse"]: - ncaa_config = self.config.get(league_key, {}) - ncaa_scroll = ncaa_config.get("scroll_settings", {}) - if ncaa_scroll: - return {**defaults, **ncaa_scroll} - - # Fall back to NCAA Women's settings (try both naming conventions) - for league_key in ["ncaa_womens", "ncaaw_lacrosse"]: - ncaa_config = self.config.get(league_key, {}) - ncaa_scroll = ncaa_config.get("scroll_settings", {}) - if ncaa_scroll: - return {**defaults, **ncaa_scroll} - - return defaults - - def _load_separator_icons(self) -> None: - """Load and resize league separator icons.""" - separator_height = self.display_height - 4 # Leave some padding - - # Load NCAA icon (try sport-specific first, then generic). Both - # entries register under the lacrosse league keys so the generic - # NCAA.png acts as a real fallback when ncaa_lacrosse.png is missing — - # otherwise separator lookups for "ncaam_lacrosse" / "ncaaw_lacrosse" - # would silently return None. - lacrosse_keys = ["ncaam_lacrosse", "ncaa_mens", - "ncaaw_lacrosse", "ncaa_womens"] - ncaa_icon_paths = [ - (self.NCAA_LACROSSE_SEPARATOR_ICON, lacrosse_keys), - (self.NCAA_SEPARATOR_ICON, [*lacrosse_keys, "ncaa"]), - ] - - for icon_path, league_keys in ncaa_icon_paths: - if os.path.exists(icon_path): - try: - # Use context manager to ensure file handle is closed - with Image.open(icon_path) as ncaa_file: - # Convert creates a copy; if already RGBA, use copy() to detach from file - if ncaa_file.mode != "RGBA": - ncaa_icon = ncaa_file.convert("RGBA") - else: - ncaa_icon = ncaa_file.copy() - # Resize to fit height while maintaining aspect ratio (after file is closed) - aspect = ncaa_icon.width / ncaa_icon.height - new_width = int(separator_height * aspect) - ncaa_icon = ncaa_icon.resize((new_width, separator_height), resample=RESAMPLE_FILTER) - # Only populate keys that haven't been set yet so the - # sport-specific icon (iterated first) always wins over - # the generic NCAA fallback. - for key in league_keys: - self._separator_icons.setdefault(key, ncaa_icon) - self.logger.debug(f"Loaded NCAA separator icon from {icon_path}: {new_width}x{separator_height}") - except Exception: - self.logger.exception(f"Error loading NCAA separator icon from {icon_path}") - - def _determine_game_type(self, game: Dict) -> str: - """ - Determine the game type from the game's status. - - Args: - game: Game dictionary - - Returns: - Game type: 'live', 'recent', or 'upcoming' - """ - state = game.get('status', {}).get('state', '') - if state == 'in': - return 'live' - elif state == 'post': - return 'recent' - elif state == 'pre': - return 'upcoming' - else: - # Default to upcoming if state is unknown - return 'upcoming' - - def prepare_scroll_content( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Optional[Dict[str, int]] = None - ) -> bool: - """ - Prepare scrolling content from a list of games. - - Args: - games: List of game dictionaries with league info - game_type: Type hint ('live', 'recent', 'upcoming', or 'mixed' for mixed types) - leagues: List of leagues in order (e.g., ['ncaam_lacrosse', 'ncaaw_lacrosse']) - rankings_cache: Optional team rankings cache - - Returns: - True if content was prepared successfully, False otherwise - """ - if not self.scroll_helper: - self.logger.error("ScrollHelper not available") - return False - - if not games: - self.logger.debug("No games to prepare for scrolling") - self.clear() # Reset all scroll state, not just cache - return False - - self._current_games = games - self._current_game_type = game_type - self._current_leagues = leagues - - # Get scroll settings using primary league from the provided leagues list - primary_league = leagues[0] if leagues else None - scroll_settings = self._get_scroll_settings(primary_league) - gap_between_games = scroll_settings.get("gap_between_games", 24) - show_separators = scroll_settings.get("show_league_separators", True) - game_card_width = scroll_settings.get("game_card_width", 128) - - # Create game renderer using game_card_width so cards are a fixed size - # regardless of the full chain width (display_width may span multiple panels) - renderer = GameRenderer( - game_card_width, - self.display_height, - self.config, - logo_cache=self._logo_cache, - custom_logger=self.logger - ) - if rankings_cache: - renderer.set_rankings_cache(rankings_cache) - - # Pre-render all game cards - content_items: List[Image.Image] = [] - current_league = None - game_count = 0 - league_counts: Dict[str, int] = {} - - for game in games: - game_league = game.get("league", "ncaam_lacrosse") # Default to NCAA Men's Lacrosse if not specified - - # Add league separator if switching leagues OR if this is the first league - if show_separators: - if current_league is None: - # First league - add separator at the start - separator = self._separator_icons.get(game_league) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {game_league} separator icon at start") - elif game_league != current_league: - # Switching leagues - add separator - separator = self._separator_icons.get(game_league) - if separator: - # Create a separator image with proper background - sep_img = Image.new('RGB', (separator.width + 8, self.display_height), (0, 0, 0)) - # Center the separator vertically - y_offset = (self.display_height - separator.height) // 2 - sep_img.paste(separator, (4, y_offset), separator) - content_items.append(sep_img) - self.logger.debug(f"Added {game_league} separator icon") - - current_league = game_league - - # Render game card - # Only determine type from game state when in 'mixed' mode; otherwise use the passed game_type - try: - if game_type == 'mixed': - individual_game_type = self._determine_game_type(game) - else: - individual_game_type = game_type - game_img = renderer.render_game_card(game, individual_game_type) - - # Add horizontal padding to prevent logos from being cut off at edges - # Logos are positioned at -10 and display_width+10, so we need padding - padding = 12 # Padding on each side to ensure logos aren't cut off - padded_width = game_img.width + (padding * 2) - padded_img = Image.new('RGB', (padded_width, game_img.height), (0, 0, 0)) - padded_img.paste(game_img, (padding, 0)) - - content_items.append(padded_img) - game_count += 1 - league_counts[game_league] = league_counts.get(game_league, 0) + 1 - except Exception: - self.logger.exception("Error rendering game card") - continue - - if not content_items: - self.logger.warning("No game cards rendered") - return False - - # Store individual items for Vegas mode (avoids scroll_helper padding) - self._vegas_content_items = list(content_items) - - # Create scrolling image using ScrollHelper - self.scroll_helper.create_scrolling_image( - content_items, - item_gap=gap_between_games, - element_gap=0 # No element gap - each item is a complete game card - ) - - # Log what we loaded - league_summary = ", ".join([f"{league.upper()}({count})" for league, count in league_counts.items()]) - self.logger.info( - f"[Lacrosse Scroll] Prepared {game_count} games for scrolling: {league_summary}" - ) - self.logger.info( - f"[Lacrosse Scroll] Total scroll width: {self.scroll_helper.total_scroll_width}px, " - f"Dynamic duration: {self.scroll_helper.calculated_duration}s" - ) - - # Reset tracking state - self._is_scrolling = True - self._scroll_start_time = time.time() - self._frame_count = 0 - self._fps_sample_start = time.time() - - return True - - def display_scroll_frame(self) -> bool: - """ - Display the next frame of the scrolling content. - - Returns: - True if a frame was displayed, False if scroll is complete or no content - """ - if not self.scroll_helper or not self.scroll_helper.cached_image: - return False - - # Update scroll position - self.scroll_helper.update_scroll_position() - - # Get visible portion - visible = self.scroll_helper.get_visible_portion() - if not visible: - return False - - # Display the visible portion - try: - self.display_manager.image = visible - self.display_manager.update_display() - - # Track frame rate - self._frame_count += 1 - self.scroll_helper.log_frame_rate() - - # Periodic logging - self._log_scroll_progress() - except Exception: - self.logger.exception("Error displaying scroll frame") - return False - else: - return True - - def _log_scroll_progress(self) -> None: - """Log scroll progress and FPS periodically.""" - current_time = time.time() - - if current_time - self._last_log_time >= self._log_interval: - # Calculate FPS - elapsed = current_time - self._fps_sample_start - if elapsed > 0: - fps = self._frame_count / elapsed - - # Get scroll info - scroll_info = self.scroll_helper.get_scroll_info() - - self.logger.info( - f"[Lacrosse Scroll] FPS: {fps:.1f}, " - f"Position: {scroll_info['scroll_position']:.0f}/{scroll_info['total_width']}px, " - f"Elapsed: {scroll_info.get('elapsed_time', 0):.1f}s/{scroll_info['dynamic_duration']}s" - ) - - # Reset FPS tracking - self._frame_count = 0 - self._fps_sample_start = current_time - self._last_log_time = current_time - - def is_scroll_complete(self) -> bool: - """Check if the scroll cycle is complete.""" - if not self.scroll_helper: - return True - return self.scroll_helper.is_scroll_complete() - - def reset_scroll(self) -> None: - """Reset the scroll position to the beginning.""" - if self.scroll_helper: - self.scroll_helper.reset_scroll() - self._frame_count = 0 - self._fps_sample_start = time.time() - self.logger.debug("Scroll position reset") - - def get_scroll_info(self) -> Dict[str, Any]: - """Get current scroll state information.""" - if not self.scroll_helper: - return {"error": "ScrollHelper not available"} - - info = self.scroll_helper.get_scroll_info() - info.update({ - "game_count": len(self._current_games), - "game_type": self._current_game_type, - "leagues": self._current_leagues, - "is_scrolling": self._is_scrolling - }) - return info - - def get_dynamic_duration(self) -> int: - """Get the calculated dynamic duration for this scroll content.""" - if self.scroll_helper: - return self.scroll_helper.get_dynamic_duration() - return 60 # Default fallback - - def clear(self) -> None: - """Clear scroll content and reset state.""" - if self.scroll_helper: - self.scroll_helper.clear_cache() - self._current_games = [] - self._current_game_type = "" - self._current_leagues = [] - self._vegas_content_items = [] - self._is_scrolling = False - self._scroll_start_time = None - self.logger.debug("Scroll display cleared") - - -class LegacyScrollDisplayManager: - """ - Manages scroll display instances for different game types. - - This class provides a higher-level interface for the lacrosse plugin - to manage scroll displays for live, recent, and upcoming games. - """ - - def __init__( - self, - display_manager, - config: Dict[str, Any], - custom_logger: Optional[logging.Logger] = None, - global_config: Optional[Dict[str, Any]] = None - ): - """ - Initialize the ScrollDisplayManager. - - Args: - display_manager: Display manager instance - config: Plugin configuration dictionary - custom_logger: Optional custom logger instance - global_config: Optional global LEDMatrix configuration dictionary - """ - self.display_manager = display_manager - self.config = config - self.logger = custom_logger or logger - self.global_config = global_config or {} - - # Create scroll displays for each game type - self._scroll_displays: Dict[str, ScrollDisplay] = {} - self._current_game_type: Optional[str] = None - - def get_scroll_display(self, game_type: str) -> 'LegacyScrollDisplay': - """ - Get or create a scroll display for a game type. - - Args: - game_type: Type of games ('live', 'recent', 'upcoming') - - Returns: - ScrollDisplay instance for the game type - """ - if game_type not in self._scroll_displays: - self._scroll_displays[game_type] = LegacyScrollDisplay( - self.display_manager, - self.config, - self.logger, - global_config=self.global_config - ) - return self._scroll_displays[game_type] - - def prepare_and_display( - self, - games: List[Dict], - game_type: str, - leagues: List[str], - rankings_cache: Optional[Dict[str, int]] = None - ) -> bool: - """ - Prepare content and start displaying scroll. - - Args: - games: List of game dictionaries - game_type: Type of games - leagues: List of leagues - rankings_cache: Optional team rankings cache - - Returns: - True if scroll was started successfully - """ - scroll_display = self.get_scroll_display(game_type) - - success = scroll_display.prepare_scroll_content( - games, game_type, leagues, rankings_cache - ) - - if success: - self._current_game_type = game_type - - return success - - def display_frame(self, game_type: Optional[str] = None) -> bool: - """ - Display the next frame of the current scroll. - - Args: - game_type: Optional game type (uses current if not specified) - - Returns: - True if a frame was displayed - """ - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return False - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return False - - return scroll_display.display_scroll_frame() - - def is_complete(self, game_type: Optional[str] = None) -> bool: - """Check if the current scroll is complete.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return True - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return True - - return scroll_display.is_scroll_complete() - - def get_dynamic_duration(self, game_type: Optional[str] = None) -> int: - """Get the dynamic duration for the current scroll.""" - if game_type is None: - game_type = self._current_game_type - - if game_type is None: - return 60 - - scroll_display = self._scroll_displays.get(game_type) - if scroll_display is None: - return 60 - - return scroll_display.get_dynamic_duration() - - def get_all_vegas_content_items(self) -> list: - """Collect _vegas_content_items from all scroll displays.""" - items = [] - for sd in self._scroll_displays.values(): - vegas_items = getattr(sd, '_vegas_content_items', None) - if vegas_items: - items.extend(vegas_items) - return items - - def clear_all(self) -> None: - """Clear all scroll displays.""" - for scroll_display in self._scroll_displays.values(): - scroll_display.clear() - self._current_game_type = None - -logger = logging.getLogger(__name__) - _USING_CORE_SCROLL = False try: from src.common.sports_scroll import ( diff --git a/scripts/check_scroll_adoption.py b/scripts/check_scroll_adoption.py new file mode 100644 index 00000000..4ce3821a --- /dev/null +++ b/scripts/check_scroll_adoption.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +"""A plugin's `scroll_display.py` must not define the fallback implementation. + +Plugins that adopted the core scroll orchestration keep two implementations: + + scroll_display.py prefers the core's src.common.sports_scroll, and + falls back to the module below on an older core + scroll_display_legacy.py the frozen previous implementation + +Three plugins ended up as those two files CONCATENATED rather than one +replacing the other, so `scroll_display.py` also carried a full copy of the +legacy classes at module level. Nothing referenced them -- the fallback branch +imports the real ones from `scroll_display_legacy` -- so they were invisible +dead weight, ~2,000 lines of it. + +That is not just untidy. The separator-icon constants whose absence broke +scroll mode on a 3.2.0 core were sitting in that dead block, which is why the +file read as correct both to a reviewer and to an AST checker that only asked +whether the names were defined *somewhere* in the module. Keeping the file down +to one implementation is what makes the next such miss visible. + +Run: python scripts/check_scroll_adoption.py [plugin-id ...] +Exit code 0 when clean, 1 when a plugin inlines a legacy class. +""" + +import ast +import sys +from pathlib import Path + +PLUGINS_DIR = Path(__file__).resolve().parent.parent / "plugins" + + +def offending_classes(path: Path) -> list[str]: + """Module-level classes named Legacy* — the ones that do not belong here. + + Only module level: the adopted file legitimately defines `ScrollDisplay` + and `ScrollDisplayManager` inside the `else:` branch of the guarded import, + and the fallback branch legitimately *imports* the Legacy names. Defining + them here is what signals the duplicate. + """ + try: + tree = ast.parse(path.read_text(encoding="utf-8")) + except SyntaxError as exc: # pragma: no cover - a broken file fails elsewhere + print(f" {path}: could not parse ({exc})") + return [] + return [n.name for n in tree.body + if isinstance(n, ast.ClassDef) and n.name.startswith("Legacy")] + + +def main(argv: list[str]) -> int: + ids = argv or sorted(p.name for p in PLUGINS_DIR.iterdir() if p.is_dir()) + + checked = 0 + problems: list[tuple[str, list[str]]] = [] + for pid in ids: + scroll = PLUGINS_DIR / pid / "scroll_display.py" + if not scroll.exists(): + continue + checked += 1 + found = offending_classes(scroll) + if found: + problems.append((pid, found)) + + for pid, names in problems: + print(f"::error::{pid}/scroll_display.py defines {', '.join(names)} at " + f"module level. The fallback implementation belongs in " + f"scroll_display_legacy.py; this file should only prefer the core " + f"module and fall back to it.") + + if problems: + print(f"\nFAIL: {len(problems)} of {checked} plugin(s) inline a legacy " + f"scroll implementation.") + return 1 + + print(f"OK: {checked} plugin(s) with a scroll_display.py, none inlining a " + f"legacy implementation.") + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) From f4eaf351331f24860c4400889b93d2500c86a7de Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 15:44:55 -0400 Subject: [PATCH 2/2] fix(scripts): address review on #252 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `module-collisions.yml` did not list itself in `pull_request.paths`, so a PR that only edited the workflow matched no path and the workflow never ran against its own change. `check_scroll_adoption.py` turned a `SyntaxError` into an empty result, so `main()` reported the plugin clean and exited 0 — a malformed `scroll_display.py` could skip the check entirely. Parse and read failures now propagate and fail the run with their own error line. The check also only inspected direct members of `tree.body`. A `Legacy*` class inside a module-level `if`/`try`/`with`/loop/`match` still binds a module global, and the guarded import in these very files is an `if/else` — so the likeliest hiding place was the one place not being looked at. It now walks every statement that executes in module scope, without descending into function or class bodies, which stay legitimate. Adds the gate's own regression suite, following `test_check_manifest_version_fields.py`. This gate reports by absence — "no legacy classes found" and "could not look" are otherwise the same answer — so a gate that quietly stopped detecting would still exit 0. Twelve cases cover each module-scope block type, the allowed nestings, the fallback branch's legitimate import, and the malformed file. The three bumped manifests kept `last_updated: 2026-07-31` while their new `versions[0]` entries are released `2026-08-05`, and `plugins.json` mirrored the stale date rather than causing it. Fixed at the source and regenerated the registry. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .github/workflows/module-collisions.yml | 10 ++ plugins.json | 6 +- plugins/basketball-scoreboard/manifest.json | 2 +- plugins/hockey-scoreboard/manifest.json | 2 +- plugins/lacrosse-scoreboard/manifest.json | 2 +- scripts/check_scroll_adoption.py | 68 +++++++++-- scripts/test_check_scroll_adoption.py | 129 ++++++++++++++++++++ 7 files changed, 200 insertions(+), 19 deletions(-) create mode 100644 scripts/test_check_scroll_adoption.py diff --git a/.github/workflows/module-collisions.yml b/.github/workflows/module-collisions.yml index 8ed9a282..8c697669 100644 --- a/.github/workflows/module-collisions.yml +++ b/.github/workflows/module-collisions.yml @@ -22,6 +22,10 @@ on: - 'plugins/**' - 'scripts/check_module_collisions.py' - 'scripts/check_scroll_adoption.py' + - 'scripts/test_check_scroll_adoption.py' + # Without this, a PR that only edits this workflow matches no path and + # the workflow never runs against its own change. + - '.github/workflows/module-collisions.yml' workflow_dispatch: jobs: @@ -38,3 +42,9 @@ jobs: - name: Check scroll adoption does not inline the fallback if: always() run: python scripts/check_scroll_adoption.py + # The gate's own regression suite. It reports by absence -- "no legacy + # classes found" and "could not look" would otherwise be the same + # answer -- so a gate that quietly stopped detecting still exits 0. + - name: Test the scroll-adoption gate + if: always() + run: python scripts/test_check_scroll_adoption.py diff --git a/plugins.json b/plugins.json index 62e9eb53..0a5353cd 100644 --- a/plugins.json +++ b/plugins.json @@ -98,7 +98,7 @@ "plugin_path": "plugins/basketball-scoreboard", "stars": 0, "downloads": 0, - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "verified": true, "screenshot": "", "latest_version": "1.10.2" @@ -332,7 +332,7 @@ "plugin_path": "plugins/hockey-scoreboard", "stars": 0, "downloads": 0, - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "verified": true, "screenshot": "", "latest_version": "1.7.2", @@ -356,7 +356,7 @@ "plugin_path": "plugins/lacrosse-scoreboard", "stars": 0, "downloads": 0, - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "verified": true, "screenshot": "", "latest_version": "1.7.2", diff --git a/plugins/basketball-scoreboard/manifest.json b/plugins/basketball-scoreboard/manifest.json index 4b95cd20..73de8001 100644 --- a/plugins/basketball-scoreboard/manifest.json +++ b/plugins/basketball-scoreboard/manifest.json @@ -144,7 +144,7 @@ ], "stars": 0, "downloads": 0, - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "verified": true, "screenshot": "", "display_modes": [ diff --git a/plugins/hockey-scoreboard/manifest.json b/plugins/hockey-scoreboard/manifest.json index 5ceab7a6..50a778ca 100644 --- a/plugins/hockey-scoreboard/manifest.json +++ b/plugins/hockey-scoreboard/manifest.json @@ -198,7 +198,7 @@ "ledmatrix_min_version": "2.0.0" } ], - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "stars": 0, "downloads": 0, "verified": true, diff --git a/plugins/lacrosse-scoreboard/manifest.json b/plugins/lacrosse-scoreboard/manifest.json index b1365c91..c6626b6e 100644 --- a/plugins/lacrosse-scoreboard/manifest.json +++ b/plugins/lacrosse-scoreboard/manifest.json @@ -138,7 +138,7 @@ "ledmatrix_min_version": "2.0.0" } ], - "last_updated": "2026-07-31", + "last_updated": "2026-08-05", "stars": 0, "downloads": 0, "verified": true, diff --git a/scripts/check_scroll_adoption.py b/scripts/check_scroll_adoption.py index 4ce3821a..52032b9e 100644 --- a/scripts/check_scroll_adoption.py +++ b/scripts/check_scroll_adoption.py @@ -30,21 +30,50 @@ PLUGINS_DIR = Path(__file__).resolve().parent.parent / "plugins" +# Statement types whose bodies still execute in module scope, so a class +# defined inside one is still a module global. `ast.FunctionDef` and +# `ast.ClassDef` are deliberately absent: a Legacy* class nested in either is +# not a module-level binding and is not what this check is looking for. +_MODULE_SCOPE_BLOCKS = ( + ast.If, ast.Try, ast.With, ast.AsyncWith, ast.For, ast.AsyncFor, ast.While, +) +_MATCH = getattr(ast, "Match", None) # 3.10+ + + +def _module_scope_statements(body: list[ast.stmt]): + """Yield every statement that executes in module scope, blocks included. + + The guarded import in these files is an `if/else`, so a legacy class + tucked into either branch — or into a `try` that swallows ImportError — + binds a module global exactly like a top-level one does. + """ + for node in body: + yield node + if isinstance(node, _MODULE_SCOPE_BLOCKS): + yield from _module_scope_statements(node.body) + yield from _module_scope_statements(getattr(node, "orelse", [])) + yield from _module_scope_statements(getattr(node, "finalbody", [])) + for handler in getattr(node, "handlers", []): + yield from _module_scope_statements(handler.body) + elif _MATCH is not None and isinstance(node, _MATCH): + for case in node.cases: + yield from _module_scope_statements(case.body) + + def offending_classes(path: Path) -> list[str]: - """Module-level classes named Legacy* — the ones that do not belong here. + """Module-scope classes named Legacy* — the ones that do not belong here. - Only module level: the adopted file legitimately defines `ScrollDisplay` + Only module scope: the adopted file legitimately defines `ScrollDisplay` and `ScrollDisplayManager` inside the `else:` branch of the guarded import, and the fallback branch legitimately *imports* the Legacy names. Defining them here is what signals the duplicate. + + Raises SyntaxError/OSError to the caller: a file this check cannot read is + not a file it can clear. """ - try: - tree = ast.parse(path.read_text(encoding="utf-8")) - except SyntaxError as exc: # pragma: no cover - a broken file fails elsewhere - print(f" {path}: could not parse ({exc})") - return [] - return [n.name for n in tree.body - if isinstance(n, ast.ClassDef) and n.name.startswith("Legacy")] + tree = ast.parse(path.read_text(encoding="utf-8")) + return sorted(n.name for n in _module_scope_statements(tree.body) + if isinstance(n, ast.ClassDef) and n.name.startswith("Legacy")) def main(argv: list[str]) -> int: @@ -52,12 +81,17 @@ def main(argv: list[str]) -> int: checked = 0 problems: list[tuple[str, list[str]]] = [] + unreadable: list[tuple[str, str]] = [] for pid in ids: scroll = PLUGINS_DIR / pid / "scroll_display.py" if not scroll.exists(): continue checked += 1 - found = offending_classes(scroll) + try: + found = offending_classes(scroll) + except (SyntaxError, ValueError, OSError) as exc: + unreadable.append((pid, str(exc))) + continue if found: problems.append((pid, found)) @@ -67,9 +101,17 @@ def main(argv: list[str]) -> int: f"scroll_display_legacy.py; this file should only prefer the core " f"module and fall back to it.") - if problems: - print(f"\nFAIL: {len(problems)} of {checked} plugin(s) inline a legacy " - f"scroll implementation.") + for pid, reason in unreadable: + print(f"::error::{pid}/scroll_display.py could not be parsed ({reason}). " + f"Treating that as a pass would let a malformed file skip this " + f"check entirely.") + + if problems or unreadable: + if problems: + print(f"\nFAIL: {len(problems)} of {checked} plugin(s) inline a legacy " + f"scroll implementation.") + if unreadable: + print(f"FAIL: {len(unreadable)} of {checked} plugin(s) could not be parsed.") return 1 print(f"OK: {checked} plugin(s) with a scroll_display.py, none inlining a " diff --git a/scripts/test_check_scroll_adoption.py b/scripts/test_check_scroll_adoption.py new file mode 100644 index 00000000..4b2053fe --- /dev/null +++ b/scripts/test_check_scroll_adoption.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +"""Regression tests for the scroll-adoption gate. + +The gate exists because three plugins shipped `scroll_display.py` as the +pre-adoption and adopted files concatenated, leaving a second copy of the +fallback classes nobody referenced. That dead block is where the missing +separator-icon constants were hiding, which is why the file read as correct to +both a reviewer and to a checker that only asked whether a name was defined +*somewhere*. + +So these pin the two ways the gate could quietly stop working: + +- **Scope, not nesting depth.** A `Legacy*` class inside a module-level + `if`/`try`/`with`/loop still binds a module global. The guarded import in + these very files is an `if/else`, so the likeliest hiding place is inside a + block — checking only `tree.body` would miss exactly the case that matters. + Classes nested in a function or another class are *not* module globals and + must stay allowed, as must the fallback branch's legitimate *import* of the + Legacy names. +- **A file that cannot be parsed is not a file that passed.** Swallowing + `SyntaxError` and returning no findings would let a malformed + `scroll_display.py` skip the check and exit 0. + +Exit codes follow the convention in `run_plugin_tests.py`: 0 pass, 1 fail. +""" + +import sys +import tempfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import check_scroll_adoption as gate # noqa: E402 + +GUARDED_IMPORT = ( + "try:\n" + " from src.common.sports_scroll import ScrollDisplay\n" + "except ImportError:\n" + " class LegacyScrollDisplay:\n pass\n" +) + +# name -> (source, expected offending class names) +CASES = { + "module level": ( + "class LegacyScrollDisplay:\n pass\n", + ["LegacyScrollDisplay"], + ), + "inside the guarded import's except branch": ( + GUARDED_IMPORT, + ["LegacyScrollDisplay"], + ), + "inside an if body": ( + "if True:\n class LegacyScrollDisplayManager:\n pass\n", + ["LegacyScrollDisplayManager"], + ), + "inside an else branch": ( + "if False:\n pass\nelse:\n class LegacyScrollDisplay:\n pass\n", + ["LegacyScrollDisplay"], + ), + "inside a loop": ( + "for _ in range(1):\n class LegacyThing:\n pass\n", + ["LegacyThing"], + ), + "inside a with block": ( + "with open(__file__) as fh:\n class LegacyThing:\n pass\n", + ["LegacyThing"], + ), + "two in one file, reported sorted": ( + "class LegacyScrollDisplayManager:\n pass\n" + "class LegacyScrollDisplay:\n pass\n", + ["LegacyScrollDisplay", "LegacyScrollDisplayManager"], + ), + # Allowed: not module globals, or not definitions at all. + "nested in a function": ( + "def make():\n" + " class LegacyScrollDisplay:\n pass\n" + " return LegacyScrollDisplay\n", + [], + ), + "nested in a class": ( + "class Outer:\n class LegacyInner:\n pass\n", + [], + ), + "imported, not defined": ( + "from scroll_display_legacy import LegacyScrollDisplay\n", + [], + ), + "adopted file's own non-Legacy classes": ( + "class ScrollDisplay:\n pass\n\nclass ScrollDisplayManager:\n pass\n", + [], + ), +} + + +def main() -> int: + failures: list[str] = [] + + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "scroll_display.py" + + for name, (source, expected) in CASES.items(): + path.write_text(source, encoding="utf-8") + actual = gate.offending_classes(path) + if actual == expected: + print(f" ok {name}") + else: + print(f" FAIL {name}: expected {expected}, got {actual}") + failures.append(f"{name}: expected {expected}, got {actual}") + + path.write_text("class Legacy(:\n", encoding="utf-8") + try: + actual = gate.offending_classes(path) + except SyntaxError: + print(" ok malformed file raises instead of reporting clean") + else: + print(f" FAIL malformed file returned {actual} instead of raising") + failures.append(f"malformed file returned {actual} instead of raising") + + print() + if failures: + print(f"{len(failures)} failure(s):", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + print(f"All {len(CASES) + 1} cases passed.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())