fix(football): NCAA and NFL teams sharing an abbreviation swap logos - #472
Merged
Conversation
NCAA and NFL share ten abbreviations -- CAR CIN DAL DEN HOU LAC MIA NE TB
TBD -- and their logos live in different directories. ncaa_logos/MIA.png
is the Hurricanes; nfl_logos/MIA.png is the Dolphins.
_logo_cache_key scopes by logo slot size only, and one _logo_cache is
shared by every renderer this plugin builds: manager.py creates a single
ScrollDisplayManager, and prepare_scroll_content takes `leagues` plural,
so one strip carries both. Both leagues therefore resolved "MIA" to
MIA@40x64 and whichever rendered first won the slot.
Scoped on the logo directory rather than the league string. That is what
actually decides which file gets opened, it is already in hand wherever a
logo is loaded, and two teams sharing an abbreviation within one
directory really are the same team -- so the cache does not fragment and
re-decode the same PNG per card. basketball-scoreboard and
lacrosse-scoreboard already prefix the name for this reason
(f"{league}:{abbr}" / f"{league}_{abbr}"); this follows them.
test_logo_cache_league_scope.py loads two real files through
_load_and_resize_logo and checks the colour that comes back. Against the
unfixed code:
FAIL the NCAA card draws the NCAA logo, not the NFL one
drew (0, 133, 122), expected (240, 130, 0)
FAIL the two leagues occupy separate cache entries
1 entry: ['MIA@40x64']
Scope, for the record. This is the scroll and Vegas path. I could not
reproduce it in switch mode: that path goes through sports.py, whose
_logo_cache is per-league-manager, and on a rig I checked end to end the
directories resolve correctly (ncaa_fb -> ncaa_logos, nfl -> nfl_logos)
and both MIA.png files are the right badge. A switch-mode sighting is
more likely a wrong file on that device than this bug.
42 plugin tests pass, harness clean at all 8 sizes, repo guards pass.
Manifest 3.5.3 -> 3.5.4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9Contributor
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 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 |
| Metric | Results |
|---|---|
| Complexity | 20 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewerTIP This summary will be updated as you push new changes.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Reported: the NCAA Football Recent card for Miami vs Stanford drew the Dolphins badge instead of the Hurricanes.
Cause
NCAA and NFL share ten abbreviations, and their logos live in different directories:
ncaa_logos/MIA.pngis the Hurricanes.nfl_logos/MIA.pngis the Dolphins. Both correct on disk — I pulled and looked at them._logo_cache_keyscopes by logo slot size only:and one
_logo_cacheis shared by every renderer this plugin builds —manager.pycreates a singleScrollDisplayManager, andprepare_scroll_content(games, game_type, leagues, …)takes leagues plural, so one strip carries both. Both leagues resolvedMIAtoMIA@40x64, and whichever rendered first won the slot.Fix
Scoped on the logo directory rather than the league string. That is what actually decides which file gets opened, it is already in hand wherever a logo is loaded, and two teams sharing an abbreviation within one directory really are the same team — so the cache does not fragment and re-decode the same PNG per card.
basketball-scoreboardandlacrosse-scoreboardalready prefix the name for this reason (f"{league}:{abbr}"andf"{league}_{abbr}"); this follows them rather than inventing a third convention.nrlandaflare single-league, and everysoccer_*league maps to onesoccer_logosdirectory, so neither collides.Test
test_logo_cache_league_scope.pywrites two real PNGs in different colours, loads both through the real_load_and_resize_logo, and checks the colour that comes back. Against the unfixed code:It also checks all ten colliding abbreviations, that the slot size is still in the key, that one directory still shares a single entry, and that a missing path does not raise on the render thread.
Scope — please read
This is the scroll and Vegas path. I could not reproduce it in switch mode, which is what the report described.
Switch mode goes through
sports.py, whose_logo_cacheis per-league-manager, so it does not collide. On the rig I checked end to end:So a switch-mode sighting is more likely a wrong file on that particular device than this bug. Worth checking on the affected display:
If it differs, delete it and let the downloader refetch. I could not reach
ledpiordevpito check them.42 plugin tests pass, safety harness clean at all 8 sizes, repo guards pass. Manifest 3.5.3 → 3.5.4.
🤖 Generated with Claude Code
https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9