Skip to content

fix(soccer): use correct ScrollHelper constructor signature#31

Merged
ChuckBuilds merged 1 commit intomainfrom
fix/soccer-scrollhelper-constructor
Feb 16, 2026
Merged

fix(soccer): use correct ScrollHelper constructor signature#31
ChuckBuilds merged 1 commit intomainfrom
fix/soccer-scrollhelper-constructor

Conversation

@ChuckBuilds
Copy link
Copy Markdown
Owner

@ChuckBuilds ChuckBuilds commented Feb 16, 2026

ScrollHelper.init() only accepts (display_width, display_height, logger) but soccer's ScrollDisplay was passing scroll_speed and target_fps kwargs, causing a TypeError. Fix to use the correct 3-arg constructor and configure via setter methods (matching hockey-scoreboard's working pattern).

Also fixes is_scroll_complete() to call the correct method name on ScrollHelper (is_scroll_complete, not is_complete).

Summary by CodeRabbit

  • Refactor
    • Streamlined scroll display configuration and initialization flow for improved maintainability and control.
    • Enhanced scroll behavior settings with improved timing and frame-based calculations.
    • Refined internal configuration logic to better manage scrolling duration and frame control.

ScrollHelper.__init__() only accepts (display_width, display_height, logger)
but soccer's ScrollDisplay was passing scroll_speed and target_fps kwargs,
causing a TypeError. Fix to use the correct 3-arg constructor and configure
via setter methods (matching hockey-scoreboard's working pattern).

Also fixes is_scroll_complete() to call the correct method name on
ScrollHelper (is_scroll_complete, not is_complete).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 16, 2026

📝 Walkthrough

Walkthrough

The changes refactor ScrollHelper initialization from a guarded construction to a simplified positional-argument approach, followed by a dedicated private configuration method that applies scroll settings, converts speed to pixels-per-frame, and enables frame-based scrolling.

Changes

Cohort / File(s) Summary
ScrollHelper Initialization & Configuration
plugins/soccer-scoreboard/scroll_display.py
Replaced guarded ScrollHelper initialization with simplified constructor call; introduced _configure_scroll_helper private method to apply scroll settings and convert speed to pixels-per-frame; extended _get_scroll_settings with new scroll_delay parameter (default 0.01); updated is_scroll_complete to call method on ScrollHelper instance instead of is_complete(); removed try/except wrapper around initialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (25 files):

⚔️ plugins.json (content)
⚔️ plugins/baseball-scoreboard/game_renderer.py (content)
⚔️ plugins/baseball-scoreboard/manager.py (content)
⚔️ plugins/baseball-scoreboard/manifest.json (content)
⚔️ plugins/baseball-scoreboard/scroll_display.py (content)
⚔️ plugins/basketball-scoreboard/game_renderer.py (content)
⚔️ plugins/basketball-scoreboard/manager.py (content)
⚔️ plugins/basketball-scoreboard/manifest.json (content)
⚔️ plugins/basketball-scoreboard/scroll_display.py (content)
⚔️ plugins/football-scoreboard/game_renderer.py (content)
⚔️ plugins/football-scoreboard/manager.py (content)
⚔️ plugins/football-scoreboard/manifest.json (content)
⚔️ plugins/football-scoreboard/scroll_display.py (content)
⚔️ plugins/hockey-scoreboard/game_renderer.py (content)
⚔️ plugins/hockey-scoreboard/manager.py (content)
⚔️ plugins/hockey-scoreboard/manifest.json (content)
⚔️ plugins/hockey-scoreboard/scroll_display.py (content)
⚔️ plugins/ledmatrix-weather/manager.py (content)
⚔️ plugins/ledmatrix-weather/manifest.json (content)
⚔️ plugins/soccer-scoreboard/manager.py (content)
⚔️ plugins/soccer-scoreboard/manifest.json (content)
⚔️ plugins/soccer-scoreboard/scroll_display.py (content)
⚔️ plugins/ufc-scoreboard/manager.py (content)
⚔️ plugins/ufc-scoreboard/manifest.json (content)
⚔️ plugins/ufc-scoreboard/scroll_display.py (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the ScrollHelper constructor call to use the correct signature with positional arguments (display_width, display_height, logger).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/soccer-scrollhelper-constructor
⚔️ Resolve merge conflicts (beta)
  • Auto-commit resolved conflicts to branch fix/soccer-scrollhelper-constructor
  • Create stacked PR with resolved conflicts
  • Post resolved changes as copyable diffs in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
plugins/soccer-scoreboard/scroll_display.py (1)

154-180: Remove the redundant first set_scroll_speed call on line 156.

Line 156 sets scroll_speed (px/s), but line 180 unconditionally overwrites it with pixels_per_frame (px/frame). The first call has no effect and can be removed.

♻️ Suggested refactor
-        # 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_speed = scroll_settings.get('scroll_speed', 50.0)
         scroll_delay = scroll_settings.get('scroll_delay', 0.01)
         self.scroll_helper.set_scroll_delay(scroll_delay)

@ChuckBuilds ChuckBuilds merged commit 52ff4e6 into main Feb 16, 2026
1 check passed
@ChuckBuilds ChuckBuilds deleted the fix/soccer-scrollhelper-constructor branch February 16, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant