Files
LEDMatrix/docs/SPORTS_UNIFICATION.md
T
Claude aaabc614cc feat(sports): promote the nine universal methods into the base classes
Phase B1b of docs/SPORTS_UNIFICATION.md. Every method here is present in
all nine bundled plugin sports.py copies and absent from core, so this is
reuse of code the fleet already agreed on — not new behavior. The
promotions are inert until B5: the plugins' own overrides still run.

SportsCore: cleanup, _get_layout_offset, _load_custom_font_from_element_config
SportsUpcoming: _select_games_for_display
SportsRecent: _get_zero_clock_duration, _clear_zero_clock_tracking,
              _select_recent_games_for_display
SportsLive: _is_game_really_over, _detect_stale_games

Where the copies disagreed, the canonical form was chosen on evidence and
the genuine per-sport differences became seams rather than branches:

- _favorite_key(game, side) -- NRL matches favorites on team id because its
  abbreviations are ambiguous (NEW is both Newcastle Knights and New
  Zealand Warriors). Default is the abbreviation; NRL overrides. Core never
  learns the string nrl.
- FINAL_PERIOD / CLOCK_COUNTS_DOWN -- hockey ends in P3, and soccer/afl/nrl
  clocks count UP, so 0:00 means kickoff, not expiry.
- _config_schema_path() / _font_root() -- plugin-supplied locations, never
  derived from this module's __file__.

BEHAVIOR CHANGE (baseball, ufc): the rejected variant coerced a missing or
non-str clock to the literal 0:00 and then declared the game over at
period >= 4. MLB has no game clock and period is the inning, so live games
were being evicted from the 5th inning onward; UFC likewise. The promoted
variant skips the clock check when the clock is unusable -- it fails safe
(keeps showing the game) instead of failing destructive.

Also fixes a regression from the package move in e591cec: the bodies were
byte-identical but __file__ gained a directory, so _resolve_project_path's
parents[2] silently began resolving to <root>/src instead of the repo root.
Both it and _font_root now derive from a single _INSTALL_ROOT constant, so
a future move needs one line changed rather than two hand-counted depths.
Tests assert the resolved values, not the index.

The font loader takes baseball's body (BDF memo cache + native-strike
retry) under hockey's Optional signature -- the older lineage is the
correct one here, and basketball's positional str default breaks on an
explicit None. It resolves through _font_root rather than the cwd, so it
does not reintroduce the bug just fixed for FontManager, and delegates to
FontManager for the alias table and BDF header parse instead of shipping
second copies. cleanup gained the two new font caches and still leaves
background_service alone -- it is a process-wide singleton.

Verified: 111 new tests (48 core + 59 modes + 4 install-root regression);
characterization + skin suites still exactly 94, unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4
2026-08-01 16:23:41 +00:00

145 lines
8.3 KiB
Markdown

