feat: penalty box countdown timer — expose box_time_remaining_ms (Option B) - #5
feat: penalty box countdown timer — expose box_time_remaining_ms (Option B)#5a1ly404 wants to merge 2 commits into
Conversation
…ck accumulator Track jam-time remaining per in-box skater using Option C (jam-clock delta accumulator). Server computes max(0, 30_000 - elapsed) so the overlay can read and render directly without any arithmetic. - SkaterPosition.box_time_remaining_ms: Optional[int] — None when not in box, 30_000 on entry, counts down to 0 while jam_running is True. - ScoreboardClient._tick_box_timers() + _remaining_ms() called after each state update; _prev_jam_running guard prevents spurious delta on jam start. - 7 new tests covering: init None, entry=30000, countdown, pause between jams, box exit reset, 0-clamp, and single-skater isolation.
There was a problem hiding this comment.
Pull request overview
Adds a pre-computed penalty box countdown (box_time_remaining_ms) to the live state model so overlays can render remaining penalty time directly without client-side arithmetic.
Changes:
- Extend
SkaterPositionwithbox_time_remaining_ms: Optional[int]. - Implement jam-clock-based accumulation in
ScoreboardClientand expose the computed remaining milliseconds per skater. - Add async client tests covering countdown start, decrement, pause, clamp-to-zero, and reset-to-None behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
models.py |
Adds box_time_remaining_ms to the per-skater model returned by /live. |
client.py |
Tracks per-position penalty elapsed time and maps it to a clamped remaining-ms countdown. |
tests/test_client.py |
Introduces tests validating basic countdown semantics for box_time_remaining_ms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Maintain per-position elapsed counters — reset on box exit. | ||
| for team_n in (1, 2): | ||
| for crg_pos in POSITION_MAP.values(): | ||
| key = f"{team_n}.{crg_pos}" | ||
| in_box = ( | ||
| self._get(f"Team({team_n}).Position({crg_pos}).PenaltyBox", bool) | ||
| or False | ||
| ) | ||
| if not in_box: | ||
| self._box_elapsed.pop(key, None) | ||
| elif key not in self._box_elapsed: | ||
| # Skater just entered — initialise at zero | ||
| self._box_elapsed[key] = 0 | ||
|
|
||
| # Accumulate only when the jam was *already* running at the previous tick. | ||
| if ( | ||
| jam_running | ||
| and self._prev_jam_running | ||
| and self._prev_jam_clock_ms is not None | ||
| and jam_clock is not None | ||
| ): | ||
| delta = self._prev_jam_clock_ms - jam_clock | ||
| if delta > 0: # negative means clock was reset; skip | ||
| for key in list(self._box_elapsed.keys()): | ||
| self._box_elapsed[key] += delta | ||
|
|
There was a problem hiding this comment.
_tick_box_timers() updates in-box membership (initialise/pop _box_elapsed) before applying the jam-clock delta, and then applies the delta to list(self._box_elapsed.keys()). If a skater enters or exits the box in the same update that includes a jam-clock change while jam_running is already True, the newly-entered skater will incorrectly get credited with the prior interval’s elapsed time, and the just-exited skater will incorrectly miss it. Consider applying the delta based on the previous-tick in-box set (e.g., snapshot keys before membership updates or track prev_in_box per key), then update membership after.
| self._box_elapsed: Dict[str, int] = {} # key: "<team_n>.<crg_pos>" | ||
| self._prev_jam_clock_ms: Optional[int] = None | ||
| self._prev_jam_running: Optional[bool] = None | ||
|
|
There was a problem hiding this comment.
The new timer state (_box_elapsed/_prev_jam_clock_ms/_prev_jam_running) is never reset on disconnect/reconnect. Because run() reuses the same client instance across reconnects, stale values can carry over and produce a large/incorrect delta (or preserve elapsed for in-box skaters) immediately after reconnect. Reset these fields when a new WS connection is established (and/or when it closes) so timers restart from a known baseline.
| self._box_elapsed: Dict[str, int] = {} # key: "<team_n>.<crg_pos>" | |
| self._prev_jam_clock_ms: Optional[int] = None | |
| self._prev_jam_running: Optional[bool] = None | |
| self._reset_timer_state() | |
| def _reset_timer_state(self) -> None: | |
| """Reset per-connection timer-related state. | |
| This should be called whenever a new WebSocket connection is | |
| established (and optionally when it is closed) to avoid carrying | |
| over stale timer values across reconnects. | |
| """ | |
| # key: "<team_n>.<crg_pos>" | |
| self._box_elapsed: Dict[str, int] = {} | |
| self._prev_jam_clock_ms = None | |
| self._prev_jam_running = None |
| async def test_box_remaining_starts_at_30000_on_box_entry(mock_server): | ||
| """box_time_remaining_ms starts at 30 000 the moment in_box becomes True.""" | ||
| client, task = await _connected_client(mock_server) | ||
| try: | ||
| await mock_server.push_update({ | ||
| "ScoreBoard.CurrentGame.Team(1).Position(Blocker1).PenaltyBox": True, | ||
| }) | ||
| await asyncio.sleep(0.1) | ||
| state = client.get_live_state() | ||
| assert state.team1.blocker1.in_box is True | ||
| assert state.team1.blocker1.box_time_remaining_ms == 30_000 | ||
| finally: | ||
| client.stop() | ||
| await asyncio.sleep(0.05) | ||
|
|
||
|
|
||
| async def test_box_remaining_counts_down_while_jam_running(mock_server): | ||
| """box_time_remaining_ms decreases by the jam-clock delta when jam_running is True.""" | ||
| client, task = await _connected_client(mock_server) | ||
| try: | ||
| await mock_server.push_update({ | ||
| "ScoreBoard.CurrentGame.Team(1).Position(Blocker2).PenaltyBox": True, | ||
| "ScoreBoard.CurrentGame.Clock(Jam).Running": True, | ||
| "ScoreBoard.CurrentGame.Clock(Jam).Time": 60000, | ||
| }) | ||
| await asyncio.sleep(0.1) | ||
|
|
||
| await mock_server.push_update({ | ||
| "ScoreBoard.CurrentGame.Clock(Jam).Time": 59000, | ||
| }) | ||
| await asyncio.sleep(0.1) | ||
| state = client.get_live_state() | ||
| assert state.team1.blocker2.box_time_remaining_ms == 29_000 | ||
|
|
||
| await mock_server.push_update({ | ||
| "ScoreBoard.CurrentGame.Clock(Jam).Time": 57000, | ||
| }) | ||
| await asyncio.sleep(0.1) | ||
| state = client.get_live_state() | ||
| assert state.team1.blocker2.box_time_remaining_ms == 27_000 | ||
| finally: | ||
| client.stop() | ||
| await asyncio.sleep(0.05) | ||
|
|
There was a problem hiding this comment.
Current tests cover steady-state countdown and pause, but they don’t cover the edge case where PenaltyBox flips True/False in the same update as a jam-clock change while jam_running is already True. Adding regression tests for (1) mid-jam box entry with a simultaneous clock decrement (should still start at 30_000), and (2) mid-jam box exit with a simultaneous clock decrement (should count the final delta before clearing) would catch the membership/delta ordering bug.
…view) Address all three Copilot review comments on PR #5. 1. Two-phase ordering in _tick_box_timers Bug: membership (pop/init _box_elapsed) was updated before the jam-clock delta was applied. Skater entering box simultaneous with a clock tick was incorrectly credited with the prior interval's elapsed. Fix: Phase 1 applies delta to the previous in-box set; Phase 2 then updates membership. 2. Stale timer state across reconnects Bug: _box_elapsed, _prev_jam_clock_ms, _prev_jam_running were never reset between disconnects, so elapsed from the previous session could carry over and produce incorrect remaining values after reconnect. Fix: _reset_timer_state() clears these fields and is called at the top of _run_once() before each new connection. 3. Regression tests added - test_box_remaining_entry_simultaneous_with_clock_tick: entry + tick in same message leaves remaining at 30_000, not less. - test_box_remaining_exit_simultaneous_with_clock_tick: exit + tick in same message; exiting skater clears to None, other skaters unaffected. - test_timer_state_resets_on_reconnect: after server bounce the initial snapshot (PenaltyBox=False) leaves remaining as None, not stale.
|
Closing — client has decided to handle penalty timing in their own overlay workflow rather than derive it server-side. The |
Overview
Implements Option B for the PP Timer discussion: expose a pre-computed countdown per in-box skater as
box_time_remaining_ms: Optional[int]onSkaterPosition.The overlay reads and renders the value directly — no arithmetic needed:
Compare with: feat/pp-timer-elapsed which exposes raw elapsed and lets the consumer compute remaining.
How it works (Option C — jam-clock accumulator)
Same accumulation logic as the elapsed PR, but
_remaining_ms()computesmax(0, 30_000 - elapsed)before the value is stored on the model.Nonewhenin_box = False30_000the momentin_boxtransitions to Truejam_runningwas True at both the current and previous tick (pauses during lineup / timeouts automatically)0— never negative_PENALTY_MS) is a single class-level value; easy to changeChanges
models.pySkaterPosition.box_time_remaining_ms: Optional[int] = Noneclient.py_tick_box_timers(),_remaining_ms(),_prev_jam_clock_ms,_prev_jam_running, wired into receive loop and_team()tests/test_client.pyDecision pending
Waiting for customer preference between this PR and feat/pp-timer-elapsed. Do not merge until decided.