From b07075849b3f8a9434bfd2067d7d18fa036e64a0 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Thu, 3 Sep 2026 16:56:51 -0400 Subject: [PATCH] fix(sports): let the plugin declare its own directory, don't deduce it _plugin_dir() returned None on every device. The consequence was silent and reached the panel: _plugin_dir() -> None _schema_font_size() -> None for every element -> a configured size equal to the schema default stops looking like a default and is treated as a deliberate user choice -> the snap to the font's pixel grid is skipped -> 4x6-font.ttf renders at 6 instead of 7: 3px-wide glyphs, not 4px On a 256x64 panel that made the odds, the team records and the date row hard to read. Both `odds` and `detail` were affected -- anything resolving a grid-snapped schema default was a pixel narrow. Why it was invisible here. PluginLoader._namespace_plugin_modules renames every bare module a plugin brought in (sports, game_renderer, ...) to "_plg__" and REMOVES the bare sys.modules entry, so two plugins owning a module of the same name cannot collide. A class defined in sports.py still reports __module__ == "sports", but sys.modules["sports"] is gone, so walking the MRO for a module with a __file__ finds nothing. Every test here imported plugins directly, which leaves the bare entry in place, so the walk succeeded. The safety harness loads plugins its own way and never reproduced it either. It was found by a user counting pixels on the panel. The directory is now declared by the plugin (_PLUGIN_DIR) and only deduced as a fallback, for hosts that declare nothing -- the plugins' own probe harnesses build classes with type(). Verified on hardware, which is the only place the original failure appeared: before, the live service logged plugin_dir=None and 4x6-font.ttf@6 for all six football managers; after, plugin_dir resolves and both odds and detail are @7. Five regression tests, including the production shape: a class whose __module__ is absent from sys.modules still resolves via its declared directory, and the precondition that the MRO walk alone returns None is pinned so the test keeps meaning something if the fallback changes. --- src/common/sports_shared.py | 49 ++++++++++++++++++++++++++++----- test/test_sports_shared.py | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/common/sports_shared.py b/src/common/sports_shared.py index f427817d..1187b846 100644 --- a/src/common/sports_shared.py +++ b/src/common/sports_shared.py @@ -211,17 +211,52 @@ def _crisp_size(cls, font_file, desired): return desired return max(grid, int(round(float(desired) / grid)) * grid) + #: Absolute path of this plugin's directory, declared by the plugin + #: itself. The mixin cannot work it out -- see _plugin_dir. + _PLUGIN_DIR: ClassVar[Optional[str]] = None + def _plugin_dir(self) -> Optional[str]: - """Directory of the plugin that defines this class. + """Directory of the plugin that owns this instance. In sports.py these methods could just use ``__file__``. Here that is - src/common/, so the plugin's own directory has to be recovered from the - instance. ``type(self).__module__`` alone is not enough: SportsCore is - an ABC, so a subclass built with ``type(name, bases, ns)`` -- which the - plugins' own tests do -- reports its module as "abc". Walking the MRO - steps past those synthetic classes to the first one whose module sits - next to a config_schema.json, which is the real plugin. + src/common/, so the directory has to come from the plugin. + + It is TOLD, not deduced. The first version walked the MRO for a class + whose module sits beside a config_schema.json. That works when a test + imports the plugin itself, and returns None under the real loader, for + a specific reason worth recording: + + PluginLoader._namespace_plugin_modules renames every bare module a + plugin brought in (sports, game_renderer, ...) to + "_plg__" and REMOVES the bare entry, so two + plugins owning a module of the same name cannot collide. + + The class still reports ``__module__ == "sports"``, but + ``sys.modules["sports"]`` no longer exists, so the walk finds no + __file__ and falls off the end. The failure was silent and expensive: + + _plugin_dir() -> None + _schema_font_size() -> None for every element + -> a configured size equal to the schema default stops looking + like a default and is treated as a deliberate user choice + -> the snap to the font's pixel grid is skipped + -> 4x6-font.ttf renders at 6 instead of 7: 3px-wide glyphs + instead of 4px + + On a 256x64 panel that made the odds, the team records and the date row + hard to read. It was found by a user counting pixels on the panel. No + gate here caught it: the tests imported plugins directly and the + safety harness loads them its own way, so neither reproduced the + loader's renaming. + + The MRO walk stays as a fallback for hosts that declare no + _PLUGIN_DIR -- the plugins' own probe harnesses build classes with + ``type()`` -- but it is no longer the primary answer. """ + declared = getattr(self, "_PLUGIN_DIR", None) + if declared and os.path.isfile(os.path.join(declared, "config_schema.json")): + return declared + for cls in type(self).__mro__: module = sys.modules.get(getattr(cls, "__module__", ""), None) path = getattr(module, "__file__", None) diff --git a/test/test_sports_shared.py b/test/test_sports_shared.py index f18fe64d..bd402ae3 100644 --- a/test/test_sports_shared.py +++ b/test/test_sports_shared.py @@ -319,3 +319,58 @@ def test_the_streak_starts_from_absent_state(self): assert not hasattr(h, "_empty_live_streak") h._note_live_fetch(False) assert h._empty_live_streak == 1 + + +class TestPluginDirIsToldNotDeduced: + """The regression that shipped: _plugin_dir returned None in production. + + The first version walked the MRO for a class whose module sits beside a + config_schema.json. That passes when a test imports the plugin directly -- + which is how it was verified -- and returns None under the real plugin + loader, which imports modules by a path that leaves no such entry on the + MRO. + + Silent, and expensive: no schema means _schema_font_size returns None for + every element, so a configured size equal to the schema default stops + looking like a default, is treated as a deliberate choice, and skips the + grid snap. 4x6-font.ttf then renders at 6 rather than 7 -- 3px-wide glyphs + instead of 4px. On a 256x64 panel that made the odds, the records and the + date row illegible. A user counted the pixels; no gate here caught it. + """ + + def test_a_declared_directory_is_used(self, tmp_path): + d = _write_plugin(tmp_path, "declared") + host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})() + assert host._plugin_dir() == str(d) + + def test_it_works_when_no_module_on_the_mro_helps(self, tmp_path, monkeypatch): + """The production case: nothing on the MRO sits beside a schema.""" + d = _write_plugin(tmp_path, "loaderstyle") + # A class whose module is not importable by name, as the loader produces. + cls = type("Loaded", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)}) + cls.__module__ = "a.module.name.that.is.not.in.sys.modules" + assert cls.__new__(cls)._plugin_dir() == str(d), ( + "the declared directory must win when the MRO walk cannot help") + + def test_without_it_the_mro_walk_would_have_failed(self): + # Pin the precondition, so this test still means something if the + # fallback is ever changed. + cls = type("Orphan", (SportsCoreSharedMixin,), {}) + cls.__module__ = "not.a.real.module" + assert cls.__new__(cls)._plugin_dir() is None + + def test_a_declared_directory_without_a_schema_is_not_trusted(self, tmp_path): + # A stale or wrong path must not shadow the fallback. + empty = tmp_path / "noschema" + empty.mkdir() + d = _write_plugin(tmp_path, "realone") + monkey = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(empty)}) + assert monkey.__new__(monkey)._plugin_dir() != str(empty) + + def test_the_font_size_consequence(self, tmp_path): + """End to end: a declared directory restores the schema lookup.""" + d = _write_plugin(tmp_path, "sizeconseq") + host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})() + assert host._schema_font_size("score") == 16, ( + "without the schema this is None, which is what made a configured " + "size look user-chosen and skipped the grid snap")