Skip to content

fix(masters): layout fixes for 32px-height displays#97

Merged
ChuckBuilds merged 9 commits intomainfrom
fix/masters-vegas-scroll
Apr 10, 2026
Merged

fix(masters): layout fixes for 32px-height displays#97
ChuckBuilds merged 9 commits intomainfrom
fix/masters-vegas-scroll

Conversation

@ChuckBuilds
Copy link
Copy Markdown
Owner

@ChuckBuilds ChuckBuilds commented Apr 10, 2026

Summary

  • Root cause fix: Wide-short displays <=32px (128x32, 256x32) now get compact tier overrides (header=8, footer=5, row=7) with smaller fonts — previously the large-tier measurements consumed 26 of 32 pixels leaving no usable content space
  • Hole card: Compact layout now shows course image to the right of stacked text instead of text-only two-column layout
  • Fun facts (vegas): Rendered as single-line wide cards so horizontal scroll reveals the full text instead of truncating at 2 lines
  • Fun facts (normal): Respects user's enabled setting in vegas mode; scroll steps calculated from display height instead of hardcoded; advance interval increased to 3s
  • Tee times: Player names stacked vertically on 48+ height displays; compact layout on <48px
  • BDF font: Added Path(__file__)-relative search path so 5x7.bdf is found on user machines regardless of CWD

Test plan

  • Verify 128x32 and 256x32 displays show 2 leaderboard rows
  • Verify hole cards show course image on right on 32px displays
  • Verify fun facts scroll through all lines before advancing
  • Verify fun facts disabled in config are hidden in vegas mode
  • Verify tee times show stacked names on 96x48+
  • Verify 5x7 BDF font loads on Pi

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Toggle for fun-facts display and improved Vegas-mode fun-fact presentation
    • Better handling of compact/wide card layouts for hole and schedule views
  • Bug Fixes

    • Smoother fun-fact rotation and smarter scroll progression
    • More responsive rendering and pagination across varied display sizes
    • Cleanup of temporary font artifacts on exit
  • Chores

    • Plugin bumped to version 2.3.0 (metadata updated)

Chuck and others added 4 commits April 9, 2026 18:16
…cache guard

Addresses three user reports from Discord.

## Report 3 (JScottyR): Vegas scroll cards spanning the full panel

On a 64×64 × 5-panel chain (320×64), masters was rendering each player
card at the full panel size, so "Vegas scroll" mode showed one giant card
at a time instead of a ticker of smaller fixed-size blocks. Every other
sports scoreboard in the repo follows a fixed-block convention — football,
hockey, and baseball all default `game_card_width=128` regardless of the
physical display width, with cards constructed at that explicit size.

Fix:

- New top-level config key `scroll_card_width` (default 128, min 32,
  max 256) in config_schema.json with a description referencing the
  Vegas scroll ticker mode.
- `manager.py` reads it as `self._scroll_card_width` in `__init__` (and
  on hot-reload), then passes `card_width=self._scroll_card_width,
  card_height=self.display_height` to each `render_*` call inside
  `get_vegas_content()`.
- `render_player_card()`, `render_hole_card()`, and `render_fun_fact()`
  in `masters_renderer.py` now accept optional `card_width`/`card_height`
  parameters. When provided, the body uses local `w`/`h`/`cw`/`ch`
  variables instead of `self.width`/`self.height`, and `is_wide_short`
  is recomputed per-card (so a 128×64 block on a 320×64 panel is
  aspect 2.0 and uses the standard vertical-stack layout, while the
  same panel's full-screen modes still use the two-column wide-short
  layout for the leaderboard, schedule, etc).
- `_render_player_card_wide_short()` takes the same `w`/`h` overrides
  and parameterizes every `self.width`/`self.height` reference.
- `_draw_gradient_bg()` accepts optional `width`/`height` so each render
  can allocate its own image at the override size.
- Enhanced renderer overrides (`render_player_card`, `render_hole_card`)
  delegate to `super()` when called with explicit card dimensions, since
  the enhanced round-scores block and left-panel-plus-image layouts are
  designed for full-panel rendering and don't fit at block sizes.

Verified locally at every `(parent, card)` combination in
`{(320,64), (384,64), (192,48), (128,64), (64,32)} × {(128,64), (128,48),
(80,64), (64,32)}`: every returned image's `.size` exactly matches the
override. A simulated `get_vegas_content()` on a 320×64 parent with
default `scroll_card_width=128` produces 33 cards (10 players, 18 holes,
5 facts), ALL at exactly 128×64. A 5-card scroll strip concatenated
side-by-side renders as a 640×64 image that looks like the football
scoreboard ticker pattern — headshot + name + country + score + pos/thru
per card, cleanly repeated.

