Skip to content

[#3280] Fix RepeatForever reset - #3873

Merged
eonarheim merged 2 commits into
excaliburjs:mainfrom
snowyukitty:fix/repeat-forever-reset
Oct 24, 2026
Merged

eonarheim merged 2 commits into
excaliburjs:mainfrom
snowyukitty:fix/repeat-forever-reset

Conversation

@snowyukitty

Copy link
Copy Markdown
Contributor

Related to #3280

RepeatForever.reset() was a no-op, so stopped repeats could not be reused and running repeats continued their current sequence. Reset now stops the active child and recalls the builder, matching Repeat.reset() and restarting at the first action from the entity's current state. Requests from a child callback rebuild after its update, so the first action is preserved; a later stop cancels the pending reset.

Adds twelve regressions for stopped-action reuse, movement cancellation, delay restart, builder order across repeated resets, empty builders, outer queue reset, and parallel/sequence composition. Includes JSDoc and a changelog entry. Reset intentionally retains the current transform, differing from the issue's suggested return to the default position and matching finite Repeat.

  • Issue exists and scope was discussed
  • Existing tests pass
  • Code follows the style guide
  • New regression tests pass
  • Changelog entry added

Validation: native Chromium action tests (126 passed; all eleven reset regressions fail on the original implementation), full Chromium unit project (2,175 passed, 210 skipped; clean base: 2,163 passed, 210 skipped), formatter, lint, engine type check, and production/development builds. Visual snapshots and API documentation were not run locally.

@eonarheim

Copy link
Copy Markdown
Member

@snowyukitty Looks totally reasonable to me, I'll take a closer look today. Many thanks!

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Excalibur benchmark

Commit e2bf15c · run attempt 2

Baseline npm:excalibur@latest (0.32.0) · Candidate: this PR's build (0.33.0-alpha.174+6e041f3) · 2 interleaved run(s), best run shown with the min–max of run medians · ms/frame · Chromium 148.0.7778.96 (swiftshader)

Test Baseline median Baseline p95 Candidate median Candidate p95 Δ median Δ p95 Extras (candidate)
actors-1000 4.65 (4.65–6.00) 10.10 5.20 (5.20–5.30) 10.90 +11.8% +7.9% actors=1000
actors-4000 31.60 (31.60–35.45) 89.93 32.35 (32.35–35.05) 89.90 +2.4% -0.0% actors=4000
arcade-1000-bouncing 25.70 (25.70–25.85) 44.11 25.45 (25.45–26.25) 38.61 -1.0% -12.4% outside=14
bunnymark-2000 131.60 (131.60–132.05) 135.53 131.70 (131.70–131.75) 135.51 +0.1% -0.0% bunnies=2000, drawCalls=1
bunnymark-5000 313.40 (313.40–314.30) 322.21 313.85 (313.85–313.95) 322.11 +0.1% -0.0% bunnies=5000, drawCalls=1
edge-floor-realistic-200 10.50 (10.50–10.60) 19.81 4.30 (4.30–4.65) 12.52 -59.0% -36.8% sleepingAtEnd=200, fellThrough=0 🚀
sleeping-pile-wake 25.30 (25.30–26.40) 49.94 14.50 (14.50–14.60) 34.65 -42.7% -30.6% sleepingAtEnd=0, spawned=5 🚀
stack-realistic-100-settle 8.60 (8.60–9.65) 23.62 7.95 (7.95–8.10) 16.21 -7.6% -31.4% sleepingAtEnd=0, firstFrameAllAsleep=-1
stack-realistic-300-rest ERROR 3.30 see job summary ❌
stack-realistic-300-settle ERROR 18.40 see job summary ❌

No test is more than 20% slower than the baseline. Results are informational, shared CI runners are noisy.

Produced by the code under test on a hosted runner without a GPU. Per-frame charts and full error text are in the job summary and benchmark-results artifact. Collaborators can comment /benchmark to re-run.

@eonarheim
eonarheim merged commit e4549b5 into excaliburjs:main Oct 24, 2026
10 checks passed
@eonarheim

Copy link
Copy Markdown
Member

@snowyukitty Thanks for the fix! LGTM

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.

2 participants