# Sports Code Unification — Architecture
How the nine sports scoreboard plugins converge onto shared core code **without**
becoming nine clients of a god class.
## The problem
Nine plugins (`afl`, `baseball`, `basketball`, `football`, `hockey`, `lacrosse`,
`nrl`, `soccer`, `ufc`) each ship a ~3,000-line `sports.py` descended from this
repo's `src/base_classes/sports.py`. They have drifted into three lineages, and
only 28 of the 66 methods appearing across them are present in all nine. One
logical fix (the UTC start-time bug) cost 75 files.
Merging everything into one base class would fix the duplication and create a
worse problem: a single 2,500-line class that all nine plugins inherit, where any
change has a nine-plugin blast radius and per-sport behavior survives only as
`if self.sport == "hockey"` branches.
## Three properties, three mechanisms
These are independent concerns. Conflating them is what produces god classes.
### Upgradability — a plugin keeps working across core versions
| Rule | Mechanism |
|---|---|
| Plugin loads on a core that predates a module | Guarded import with a bundled fallback (`try: from src.X import Y / except ModuleNotFoundError: from y import Y`) |
| Plugin loads on a core that predates a *method* | Capability probing — `hasattr(SportsCore, "_detect_stale_games")` — never a version comparison. The loader's compat check is advisory-only (it logs and continues), so probing is the real protection. |
| Core changes never break a plugin's rendering | The **view-model contract**: `_extract_game_details_common` returns a dict whose `GUARANTEED_KEYS` are frozen by `test/test_skin_system.py::TestViewModelContract`. Keys may be added, never renamed or removed. |
| A plugin can drop its bundled copy safely | The **sunset rule**: only when its manifest floors `ledmatrix_min_version` at the first core release shipping the module (recorded in `CHANGELOG.md`). |
The core API is **additive-only**. A method the plugins call is never removed or
given a new required parameter; new behavior arrives as new methods with
defaults, or as capabilities they opt into.
### Reusability — write once, nine plugins benefit
Only code that is **identical in intent across all nine** moves into the base
class. That set is small and knowable — it is exactly the methods present in every
copy today (phase B1 below). Everything else stays where it is until it earns
promotion.
### Modularity — a change to one feature cannot reach a plugin that doesn't use it
This is the property the naive merge destroys, and it is enforced structurally:
1. **Capabilities are separate modules composed by inheritance, not config
branches inside the base class.** Hockey has no celebrations, so
`HockeyLive` does not inherit `CelebrationMixin` — the celebration code is not
merely disabled for hockey, it is *not in hockey's MRO at all*. No shared
state, no dead branches, no risk. Contrast with
`if self.celebrations_enabled:` inside `SportsLive`, where a bug in
celebration code can still crash a plugin that never wanted the feature.
2. **Variant behavior is a strategy object chosen by name, not a branch.**
Live rotation exists in three dialects across the lineages; core ships all
three behind `rotation_strategy: "swrr" | "weighted" | "simple"` and a plugin
may register its own. Core never learns sport names.
3. **Sport-specific behavior is a documented override point.** The base class
declares the seam; the plugin fills it. Basketball's tournament-round parsing
and baseball's BDF sizing stay in their plugins forever — they are not
candidates for promotion, and core must never grow a branch for them.
4. **Files bound the blast radius.** Capabilities live in their own modules so a
diff shows at a glance which plugins a change can reach.
## Layering
```
src/base_classes/sports/
__init__.py re-exports the public API (import path unchanged)
core.py SportsCore — fetch, cache, config, logos, fonts, odds,
view-model extraction, the skin seam
modes.py SportsUpcoming / SportsRecent / SportsLive
capabilities/
celebrations.py CelebrationMixin (opt-in: 4 of 9 plugins)
rotation.py RotationStrategy + registry
```
`from src.base_classes.sports import SportsCore` keeps working — the package
`__init__` re-exports, so the conversion is invisible to every existing importer.
## Override points (the plugin-facing seam)
The base class calls these; plugins implement or override them. This table is the
contract — additions require a default implementation, removals require a
deprecation cycle.
| Hook | Purpose | Default |
|---|---|---|
| `_fetch_data()` | Sport's schedule source | abstract |
| `_extract_game_details(event)` | Sport-specific view-model fields on top of the common ones | delegates to `_extract_game_details_common` |
| `_draw_scorebug_layout(game, force_clear)` | Sport's card rendering | base layout |
| `_custom_scorebug_layout(game, draw)` | Per-sport overlay on the base layout | no-op |
| `render_skin_card(game, size)` | Skin-system entry point | built-in fallback |
| `score_phrase(game)` | Celebration wording (`"GOAL"` vs `"TOUCHDOWN"`) | `"SCORE"` — only consulted when `CelebrationMixin` is present |
| `_favorite_key(game, side)` | Which view-model field identifies a team for favorites matching | `game["<side>_abbr"]` |
| `_config_schema_path()` | Plugin's `config_schema.json` — returning it routes `_get_layout_offset` through the `src.element_style` resolver (and gives it the defaults to compare against) | `None`, i.e. the classic inline `customization.layout` read |
| `_font_root()` | Directory to resolve `assets/fonts` against | core install root |
Two class attributes serve the same purpose for values that are per-sport
constants rather than behavior:
| Attribute | Meaning | Default |
|---|---|---|
| `FINAL_PERIOD` | Period at/after which a zero clock can mean "over" | `4` (hockey overrides to `3`) |
| `CLOCK_COUNTS_DOWN` | Whether `0:00` means "expired" | `True` (soccer/afl/nrl override to `False` — their clocks count up, so `0:00` is kickoff) |
### Why these are seams and not branches
`_favorite_key` exists because NRL abbreviations are **not unique** — "NEW" is both
Newcastle Knights and New Zealand Warriors, "CAN" both Canberra and Canterbury —
so NRL matches favorites on team ID. Flattening every plugin to abbreviations
would silently select the wrong club for NRL users. The base declares the seam,
NRL fills it, and core never learns the string `"nrl"`.
`CLOCK_COUNTS_DOWN` exists for the same reason in the opposite direction: a
soccer clock reading `0:00` means the match has not kicked off, so running the
clock-expiry branch there would evict live games.
## Phases
| Phase | Scope | Risk control |
|---|---|---|
| **B0** ✅ | Characterization tests, CI unit job, `element_style`, font cwd fix, CHANGELOG discipline | — |
| **B1** | Promote the nine universal methods; convert `sports.py` → package | Characterization suite must stay green; no behavior change intended |
| **B2** | `CelebrationMixin` + rotation strategies as opt-in capabilities | Plugins that don't opt in have zero new code in their MRO |
| **B3** | Upstream `ScrollDisplay` as `src/common/sports_scroll.py`, reading `global_config['target_fps']` natively | Plugin copies remain until sunset |
| **B4** | Bump to 3.2.0, record modules in CHANGELOG, migrate `ledmatrix_min``ledmatrix_min_version` | Gives plugins a version to floor on |
| **B5** | Pilot one plugin per lineage (hockey, soccer, football) on guarded core imports; then the remaining six; then delete bundled copies | Pilot soaks before rollout; harness + golden suites gate each |
## Rules for contributors
- **Promote on evidence, not intuition.** A method moves to core when every copy
has it and they agree on intent. Otherwise it stays in the plugins.
- **Never add a sport name to core.** If core needs to know which sport it is,
the design is wrong — add an override point instead.
- **A capability that is not opted into must not execute.** If you find yourself
writing `if self.<capability>_enabled` inside a base class, it belongs in a
mixin.
- **Touch the view-model keys only additively.** Published skins depend on them.
- **Every promotion lands with the characterization suite green**, and every
pilot adoption lands with that plugin's harness and golden suites green.