## Reports 1 & 2 (Fish Man): defensive stale-cache guard

Reports 1 (`'str' object has no attribute 'tzinfo'`) and 2 (`'<=' not
supported between instances of 'float' and 'NoneType'`) are both already
fixed in PR #95 via `_rehydrate_meta()` and `_NEVER_EXPIRE` respectively,
but neither has merged to `main` yet — a user who pulled the plugin-store
update to 2.1.2 after PR #94 merged is running the exact version where
both errors fire.

Residual risk even after PR #95 / this branch ships: the disk cache file
`/var/cache/ledmatrix/masters_tournament_meta.json` persists from the
broken 2.1.2 run. The core `CacheManager` logs the `<= None` error and
returns None (cache miss) rather than re-raising, but the log noise
persists every tick until the file is overwritten by a successful fetch.

Added `MastersDataSource._safe_cache_get()` that wraps every
`cache_manager.get()` call in a try/except, treats any exception as a
cache miss, and logs a single warning per instance (not per tick)
pointing at the stale file path. Replaced all 9 `cache_manager.get()`
call sites in `masters_data.py` with the safe wrapper.

Verified with a stub `BrokenCache` that raises `TypeError('<=' not
supported...)` on every `.get()` AND a blocked network: the plugin
still initializes without crashing, returns the computed second-Thursday-
of-April fallback meta, falls back to mock leaderboard data, and logs
the stale-cache warning exactly once.

## Other

- Bumps manifest `2.2.4 → 2.2.5`.
- test_plugin_standalone.py: 45/45 still passing.

Depends on PR #95 being merged first (same branch base).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s wordmark assets

## Hole card layout (bundled with Vegas scroll fix)

The earlier Vegas scroll fix (2.2.5) had the enhanced `render_hole_card`
delegating to the base class when called with `card_width`/`card_height`
overrides. The base class uses a different layout (horizontal header +
centered image + footer) that's neither the user's desired "1 text column
+ image" for larger cards nor the "2 text columns, no image" for smaller
ones — so Vegas scroll hole cards looked inconsistent with the full-panel
layouts.

Rewrote the enhanced `render_hole_card` to:

1. Accept `card_width`/`card_height` directly and render at those
   dimensions.
2. Choose between two layouts based on the EFFECTIVE card dimensions
   (not `self.tier`), using thresholds `cw >= 96 AND ch >= 40`:
     * **Large enough for image** → single left-column text
       [Hole #, Name, Par, Yards] + hole image on the right, zone
       badge in the bottom-right. Split into a new helper
       `_render_hole_card_with_image()`.
     * **Smaller canvases** → two text columns: [# / Name] | [Par /
       Yards / Zone], no image. `_render_hole_card_compact()` now
       also accepts `cw`/`ch` overrides.
3. `left_w` (text column width) now scales with `cw` via
   `max(38, min(56, cw // 3))` so 256x64 cards get a roomier text
   column than 128x48 blocks.

Verified across the full size matrix:
  * 192x48 full panel      → IMAGE (#12 + Golden Bell + Par 3 + 155y + map + AMEN CORNER)
  * 128x64 full panel      → IMAGE
  * 256x64 full panel      → IMAGE
  * 320x64 Vegas 128x64    → IMAGE (inside a scrollable block, not full-panel)
  * 320x64 Vegas 128x48    → IMAGE
  * 192x48 Vegas 128x48    → IMAGE
  * 320x64 Vegas 80x64     → COMPACT 2-col (too narrow for image)
  * 128x32 full panel      → COMPACT 2-col
  * 128x32 Vegas 80x32     → COMPACT 2-col
  * 64x32 full panel       → COMPACT 2-col

Both layouts have the user's requested content layout:
  * Image layout: one text column [Hole #, Hole Name, Par, Yards]
  * Compact layout: col 1 [Hole # + Name], col 2 [Par, Yards, Zone]

## Masters wordmark assets

Extracted from `masters-icons.ttf` (IcoMoon icon font from masters.com).
The font has 143 glyphs in the U+E900–U+E98F Private Use Area, mostly
UI icons, but scanning for unusually-wide advance widths revealed two
wordmarks:

  * U+E954 (5.95 em) - the iconic MASTERS wordmark with the Augusta
    National contour + fairway flag + "Masters" in italic serif
  * U+E95B (4.89 em) - AUGUSTA wordmark in matching italic serif

Added to `plugins/masters-tournament/assets/masters/logos/`:
  * wordmark_32.png          191x32    white on transparent
  * wordmark_48.png          286x49
  * wordmark_64.png          381x64
  * wordmark_128.png         762x128
  * augusta_wordmark_32.png  156x30
  * augusta_wordmark_48.png  235x45

Assets only — not yet wired into the renderer. A future change can
swap these in for the current `masters_logo_*.png` in countdown,
player card headers, etc.

Bumps manifest 2.2.5 → 2.2.6.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…card

Previous attempts to use 5by7.regular.ttf via _load_font_sized() resulted
in washed-out anti-aliased text because PIL's TTF renderer smooths small
pixel fonts. The 5x7.bdf asset in the LEDMatrix core assets dir is a
true fixed-size bitmap font, and PIL.BdfFontFile can convert it to PIL
format for pixel-perfect 1:1 rendering.

Changes:

- Added _load_bdf_font(filename) in masters_renderer.py. Converts
  BDF → PIL format once per process via tempfile.mkdtemp(), caches
  the loaded ImageFont in _BDF_FONT_CACHE (keyed by filename), and
  stores failure results so missing files don't re-hit the disk.
  Uses a local `from PIL import BdfFontFile` import since it's only
  needed by this helper.
- Enhanced renderer imports _load_bdf_font alongside _load_font_sized.
- _render_hole_card_compact() now loads 5x7.bdf for both text_font
  and hole_font. The BDF renders at its native 7px height with no
  anti-aliasing, so glyphs look crisp and bold. Falls back to
  self.font_detail / self.font_body when the BDF file isn't on the
  search path.
- The previous 4x6 code is kept in a commented block directly
  underneath so we can flip fonts back in one edit if users report
  issues with 5x7 on actual hardware.

Verified:
- _load_bdf_font("5x7.bdf") returns a valid ImageFont, 'Ag' bbox
  height exactly 7px, second call hits the cache (same object id).
- _load_bdf_font("nonexistent.bdf") returns None cleanly.
- Render matrix across full panels (64x32, 128x32, 128x48, 128x64,
  192x48, 256x64) and Vegas scroll blocks (128x64, 128x48, 128x32,
  80x32) all produce expected layouts:
    * ch >= 48 → IMAGE layout (left-panel text + hole image, unchanged)
    * ch < 48  → COMPACT 2-col (# + Name left, Par + Yards right)
- Compact layouts now render with pixel-perfect 5x7 glyphs; the
  narrower-per-glyph 5x7 font also lets "AMEN CORNER" fit fully on
  a 128-wide compact card where 4x6 was truncating to "AMEN COR".
- test_plugin_standalone.py: 45/45 passing.

Note: 5x7.bdf is NOT copied into the plugin repo — it already lives
in the core LEDMatrix assets dir which the plugin's FONT_SEARCH_DIRS
already knows about. If a future deployment doesn't ship the core
fonts, the fallback to self.font_detail keeps the hole card working.

Bumps manifest 2.2.6 → 2.2.7.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…2, 256x32)

Addresses multiple layout issues reported on 2-chain and 4-chain 64x32
displays where the large tier's vertical measurements overflowed 32px.

- Add compact tier overrides for wide-short <=32px (header=8, footer=5,
  row=7) with appropriately sized fonts
- Hole card compact layout: text stacked left, course image on right
  (replaces two-column text-only layout)
- Fun facts in vegas mode: render as single-line wide cards for natural
  horizontal scroll reveal instead of truncating
- Fun facts: respect user's enabled setting in vegas mode
- Fun facts: calculate scroll steps from display height instead of
  hardcoding 5; increase advance interval to 3s
- Tee times: stack player names vertically on 48+ displays; compact
  single-line layout on <48px
- BDF font search: add Path(__file__)-relative path so 5x7.bdf is
  found regardless of working directory

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

Warning

Rate limit exceeded

@ChuckBuilds has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 38 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 12 minutes and 38 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2e8a34bb-6632-47de-a4d2-028c9dbdd141

📥 Commits

Reviewing files that changed from the base of the PR and between 433d469 and d426974.

📒 Files selected for processing (1)
  • plugins/masters-tournament/masters_renderer.py
📝 Walkthrough

Walkthrough

Bumps masters-tournament to v2.3.0 and updates plugins.json timestamp; adjusts fun-fact timing and dynamic scrolling/vegas rendering; adds renderer helpers for wrapping/line counts and vegas cards; adds font search and BDF temp cleanup; improves schedule pagination for small displays; makes hole-card layouts image-width aware.

Changes

Cohort / File(s) Summary
Version & Metadata
plugins.json, plugins/masters-tournament/manifest.json
Bumped plugin version from 2.2.72.3.0; updated plugins.json last_updated timestamp and manifest version/history released date.
Manager Logic
plugins/masters-tournament/manager.py
Increased fun-fact scroll interval (2→3s); replaced fixed scroll threshold with dynamic progression using renderer.get_fun_fact_line_count(); gated Vegas-mode fun-fact rendering by config and switched to render_fun_fact_vegas(...).
Core Renderer
plugins/masters-tournament/masters_renderer.py
Added project font search path, BDF temp-dir cleanup (atexit), _wrap_text helper, get_fun_fact_line_count() and render_fun_fact_vegas(); adjusted wide-short sizing/fonts and reworked schedule pagination/layout for compact vs standard heights.
Enhanced Renderer / Hole Cards
plugins/masters-tournament/masters_renderer_enhanced.py
Made hole-card layouts resolution-aware: compute image/text widths, enforce minimum image width, conditionally render dividers/images, and fall back to text-only when image space is insufficient; adjusted compact text positioning/clipping.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 79.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: layout fixes targeting 32px-height displays, which is the primary focus of the PR objectives and the most significant change across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/masters-vegas-scroll

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
plugins/masters-tournament/masters_renderer.py (1)

150-189: Consider cleaning up the converted-BDF temp directory on shutdown.

This module now owns a process-lifetime temp directory, but plugins/masters-tournament/manager.py::cleanup only clears logo_loader caches. Exposing a small cleanup hook here or registering an atexit handler would avoid leaving masters_bdf_* directories behind across restarts on the Pi.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/masters-tournament/masters_renderer.py` around lines 150 - 189, Add a
cleanup routine to remove the process-temp BDF directory so masters_bdf_* dirs
aren't left behind: implement a function (e.g., cleanup_bdf_temp or
remove_bdf_temp_dir) in the same module that checks _BDF_TEMP_DIR and, if set,
deletes the directory and clears _BDF_TEMP_DIR and _BDF_FONT_CACHE, then
register it with atexit.register or expose it for callers (e.g., have
plugins/masters-tournament/manager.py::cleanup call it). Ensure the routine uses
safe deletion (shutil.rmtree) and handles exceptions so shutdown doesn't crash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/masters-tournament/manager.py`:
- Around line 511-517: The current max_scroll uses a fixed heuristic (max_scroll
= max(5, 15 // visible)) which causes long facts to advance before all wrapped
lines are shown; instead compute the number of wrapped lines for the current (or
longest) fact using the renderer's wrapping logic and derive max_scroll from
that count and visible. Concretely: use the renderer to get the wrapped line
count (e.g., call the same wrap/measure routine used for rendering) for the fact
being displayed (or the longest fact), compute needed_steps = max(0,
wrapped_lines - visible + 1), then set max_scroll = max(1, needed_steps) (and
optionally clamp with a reasonable upper bound); replace the fixed 15//visible
heuristic with this computation so self._fact_scroll is compared against the
actual wrapped line count; keep existing variables line_h, visible,
self._fact_scroll and header calculations unchanged.

In `@plugins/masters-tournament/manifest.json`:
- Line 4: The manifest version in plugins/masters-tournament/manifest.json is
too small for a change that adds a new config key and behavior; update the
"version" field from "2.2.7" to the next MINOR release (e.g., "2.3.0") and also
update the top entry in the "versions" history array to the same value so they
stay synced; ensure any other manifest entries mentioned (lines ~45-60) are
likewise updated to the new minor version to reflect the
config_schema.json/manager.py changes.

In `@plugins/masters-tournament/masters_renderer_enhanced.py`:
- Around line 252-267: The code assumes enough horizontal space for image/text
columns and can produce negative widths for very narrow cards (e.g., 32px); fix
by clamping reserved image/text column widths and falling back to a text-only
layout when cw is smaller than the minimum required width. In the block that
computes cw/ch (and similar blocks at the other mentioned sites), compute
min_image_width and min_text_width (e.g., 36–38px), ensure image_width = max(0,
cw - reserved_text_column) and text_width = max(0, cw - reserved_image_column),
and if cw < (min_image_width + min_text_width) or image_width <= 0 then call the
compact/text-only renderer (or skip reserving an image column) instead of
attempting to draw negative-width images; update the code paths that call
_render_hole_card_with_image, _render_hole_card_compact, and any image layout
math at the other locations (lines near 279-283, 404-406, 448-450) to use these
clamped widths and fallback logic.

---

Nitpick comments:
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 150-189: Add a cleanup routine to remove the process-temp BDF
directory so masters_bdf_* dirs aren't left behind: implement a function (e.g.,
cleanup_bdf_temp or remove_bdf_temp_dir) in the same module that checks
_BDF_TEMP_DIR and, if set, deletes the directory and clears _BDF_TEMP_DIR and
_BDF_FONT_CACHE, then register it with atexit.register or expose it for callers
(e.g., have plugins/masters-tournament/manager.py::cleanup call it). Ensure the
routine uses safe deletion (shutil.rmtree) and handles exceptions so shutdown
doesn't crash.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a23306ee-32a5-4544-b9b8-9b67c649f6ee

📥 Commits

Reviewing files that changed from the base of the PR and between eda1201 and cfd35de.

⛔ Files ignored due to path filters (6)
  • plugins/masters-tournament/assets/masters/logos/augusta_wordmark_32.png is excluded by !**/*.png
  • plugins/masters-tournament/assets/masters/logos/augusta_wordmark_48.png is excluded by !**/*.png
  • plugins/masters-tournament/assets/masters/logos/wordmark_128.png is excluded by !**/*.png
  • plugins/masters-tournament/assets/masters/logos/wordmark_32.png is excluded by !**/*.png
  • plugins/masters-tournament/assets/masters/logos/wordmark_48.png is excluded by !**/*.png
  • plugins/masters-tournament/assets/masters/logos/wordmark_64.png is excluded by !**/*.png
📒 Files selected for processing (7)
  • plugins.json
  • plugins/masters-tournament/config_schema.json
  • plugins/masters-tournament/manager.py
  • plugins/masters-tournament/manifest.json
  • plugins/masters-tournament/masters_data.py
  • plugins/masters-tournament/masters_renderer.py
  • plugins/masters-tournament/masters_renderer_enhanced.py

Chuck and others added 2 commits April 10, 2026 13:34
…, width guards, temp cleanup

- Fun facts scroll: compute max_scroll from actual wrapped line count
  via new get_fun_fact_line_count() instead of fixed 15//visible heuristic
- Bump manifest version to 2.3.0 for new behavior/config changes
- Clamp hole card column widths: fall back to text-only layout when
  card is too narrow for a useful image column (< 20px)
- Register atexit cleanup for BDF temp directory so masters_bdf_* dirs
  don't accumulate in /tmp

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts:
#	plugins.json
#	plugins/masters-tournament/manager.py
#	plugins/masters-tournament/manifest.json
#	plugins/masters-tournament/masters_renderer.py
#	plugins/masters-tournament/masters_renderer_enhanced.py
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
plugins/masters-tournament/masters_renderer.py (1)

1093-1122: Share the wrapping routine with render_fun_fact().

get_fun_fact_line_count() re-implements the same wrap logic used in render_fun_fact(). If one side changes and the other doesn't, the scroll math drifts from what is actually rendered and you'll reintroduce skipped/repeated pages.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/masters-tournament/masters_renderer.py` around lines 1093 - 1122,
Extract the text-wrapping logic used in get_fun_fact_line_count and
render_fun_fact into a single helper (e.g. _wrap_fun_fact_lines or
_wrap_text_for_fact) and have both functions call that helper; specifically,
move the code that creates the ImageDraw, chooses font (self.font_detail),
computes max_w, iterates words and builds lines into the new helper and return
the list/count of wrapped lines, then replace the duplicate loop in
get_fun_fact_line_count and the equivalent loop in render_fun_fact to use this
helper so line counting and actual rendering use identical wrapping logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/masters-tournament/manager.py`:
- Around line 511-516: The scroll-advance check is off-by-one: in manager.py
where you compute total_lines, visible via
self.renderer.get_fun_fact_line_count(self._fact_index) and set max_scroll =
max(1, total_lines - visible + 1), change the condition that checks
self._fact_scroll so the fact advances when _fact_scroll reaches the last valid
start position (i.e., use >= instead of >). Update the comparison that currently
reads "if self._fact_scroll > max_scroll:" to "if self._fact_scroll >=
max_scroll:" so render_fun_fact() does not repeat the first page once more.

In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 156-168: The _cleanup_bdf_temp function currently swallows all
exceptions from shutil.rmtree; change the except block to capture the exception
(e.g., except Exception as e) and log a warning/debug message including the
exception info instead of passing silently. Use the module logger
(logging.getLogger(__name__)) or an existing logger, include context that rmtree
failed for _BDF_TEMP_DIR and set exc_info=True (or include str(e)), and ensure
the final steps still run (set _BDF_TEMP_DIR = None and
_BDF_FONT_CACHE.clear())—you can do the rmtree call inside try/finally so
cleanup always happens.

---

Nitpick comments:
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 1093-1122: Extract the text-wrapping logic used in
get_fun_fact_line_count and render_fun_fact into a single helper (e.g.
_wrap_fun_fact_lines or _wrap_text_for_fact) and have both functions call that
helper; specifically, move the code that creates the ImageDraw, chooses font
(self.font_detail), computes max_w, iterates words and builds lines into the new
helper and return the list/count of wrapped lines, then replace the duplicate
loop in get_fun_fact_line_count and the equivalent loop in render_fun_fact to
use this helper so line counting and actual rendering use identical wrapping
logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 51cc93b0-7eed-4aa4-a5de-c977bc8e5d8f

📥 Commits

Reviewing files that changed from the base of the PR and between cfd35de and ea593c9.

📒 Files selected for processing (5)
  • plugins.json
  • plugins/masters-tournament/manager.py
  • plugins/masters-tournament/manifest.json
  • plugins/masters-tournament/masters_renderer.py
  • plugins/masters-tournament/masters_renderer_enhanced.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugins/masters-tournament/manifest.json
  • plugins.json
  • plugins/masters-tournament/masters_renderer_enhanced.py

…act wrap helper

- Fix off-by-one in fun facts scroll: use >= instead of > so the fact
  advances immediately after showing the last scroll position
- BDF temp cleanup: log on failure instead of silently swallowing,
  use try/finally so cache is always cleared
- Extract _wrap_text() helper used by both get_fun_fact_line_count()
  and render_fun_fact() to eliminate duplicate wrapping logic

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 1300-1307: The stacked tee-time layout currently computes
player_rows and then slices players[:player_rows], which silently drops players
when len(players) > player_rows; update the rendering in the block using
player_rows (around the variables player_rows, players[:player_rows], entry_h,
rows, per_page) so that if len(players) > player_rows you render the first
(player_rows - 1) players individually and fold the remaining players into the
last visible line (e.g., combine them into a single overflow string like
"PlayerA / PlayerB / ..." or "PlayerA +N") instead of dropping them; ensure this
logic handles the edge case player_rows == 1 by folding all players into that
single line.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a3398ec1-4e4a-4594-8e7f-ee0c8d083343

📥 Commits

Reviewing files that changed from the base of the PR and between ea593c9 and d236f4a.

📒 Files selected for processing (2)
  • plugins/masters-tournament/manager.py
  • plugins/masters-tournament/masters_renderer.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugins/masters-tournament/manager.py

…imes

When player_rows < len(players), fold remaining players into the last
visible line as a comma-separated string instead of silently dropping
them. Handles edge case where player_rows == 1 by folding all players
into that single line.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 1097-1110: _wrap_text currently only splits on spaces so a single
long token can exceed max_w; modify the loop inside _wrap_text to detect when a
word itself is wider than max_w (using self._text_width(draw, word, font)) and,
in that case, break the word into smaller substrings that fit by iteratively
slicing the token (or greedily accumulating characters) until each chunk width
<= max_w, appending those chunks to lines (or to current_line when space
allows), so that no returned line exceeds max_w; reference symbols: _wrap_text,
self._text_width(draw, …, font), draw, font, max_w, current_line, lines.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72e51432-85ad-4135-beff-8538cf8baded

📥 Commits

Reviewing files that changed from the base of the PR and between d236f4a and 433d469.

📒 Files selected for processing (1)
  • plugins/masters-tournament/masters_renderer.py

…s in _wrap_text

Words wider than max_w (e.g. long unbroken tokens) are now split
character-by-character so no wrapped line exceeds the target width.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant