fix(basketball): fix records display and reduce no-live-games log noise#17
Conversation
- When both show_ranking and show_records are enabled, fall back to showing the team record if no ranking is available (previously showed nothing for unranked teams) - Lower "no game data available" log from WARNING to DEBUG since the display controller already skips modes with no content - Fix GameRenderer config reading to find show_records/show_ranking from per-league display_options instead of a nonexistent defaults key - Fix GameRenderer to use flat game dict format (home_abbr, home_score, home_record) matching what sports.py produces instead of nested home_team/away_team dicts - Fix scroll_display game type detection to use is_live/is_final/ is_upcoming flags instead of nested status.state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe basketball scoreboard plugin is refactored to use flat game dictionary fields instead of nested home_team/away_team structures. Additionally, when both rankings and records display options are enabled, the system now falls back to displaying team records when ranking data is unavailable, applied consistently across rendering logic. Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/basketball-scoreboard/sports.py (2)
1-31:⚠️ Potential issue | 🟡 MinorBump version from 1.0.5 to 1.0.6 in
plugins/basketball-scoreboard/manifest.json.Code changes to plugin files require a version bump per semantic versioning guidelines. Update the
versionfield and add a new entry to theversionsarray with the bumped version and today's release date. For bug fixes, use PATCH version bump (1.0.5 → 1.0.6).
2217-2228:⚠️ Potential issue | 🔴 CriticalFix critical indentation bug: live game data fetch is incorrectly nested inside
if self.show_ranking:Lines 2222–2412 (all live game fetching and processing) are indented under
if self.show_ranking:(line 2218). Whenshow_rankingisFalse,_fetch_data()is never called, so live games will never appear. De-indent the entiredata = self._fetch_data()block through line 2412 by one level to align with theif self.show_ranking:block.Proposed fix
if self.show_ranking: self._fetch_team_rankings() # Fetch live game data - data = self._fetch_data() - new_live_games = [] + data = self._fetch_data() + new_live_games = []The entire block from
data = self._fetch_data()(line 2222) through the end of live-game processing (around line 2412) needs to be de-indented by one level.
🤖 Fix all issues with AI agents
In `@plugins/basketball-scoreboard/game_renderer.py`:
- Around line 329-330: The current conversion uses str(game.get("home_score",
"0")) which still yields "None" when the key exists but its value is None;
update the handling in game_renderer.py so you first retrieve the raw value
(e.g., val = game.get("home_score")) and then set home_score = str(val) if val
is not None else "0" (and do the same for away_score) to ensure explicit None
values render as "0" instead of "None".
🧹 Nitpick comments (1)
plugins/basketball-scoreboard/sports.py (1)
1486-1526: Consider extracting the duplicated ranking/record fallback logic into a shared helper.The same ~20 lines of
if self.show_ranking and self.show_records: ... elif ... elif ... else ...logic is repeated verbatim in:
SportsUpcoming._draw_scorebug_layout(away + home)SportsRecent._draw_scorebug_layout(away + home)BasketballLive._draw_scorebug_layoutinbasketball.py(away + home)
GameRenderer._get_team_display_textalready encapsulates this — a similar helper onSportsCorecould eliminate the duplication.Also applies to: 2005-2045
| home_score = str(game.get("home_score", "0")) | ||
| away_score = str(game.get("away_score", "0")) |
There was a problem hiding this comment.
Minor: str(None) produces "None" if score value is explicitly None.
game.get("home_score", "0") returns None (not "0") when the key exists with a None value, so str(None) → "None" would render on screen. In practice sports.py always normalizes scores to strings, but a small guard would improve robustness:
Proposed fix
- home_score = str(game.get("home_score", "0"))
- away_score = str(game.get("away_score", "0"))
+ home_score = str(game.get("home_score") or "0")
+ away_score = str(game.get("away_score") or "0")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| home_score = str(game.get("home_score", "0")) | |
| away_score = str(game.get("away_score", "0")) | |
| home_score = str(game.get("home_score") or "0") | |
| away_score = str(game.get("away_score") or "0") |
🤖 Prompt for AI Agents
In `@plugins/basketball-scoreboard/game_renderer.py` around lines 329 - 330, The
current conversion uses str(game.get("home_score", "0")) which still yields
"None" when the key exists but its value is None; update the handling in
game_renderer.py so you first retrieve the raw value (e.g., val =
game.get("home_score")) and then set home_score = str(val) if val is not None
else "0" (and do the same for away_score) to ensure explicit None values render
as "0" instead of "None".
Fixes ChuckBuilds/LEDMatrix#302 - users couldn't add por.1 or other leagues because: 1. Only 8 leagues were hardcoded in the schema with additionalProperties:false 2. The custom_leagues array-table (which works) was buried at position #17 in a 384px scrollable container, below 8 collapsed league sections - users never found it Changes: - Add por.1 (Liga Portugal) as a 9th predefined league with full config schema, factory function, and manager initialization - Add x-propertyOrder to place "Add More Leagues" directly above the predefined "Leagues" section so users see it - Rename custom_leagues from "Custom Leagues" to "Add More Leagues" with clearer description listing common ESPN league codes - Bump version to 1.5.3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(soccer): add Liga Portugal and make custom leagues discoverable Fixes ChuckBuilds/LEDMatrix#302 - users couldn't add por.1 or other leagues because: 1. Only 8 leagues were hardcoded in the schema with additionalProperties:false 2. The custom_leagues array-table (which works) was buried at position #17 in a 384px scrollable container, below 8 collapsed league sections - users never found it Changes: - Add por.1 (Liga Portugal) as a 9th predefined league with full config schema, factory function, and manager initialization - Add x-propertyOrder to place "Add More Leagues" directly above the predefined "Leagues" section so users see it - Rename custom_leagues from "Custom Leagues" to "Add More Leagues" with clearer description listing common ESPN league codes - Bump version to 1.5.3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(soccer): resolve custom league config path mismatch and review findings Runtime functions (_get_enabled_leagues_for_mode, _parse_display_mode_settings, _get_available_modes, supports_dynamic_duration, get_dynamic_duration_cap) were reading custom leagues from config["leagues"]["custom_leagues"] while the initializer and schema define them at config["custom_leagues"]. Extract shared _get_league_config() helper to consistently resolve league config for both predefined and custom leagues. Also: add missing 'por.1' -> 'Liga Portugal' to LEAGUE_NAMES, relax league_code validation pattern to accept codes like 'uefa.champions', and bump version to 1.6.0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(soccer): update stale get_info version and cache custom league lookups Update get_info() version from "1.4.0" to "1.6.0" to match the current release. Also replace O(n) linear scan in _get_league_config with an O(1) dict lookup via _custom_league_map, rebuilt on init and config reload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements