Skip to content

Conversation

@Arukuen
Copy link
Contributor

@Arukuen Arukuen commented Jan 15, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of timeline triggers configured to play only once, preventing unintended replays when triggered multiple times.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

Refactors trigger tracking in timeline initialization by introducing conditional creation of _playedTriggers only when onceOnly is enabled, and restructures the early-return logic in createInstance to check for already-played triggers with tighter guard conditions involving frontend, currentTrigger existence, and onceOnly flag.

Changes

Cohort / File(s) Summary
Timeline trigger tracking refactor
src/frontend/scripts/class-timeline.js
Conditional initialization of _playedTriggers (now a per-timeline Set guarded by onceOnly instead of unconditional per-trigger tracking). Modified createInstance early-return logic to check played triggers only when frontend, currentTrigger, and onceOnly are all true; returns null if already played, otherwise adds to tracked set.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Once was never quite so clear,
Till triggers learned when not to steer,
A Set now guards what plays but once,
No second dance for hasty hunts!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix: handle undefined trigger' is vague and does not clearly describe the main change, which involves conditional initialization of _playedTriggers and tightened onceOnly logic. Consider using a more descriptive title that reflects the core change, such as 'fix: prevent multiple timeline instances with onceOnly flag' or 'fix: conditional tracking of played triggers with onceOnly'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a29facd and 470f93d.

📒 Files selected for processing (1)
  • src/frontend/scripts/class-timeline.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (2)
src/frontend/scripts/class-timeline.js (2)

28-31: LGTM! Conditional initialization is appropriate.

Creating _playedTriggers only when onceOnly is enabled avoids unnecessary memory allocation and correctly guards against re-creating an existing Set when multiple Timeline instances share the same timelineData.


46-54: Good fix for handling undefined triggers.

The compound guard isFrontend && currentTrigger && this._onceOnly correctly prevents:

  1. Calling .has() on _playedTriggers when currentTrigger is undefined
  2. Adding undefined to the Set

The ordering of conditions ensures _playedTriggers is always initialized before access (since it's created in the constructor when _onceOnly is true).

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@Arukuen Arukuen self-assigned this Jan 15, 2026
@github-actions
Copy link

🤖 Pull request artifacts

file commit
pr13-interactions-13-merge.zip 470f93d

github-actions bot added a commit that referenced this pull request Jan 15, 2026
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