john/McRogueFace

Fork 0

[Bugfix] Timer::test re-phases on a wide step — `last_ran = now` discards skipped intervals AND the residual #397

New issue

Open

opened 2026-07-27 03:20:52 +00:00 by john · 0 comments

john commented

2026-07-27 03:20:52 +00:00

Owner

Found while implementing #396. This issue is about the behavior #396 deliberately did NOT change —
that work added a new bounded advance() primitive that sidesteps the problem; step() itself still
has it.

Behavior

Timer::test stamps last_ran = now rather than last_ran += interval. Combined with one frame
testing each timer at most once, a single wide step does two lossy things, not one:

Under-fires — step(10.0) fires a 1000 ms repeating timer once, at t=10000, not ten times.
This half is well understood and is what #396 addressed.
Silently re-phases — the timer's next deadline is recomputed from the end of the span, so the
nine skipped intervals and any residual are discarded rather than deferred. The timer's phase
relative to its original schedule is permanently lost.

Half 2 is the nastier one. Under-firing produces a sparse trace; re-phasing produces a trace that is
sparse and misaligned, and nothing downstream can reconstruct the original cadence to notice.

Why it wasn't fixed in #396

GameEngine::advance(int ms) avoids it by construction: it finds the nearest pending deadline within
the remaining span and runs one frame landing exactly on it, so last_ran = now happens to be exact
and no drift accumulates. step() was left unchanged on purpose — "run one frame" and "let a span of
time pass" are different operations, and folding them together would remove the one-frame primitive
and shift current_frame accounting under every existing headless test.

So the bug is now documented (step()'s docstring states one frame tests each timer at most once,
and points at advance()), but still reachable by anyone calling step() with a wide dt.

Question this issue needs to answer

Is last_ran = now ever the intended semantic? There is a defensible reading where a timer that
misses deadlines should not then fire a burst to catch up. If so, the fix is not to change step()
but to make the choice explicit and named, rather than an accident of assignment order. If not,
last_ran += interval with explicit catch-up is the correction, and it needs a decision about what
happens when many intervals have elapsed.

Note the related constraint from the simulation work driving #396: advancement should be driven by
elapsed time with catch-up, never by frame count, because a frame-driven world slows down exactly
when the consumer is slow.

Acceptance

Whatever the decision, it needs a test asserting phase behavior across a wide step — currently nothing
covers it, which is why this survived.

Related: #396 (the clock exposure and advance() primitive), #394 (unseeded behavior RNG — the other
determinism gap).

Found while implementing #396. **This issue is about the behavior #396 deliberately did NOT change** — that work added a new bounded `advance()` primitive that sidesteps the problem; `step()` itself still has it. ## Behavior `Timer::test` stamps `last_ran = now` rather than `last_ran += interval`. Combined with one frame testing each timer at most once, a single wide step does two lossy things, not one: 1. **Under-fires** — `step(10.0)` fires a 1000 ms repeating timer **once**, at t=10000, not ten times. This half is well understood and is what #396 addressed. 2. **Silently re-phases** — the timer's next deadline is recomputed from the *end* of the span, so the nine skipped intervals and any residual are discarded rather than deferred. The timer's phase relative to its original schedule is permanently lost. Half 2 is the nastier one. Under-firing produces a sparse trace; re-phasing produces a trace that is sparse *and* misaligned, and nothing downstream can reconstruct the original cadence to notice. ## Why it wasn't fixed in #396 `GameEngine::advance(int ms)` avoids it by construction: it finds the nearest pending deadline within the remaining span and runs one frame landing exactly on it, so `last_ran = now` happens to be exact and no drift accumulates. `step()` was left unchanged on purpose — "run one frame" and "let a span of time pass" are different operations, and folding them together would remove the one-frame primitive and shift `current_frame` accounting under every existing headless test. So the bug is now *documented* (`step()`'s docstring states one frame tests each timer at most once, and points at `advance()`), but still reachable by anyone calling `step()` with a wide dt. ## Question this issue needs to answer Is `last_ran = now` ever the *intended* semantic? There is a defensible reading where a timer that misses deadlines should not then fire a burst to catch up. If so, the fix is not to change `step()` but to make the choice explicit and named, rather than an accident of assignment order. If not, `last_ran += interval` with explicit catch-up is the correction, and it needs a decision about what happens when many intervals have elapsed. Note the related constraint from the simulation work driving #396: advancement should be driven by elapsed time with catch-up, never by frame count, because a frame-driven world slows down exactly when the consumer is slow. ## Acceptance Whatever the decision, it needs a test asserting phase behavior across a wide step — currently nothing covers it, which is why this survived. Related: #396 (the clock exposure and `advance()` primitive), #394 (unseeded behavior RNG — the other determinism gap).

john added the

Bugfix

priority:tier2-foundation

system:python-binding

labels

2026-07-27 03:20:52 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

john/McRogueFace#397

No description provided.

Rows
Columns

[Bugfix] Timer::test re-phases on a wide step — last_ran = now discards skipped intervals AND the residual #397

Behavior

Why it wasn't fixed in #396

Question this issue needs to answer

Acceptance

[Bugfix] Timer::test re-phases on a wide step — `last_ran = now` discards skipped intervals AND the residual #397