Fix baseball scoreboard scroll display mode ignored outside Vegas mode - #201
Merged
Merged
Conversation
_display_league_mode() (the path display() actually uses for granular modes like mlb_live/mlb_recent/mlb_upcoming) never checked the live_display_mode/recent_display_mode/upcoming_display_mode config -- it always fell through to switch/flip behavior via _try_manager_display. The scroll wiring (_should_use_scroll_mode, _display_scroll_mode) only existed on a combined-mode code path that display() never calls for granular modes, so the Scroll setting had no effect. Vegas mode worked because it bypasses display() entirely and calls the scroll manager directly and unconditionally. Add _display_league_scroll_mode(), invoked from _display_league_mode() when the league/mode's display_mode is 'scroll'. It mirrors _display_scroll_mode() but is scoped to a single league, and tracks which league's content is currently prepared per mode_type so rotating between leagues that are both set to scroll (e.g. mlb_live and milb_live) forces a re-prepare instead of reusing stale content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gfy4jbtgTQHjxbFwYqd7wG
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe baseball scoreboard now prepares scroll content per league for granular Live, Recent, and Upcoming modes. It resets active-league state on configuration changes, updates league-specific cycle checks, and documents the fix in version 1.19.2. ChangesBaseball scroll-mode support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DisplayMode as _display_league_mode
participant ScrollMode as _display_league_scroll_mode
participant ScrollManager as ScrollDisplayManager
DisplayMode->>ScrollMode: delegate selected league scroll mode
ScrollMode->>ScrollManager: prepare league games
ScrollMode->>ScrollManager: display_frame
ScrollManager-->>ScrollMode: report is_complete(mode_type)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/baseball-scoreboard/manager.py`:
- Around line 1549-1552: Update the duration and completion checks near the
paths corresponding to lines 2624 and 2912 to call _get_display_mode(league,
mode_type), matching the league-specific routing in _display_league_scroll_mode.
Remove any global “any league uses scroll” classification so each league
receives only its own mode’s scroll state.
- Around line 1696-1709: Update the display_frame failure branch in the scroll
handling flow to also reset self._scroll_prepared[mode_type] to False when
playback returns False. Preserve the existing inactive-state reset and False
return so the next call can prepare and retry the league.
In `@plugins/baseball-scoreboard/manifest.json`:
- Around line 33-38: Update the new release entry in the manifest to version
1.19.2, rename its minimum-version field from ledmatrix_min_version to
ledmatrix_min, and synchronize the top-level version with 1.19.2. Preserve the
required released, version, and ledmatrix_min fields for the entry while keeping
the existing 1.19.1 history intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 63bec854-af5b-4106-bd54-6343ba396a35
📒 Files selected for processing (2)
plugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.json
…board-scroll-mode-yft8sw # Conflicts: # plugins/baseball-scoreboard/manifest.json
…aware duration/completion checks - _display_league_scroll_mode: reset _scroll_prepared and _scroll_active_league (not just _scroll_active) when display_frame() fails, so the next call re-prepares and retries instead of getting stuck in a prepared-but-inactive state. - get_cycle_duration() and is_cycle_complete(): use the granular mode's own _get_display_mode(league, mode_type) instead of _should_use_scroll_mode(mode_type) (which is true if *any* enabled league uses scroll). Previously a league configured for 'switch' could report another league's scroll duration/completion state just because that other league was set to 'scroll' for the same game type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gfy4jbtgTQHjxbFwYqd7wG
…l-mode-yft8sw' into claude/baseball-scoreboard-scroll-mode-yft8sw # Conflicts: # plugins/baseball-scoreboard/manifest.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Baseball Scoreboard's Live/Recent/Upcoming Display Mode setting being ignored when set to "Scroll" outside of Vegas mode. Games kept flipping/switching one at a time instead of scrolling, even though the config was set to scroll.
Type of change
Plugin(s) affected
baseball-scoreboard
Related issues
N/A — reported directly by a user, no linked issue.
Root cause
display()routes every granular per-league mode (mlb_live,mlb_recent,mlb_upcoming,milb_*,ncaa_baseball_*— the only modes registered inmanifest.json) straight to_display_league_mode(). That method never checked thelive_display_mode/recent_display_mode/upcoming_display_modeconfig and always fell through to the switch/flip behavior via_try_manager_display().The existing scroll machinery (
_should_use_scroll_mode,_display_scroll_mode) was only reachable from_display_external_mode()'s "combined mode" branch — a code pathdisplay()never actually calls for granular modes (there's even a self-aware log line admitting this in the code). Vegas mode worked because it bypassesdisplay()entirely and calls the scroll manager directly and unconditionally viaget_vegas_content()→_collect_games_for_scroll(mode_type=None, ...).Fix
Added
_display_league_scroll_mode(), called from_display_league_mode()whenever the league/mode's configured display mode is'scroll'. It mirrors_display_scroll_mode()'s prepare/display-frame/is-complete flow but is scoped to a single league. Since the underlyingScrollDisplayManagerkeeps one active scroll session permode_type(shared across leagues), it also tracks which league's content is currently prepared and forces a re-prepare when that changes (e.g. rotation switches frommlb_livetomilb_liveand both are set to scroll), so leagues don't stomp on each other's scroll state.Test plan
EMULATOR=true python3 run.py)scripts/dev_server.py)python3 -m py_compileandscripts/check_module_collisions.pypass; confirmed every referenced method exists via AST parse. Runtime verification (setting MLB Display Mode to Scroll and observing on-device/emulator behavior) was not performed in this environment — please verify on hardware/emulator before merge.Required for plugin changes
versioninplugins/baseball-scoreboard/manifest.json(1.19.0 → 1.19.1)class_nameunchanged, still matchesmanager.pyentry_pointunchangedREADME.md— no config keys changed, so no update neededconfig_schema.jsonunchanged (no new options — fixing existing scroll config to actually work)plugins.json)Notes for reviewer
The fix intentionally scopes scroll state by
mode_typeonly (matching the existing{game_type: is_active}comment/design in__init__), not by league+mode_type, becauseScrollDisplayManageritself only keeps oneScrollDisplaypergame_type. A per-league scroll_key would silently show stale/wrong-league content when rotation switches leagues. The new_scroll_active_leaguedict detects that case and forces a re-prepare.Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Release