refactor(sports): one reader for element colours and layout offsets

There were two copies of the per-element colour read and three of the
layout-offset read. They had already drifted -- the scroll-card renderer
carries a comment about having ignored offsets its own schema advertised
-- and each new capability had to be added to all of them or silently work
in some places and not others.

All of them now go through src.element_style, which is what carries the
alias handling and the per-mode lookup. That lands immediately for the
nine plugins importing these modules: a scoreboard asking for `score_text`
offsets finds the `layout.score` its users configured, and a Live instance
resolves its own colours through SKIN_MODE without any call site passing a
mode.

_normalize_color learned "#RRGGBB" in the process. The scoreboards' own
readers have always accepted it, so the shared one had to, or consolidating
would have quietly dropped a form users' configs may hold. _coerce_offset
picked up the non-finite guard the scroll-card reader had and the other two
did not.

_get_layout_offset is promoted onto SportsCoreSharedMixin. Each plugin
still carries its own copy in its bundled sports.py, which wins by MRO --
so adopting this is a deletion in the plugin, and until that deletion
nothing changes for it.

Note for whoever runs the suite next: test_display_dirty_tracking.py is
order-dependent. Fifteen of its tests failed in one full run and passed in
the next with no change in between, and pass in isolation. Pre-existing,
unrelated to this, but it makes a full-run diff untrustworthy until it is
fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

src/common/sports_card.py

@@ -99,18 +99,18 @@ def upcoming_center_mode(config: Optional[Dict[str, Any]]) -> str:
 # ---------------------------------------------------------------------------
 
 def element_color(config: Optional[Dict[str, Any]], element: str,
-                  default: Tuple[int, int, int] = (255, 255, 255)):
-    """Per-element text colour from customization.<element>.text_color."""
-    try:
-        cfg = (config or {}).get("customization", {}).get(element, {})
-        value = cfg.get("text_color")
-        if isinstance(value, (list, tuple)) and len(value) == 3:
-            return tuple(max(0, min(255, int(c))) for c in value)
-        if isinstance(value, str) and value.startswith("#") and len(value) == 7:
-            return tuple(int(value[i:i + 2], 16) for i in (1, 3, 5))
-    except (TypeError, ValueError):
-        pass
-    return default
+                  default: Tuple[int, int, int] = (255, 255, 255),
+                  mode: Optional[str] = None):
+    """Per-element text colour from customization.<element>.text_color.
+
+    Delegated rather than reimplemented: there were two copies of this
+    read and three of the offset read, and the shared one also resolves
+    the element under the names plugins actually use (the layout block
+    says `score` where the style block says `score_text`) and honours a
+    per-mode override. Hex strings are still accepted.
+    """
+    from src.element_style import element_color as _shared
+    return _shared(config, element, default, mode)
 
 
 def font_color(config: Optional[Dict[str, Any]], fonts: Optional[Dict[str, Any]],

src/common/sports_game_renderer.py

@@ -134,19 +134,9 @@ class SportsGameRendererMixin:
         the element on the scroll/Vegas card too -- previously the schema
         advertised these offsets but this renderer ignored them.
         """
-        try:
-            layout = (self.config or {}).get("customization", {}).get("layout", {})
-            value = (layout.get(element) or {}).get(axis, default)
-            if isinstance(value, bool):
-                return default
-            if isinstance(value, (int, float)):
-                return int(value) if math.isfinite(value) else default
-            if isinstance(value, str):
-                parsed = float(value)
-                return int(parsed) if math.isfinite(parsed) else default
-        except (TypeError, ValueError, OverflowError):
-            pass
-        return default
+        from src.element_style import layout_offset
+        return layout_offset(self.config, element, axis, default,
+                             getattr(self, "SKIN_MODE", None))
 
     # ---- upcoming cards ------------------------------------------------
 

src/common/sports_shared.py

@@ -757,18 +757,31 @@ class SportsCoreSharedMixin:
             self.logger.debug("Headline font scaling skipped", exc_info=True)
         return fonts
 
+    def _get_layout_offset(self, element: str, axis: str,
+                           default: int = 0) -> int:
+        """X/Y nudge for one element, from ``customization.layout``.
+
+        Promoted here so every scoreboard reads offsets the same way the
+        scroll card does. Each plugin still carries its own copy in its
+        bundled sports.py, which wins by MRO until that copy is deleted --
+        deleting it is what buys the alias handling (a plugin asking for
+        ``score_text`` finds the ``score`` its users configured) and the
+        per-mode overrides, since this resolves through SKIN_MODE.
+        """
+        from src.element_style import layout_offset
+        return layout_offset(self.config, element, axis, default,
+                             getattr(self, "SKIN_MODE", None))
+
     def _element_color(self, element: str, default: Tuple[int, int, int] = (255, 255, 255)):
-        """Per-element text colour from customization.<element>.text_color."""
-        try:
-            cfg = (self.config or {}).get("customization", {}).get(element, {})
-            value = cfg.get("text_color")
-            if isinstance(value, (list, tuple)) and len(value) == 3:
-                return tuple(max(0, min(255, int(c))) for c in value)
-            if isinstance(value, str) and value.startswith("#") and len(value) == 7:
-                return tuple(int(value[i:i + 2], 16) for i in (1, 3, 5))
-        except (TypeError, ValueError):
-            pass
-        return default
+        """Per-element text colour from customization.<element>.text_color.
+
+        Mode-aware through SKIN_MODE, so Live and Recent instances of the
+        same scoreboard resolve their own colours without any call site
+        passing a mode.
+        """
+        from src.element_style import element_color as _shared
+        return _shared(self.config, element, default,
+                       getattr(self, "SKIN_MODE", None))
 
     def _unshare_element_fonts(self, fonts):
         """Give each colourable element its own face object.

src/element_style.py

@@ -46,6 +46,7 @@ root derived from this module's own location).
 import copy
 import json
 import logging
+import math
 import os
 from collections import OrderedDict
 from dataclasses import dataclass
@@ -886,7 +887,21 @@ def defaults_from_schema_file(schema_path: Union[str, os.PathLike]) -> Dict[str,
 # ---------------------------------------------------------------------------
 
 def _normalize_color(value: Any) -> Optional[Tuple[int, int, int]]:
-    """An (r, g, b) tuple of ints in 0..255, or None for anything else."""
+    """An (r, g, b) tuple of ints in 0..255, or None for anything else.
+
+    ``"#RRGGBB"`` is accepted as well as ``[r, g, b]``: the scoreboards'
+    own colour readers have always taken both, and this is the function
+    they now share.
+    """
+    if isinstance(value, str):
+        text = value.strip()
+        if len(text) == 7 and text.startswith('#'):
+            try:
+                return (int(text[1:3], 16), int(text[3:5], 16),
+                        int(text[5:7], 16))
+            except ValueError:
+                return None
+        return None
     if isinstance(value, (list, tuple)) and len(value) == 3:
         try:
             rgb = tuple(int(c) for c in value)
@@ -1005,6 +1020,101 @@ def _coerce_scale(value: Any, default: float) -> float:
     return min(scale, 10.0)
 
 
+def _coerce_offset(value: Any, default: int, element_key: str,
+                   axis: str) -> int:
+    """A pixel offset as an int; anything nonsensical is ``default``.
+
+    A bool degrades rather than counting as 1/0 -- the more correct reading
+    of a pixel offset, and what the shared resolver has always done
+    relative to the classic inline read. Non-finite floats degrade too:
+    the scroll-card reader guarded against those explicitly and this is now
+    the one implementation.
+    """
+    if isinstance(value, bool):
+        return int(default)
+    if isinstance(value, (int, float)):
+        if isinstance(value, float) and not math.isfinite(value):
+            return int(default)
+        return int(value)
+    if isinstance(value, str):
+        try:
+            parsed = float(value)
+        except (TypeError, ValueError):
+            logger.warning("Invalid layout offset for %s.%s: %r, using %s",
+                           element_key, axis, value, default)
+            return int(default)
+        if not math.isfinite(parsed):
+            return int(default)
+        return int(parsed)
+    return int(default)
+
+
+def layout_offset(config: Any, element_key: str, axis: str,
+                  default: int = 0, mode: Optional[str] = None) -> int:
+    """One ``customization.layout.<element>.<axis>`` value, as an int.
+
+    The stateless form of :meth:`ElementStyleResolver.offset_value`, for the
+    scoreboard helpers that are handed a config rather than holding one.
+    Both go through here, so the alias handling and the per-mode lookup
+    cannot drift between them -- there were three separate readers of this
+    block before, and the scroll-card one had already been found ignoring
+    offsets the schema advertised.
+    """
+    try:
+        block = config.get('customization') if isinstance(config, dict) else None
+        block = block if isinstance(block, dict) else {}
+        base = _lookup_element(block.get('layout'), element_key).get(axis)
+        base_value = (int(default) if base is None
+                      else _coerce_offset(base, default, element_key, axis))
+
+        if mode:
+            modes = block.get('modes')
+            mode_block = modes.get(mode) if isinstance(modes, dict) else None
+            if isinstance(mode_block, dict):
+                override = _lookup_element(mode_block.get('layout'),
+                                           element_key).get(axis)
+                if override is not None:
+                    return _coerce_offset(override, base_value,
+                                          element_key, axis)
+        return base_value
+    except Exception as e:
+        logger.warning("Error reading layout offset %s.%s: %s",
+                       element_key, axis, e)
+        try:
+            return int(default)
+        except (TypeError, ValueError):
+            return 0
+
+
+def element_color(config: Any, element_key: str,
+                  default: Tuple[int, int, int] = (255, 255, 255),
+                  mode: Optional[str] = None) -> Tuple[int, int, int]:
+    """``customization.<element>.text_color``, or ``default``.
+
+    The stateless colour lookup the scoreboards share. Unlike
+    :meth:`ElementStyleResolver.style` this does not compare against a
+    schema default -- the callers have no schema to hand -- so any
+    configured colour counts, which is what their own readers did.
+    """
+    try:
+        block = config.get('customization') if isinstance(config, dict) else None
+        block = block if isinstance(block, dict) else {}
+        if mode:
+            modes = block.get('modes')
+            mode_block = modes.get(mode) if isinstance(modes, dict) else None
+            if isinstance(mode_block, dict):
+                override = _normalize_color(
+                    _lookup_element(mode_block, element_key).get('text_color'))
+                if override is not None:
+                    return override
+        value = _normalize_color(
+            _lookup_element(block, element_key).get('text_color'))
+        return value if value is not None else default
+    except Exception as e:
+        logger.warning("Error reading colour for %s: %s", element_key, e)
+        return default
+
+
 class ElementStyleResolver:
     """Resolves per-element user styling against schema defaults.
 
@@ -1160,31 +1270,8 @@ class ElementStyleResolver:
         axis — ``None`` there means inherit the base offset, which is what
         lets a mode nudge one element without restating the rest.
         """
-        try:
-            # Base first, so it is the thing a malformed mode value falls
-            # back to. Resolving the mode first and passing the caller's
-            # default would let one bad string in a mode block silently
-            # discard a perfectly good base offset.
-            base = self._layout_axis(self._customization(), element_key, axis)
-            base_value = (int(default) if base is None
-                          else self._coerce_offset(base, default,
-                                                   element_key, axis))
-
-            effective_mode = self._effective_mode(mode)
-            if effective_mode:
-                override = self._layout_axis(
-                    self._mode_block(effective_mode), element_key, axis)
-                if override is not None:
-                    return self._coerce_offset(override, base_value,
-                                               element_key, axis)
-            return base_value
-        except Exception as e:
-            logger.warning("Error reading layout offset %s.%s: %s",
-                           element_key, axis, e)
-            try:
-                return int(default)
-            except (TypeError, ValueError):
-                return 0
+        return layout_offset(self._config, element_key, axis, default,
+                             self._effective_mode(mode))
 
     @staticmethod
     def _layout_element(block: Dict[str, Any],
@@ -1205,27 +1292,6 @@ class ElementStyleResolver:
         """``block['layout'][element][axis]``, or None if absent anywhere."""
         return cls._layout_element(block, element_key).get(axis)
 
-    @staticmethod
-    def _coerce_offset(value: Any, default: int, element_key: str,
-                       axis: str) -> int:
-        """A pixel offset as an int; anything nonsensical is ``default``.
-
-        A bool degrades rather than counting as 1/0 -- it is the more
-        correct reading of a pixel offset, and matches what the shared
-        resolver has always done relative to the classic inline read.
-        """
-        if isinstance(value, bool):
-            return int(default)
-        if isinstance(value, (int, float)):
-            return int(value)
-        if isinstance(value, str):
-            try:
-                return int(float(value))
-            except (TypeError, ValueError):
-                logger.warning("Invalid layout offset for %s.%s: %r, using %s",
-                               element_key, axis, value, default)
-                return int(default)
-        return int(default)
 
     # -- resolution internals -----------------------------------------------
 

test/test_sports_card.py

@@ -262,3 +262,53 @@ class TestUnshareElementFonts:
         fonts = {"score": a, "time": b}
         C.unshare_element_fonts(log, fonts)
         assert fonts["score"] is a and fonts["time"] is b
+
+class TestSharedElementReaders:
+    """The scoreboards' colour and offset reads now go through one
+    implementation.
+
+    There were two copies of the colour read and three of the offset read.
+    They had already drifted: the scroll-card renderer carries a comment
+    noting it used to ignore offsets its own schema advertised. Sharing one
+    reader is what lets a fix (or an alias, or a per-mode override) reach
+    the full-screen scorebug and the scroll card together.
+    """
+
+    CONFIG = {
+        "customization": {
+            "score_text": {"text_color": [255, 200, 0]},
+            "status": {"text_color": "#00ff00"},
+            "layout": {"score": {"y_offset": -3}},
+            "modes": {
+                "live": {"score_text": {"text_color": [255, 0, 0]},
+                         "layout": {"score": {"y_offset": 7}}},
+            },
+        },
+    }
+
+    def test_colour_reads_the_configured_value(self, log):
+        assert C.element_color(self.CONFIG, "score_text") == (255, 200, 0)
+
+    def test_hex_colours_are_still_accepted(self, log):
+        """These readers have always taken '#RRGGBB' as well as [r, g, b],
+        so the shared one had to learn it rather than the callers losing it."""
+        assert C.element_color(self.CONFIG, "status") == (0, 255, 0)
+
+    def test_colour_resolves_through_an_alias(self, log):
+        """The style block says status_text where this config says status."""
+        assert C.element_color(self.CONFIG, "status_text") == (0, 255, 0)
+
+    def test_a_mode_overrides_the_colour(self, log):
+        assert C.element_color(self.CONFIG, "score_text",
+                               mode="live") == (255, 0, 0)
+
+    def test_an_unset_element_gets_the_default(self, log):
+        assert C.element_color(self.CONFIG, "nothing", (1, 2, 3)) == (1, 2, 3)
+
+    @pytest.mark.parametrize("config", [
+        None, {}, "nonsense", {"customization": "nonsense"},
+        {"customization": {"score_text": "nonsense"}},
+        {"customization": {"score_text": {"text_color": "not a colour"}}},
+    ])
+    def test_a_hostile_config_gives_the_default(self, config, log):
+        assert C.element_color(config, "score_text", (9, 9, 9)) == (9, 9, 9)

test/test_sports_game_renderer.py

@@ -256,3 +256,48 @@ class TestContract:
         h._draw_upcoming_center(_draw(), {})
         h._draw_upcoming_game_status(_draw(), {})
         h.set_rankings_cache({})
+
+
+class TestLayoutOffsetsAreShared:
+    """The scroll/Vegas card reads offsets through the same function the
+    full-screen scorebug does.
+
+    This renderer already carries a comment about having ignored offsets its
+    own schema advertised -- it was the third copy of this read. Sharing one
+    means the alias handling and per-mode overrides land here too, without
+    this file knowing about either.
+    """
+
+    CONFIG = {"customization": {
+        "layout": {"score": {"y_offset": -3}},
+        "modes": {"live": {"layout": {"score": {"y_offset": 7}}}},
+    }}
+
+    def _host(self, config, mode=None):
+        attrs = {"config": config}
+        if mode:
+            attrs["SKIN_MODE"] = mode
+        return type("Card", (SportsGameRendererMixin,), attrs)()
+
+    def test_it_reads_the_configured_offset(self):
+        assert self._host(self.CONFIG)._layout_offset("score", "y_offset") == -3
+
+    def test_it_resolves_through_an_alias(self):
+        """A caller asking for score_text finds the layout.score the user
+        configured -- the two namespaces disagree by convention."""
+        assert self._host(self.CONFIG)._layout_offset(
+            "score_text", "y_offset") == -3
+
+    def test_skin_mode_selects_the_per_mode_offset(self):
+        assert self._host(self.CONFIG, "live")._layout_offset(
+            "score", "y_offset") == 7
+
+    def test_a_host_without_a_skin_mode_gets_the_base(self):
+        assert self._host(self.CONFIG)._layout_offset("score", "y_offset") == -3
+
+    @pytest.mark.parametrize("written", [
+        float("inf"), float("nan"), True, "bad", None, [], {},
+    ])
+    def test_nonsense_degrades_to_the_default(self, written):
+        config = {"customization": {"layout": {"score": {"y_offset": written}}}}
+        assert self._host(config)._layout_offset("score", "y_offset", 4) == 4

test/test_sports_shared.py

@@ -422,3 +422,43 @@ class TestUnshareKeepsTheLayoutEngine:
         fonts = {"score": shared, "time": shared}
         self._host()._unshare_element_fonts(fonts)
         assert fonts["time"].getlength("88-88") == shared.getlength("88-88")
+
+
+class TestPromotedLayoutOffset:
+    """_get_layout_offset moved here so every scoreboard reads offsets the
+    way the scroll card does.
+
+    Each plugin still carries its own copy in its bundled sports.py, which
+    wins by MRO. That is the migration property: adopting this is a
+    deletion in the plugin, and until that deletion nothing changes.
+    """
+
+    CONFIG = {"customization": {
+        "layout": {"score": {"y_offset": -3}},
+        "modes": {"recent": {"layout": {"score": {"y_offset": 9}}}},
+    }}
+
+    def _host(self, **attrs):
+        return type("H", (SportsCoreSharedMixin,),
+                    dict({"config": self.CONFIG}, **attrs))()
+
+    def test_it_reads_the_configured_offset(self):
+        assert self._host()._get_layout_offset("score", "y_offset") == -3
+
+    def test_it_resolves_through_an_alias(self):
+        """What a plugin gains by deleting its own copy: the style block
+        says score_text where the layout block says score."""
+        assert self._host()._get_layout_offset("score_text", "y_offset") == -3
+
+    def test_skin_mode_selects_the_per_mode_offset(self):
+        assert self._host(SKIN_MODE="recent")._get_layout_offset(
+            "score", "y_offset") == 9
+
+    def test_a_plugins_own_copy_still_wins(self):
+        """Until a plugin deletes its copy, this changes nothing for it."""
+        host = self._host(
+            _get_layout_offset=lambda self, element, axis, default=0: 99)
+        assert host._get_layout_offset("score", "y_offset") == 99
+
+    def test_an_unset_offset_is_the_default(self):
+        assert self._host()._get_layout_offset("nothing", "y_offset", 5) == 5