Skip to content

fix(studio): auto-enable loop when work-area markers are set#859

Merged
miguel-heygen merged 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/studio-work-area-loop-on-marker
May 15, 2026
Merged

fix(studio): auto-enable loop when work-area markers are set#859
miguel-heygen merged 1 commit into
heygen-com:mainfrom
calcarazgre646:fix/studio-work-area-loop-on-marker

Conversation

@calcarazgre646
Copy link
Copy Markdown
Contributor

Summary

Setting an in-point or out-point now auto-enables loopEnabled, so the playhead respects the work-area marker instead of running past the out-point. Closes the last open sub-bug of #834.

Background

PR #811 wired the work-area RAF loop to read inPoint/outPoint (useTimelinePlayer.ts:185-201), but kept the loop branch gated behind loopEnabled. Default for that flag is false (playerStore.ts:102), so users who set markers without first toggling the loop button saw playback sail past the out-point. With the L-key shuttle (multi-x speed), the playhead also overshoots by a few frames before the next RAF tick clamps it.

The original spec for the feature in #807 described markers as logic that "constrains the playback engine". The actual UX did not match that intent until the loop toggle was on, which the issue's reporter flagged as the bug under sub-bug #1.

Fix

setInPoint and setOutPoint flip loopEnabled to true when given a non-null value. This extends the existing "smart setter" pattern already in the store (setting one marker past the other already nullifies the counterpart). Clearing a marker with null preserves the current loopEnabled, so a user who manually toggles the loop button afterward stays in control.

setInPoint: (time) =>
  set((state) => {
    const t = time !== null && Number.isFinite(time) ? time : null;
    return {
      inPoint: t,
      outPoint:
        t !== null && state.outPoint !== null && t >= state.outPoint ? null : state.outPoint,
      loopEnabled: t !== null ? true : state.loopEnabled,
    };
  }),

Symmetric for setOutPoint. The RAF loop body in useTimelinePlayer (both forward and reverse paths) already does the right thing once loopEnabled is true.

Tests

packages/studio/src/player/store/playerStore.test.ts had no existing coverage for setInPoint/setOutPoint. Added 14 new tests across both setters covering:

  • Basic set / clear / non-finite rejection
  • Overlap nullification (existing behavior, was uncovered)
  • Auto-enable on non-null set
  • Preserve loopEnabled on clear (in both directions: was-true stays true, was-false stays false)

Full studio suite: 525/525 ✅, oxlint clean, oxfmt clean, typecheck clean.

Behavior of edge cases

  • Manual toggle after set: if the user disables loop manually after setting markers, then updates a marker, loop re-enables. Each "set marker" is treated as a fresh expression of intent ("I want bounded playback here").
  • Clear preserves: setInPoint(null) or setOutPoint(null) does not touch loopEnabled, so a user who built up a loop-on state by other means keeps it.
  • Reset preserves: reset() continues to preserve loopEnabled as before (it bypasses the setters with a direct set(...)).

Issue #834 status after this PR

Issue can be closed once this lands.

Setting an in or out point now turns on loopEnabled so the playhead
respects the marker instead of running past the out-point. Closes the
last open sub-bug of heygen-com#834.

Background: PR heygen-com#811 wired the work-area RAF loop to read inPoint/outPoint
but kept the loop branch gated behind loopEnabled. Default for that flag
is false, so users who set markers without first toggling the loop button
saw playback sail past the out-point (or, with the L shuttle, overshoot
by a few frames before pausing). The original spec for the feature in
issue heygen-com#807 described markers as logic that "constrains the playback
engine"; the actual UX did not match that until the toggle was on.

Fix: setInPoint and setOutPoint flip loopEnabled to true when given a
non-null value. This sits next to the existing "smart setter" behavior
already in the store (setting one marker past the other nullifies the
counterpart). Clearing a marker with null preserves the current
loopEnabled, so a user who manually toggles the loop button stays in
control after that point.

Tests: full coverage for setInPoint and setOutPoint (none existed
before), including overlap nullification, non-finite rejection,
auto-enable on set, and preserve-on-clear in both directions.

Closes heygen-com#834
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen left a comment

Choose a reason for hiding this comment

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

Verified the fix end-to-end:

  • RAF loop (useTimelinePlayer.ts:192-211 forward, 296-314 reverse): both paths compute loop bounds from inPoint/outPoint unconditionally but gate the actual loop-back on loopEnabled. Flipping loopEnabled to true on marker set is the minimal, correct fix.
  • All 3 call sites (usePlaybackKeyboard.ts for I/O/Shift+I/Shift+O, PlayerControls.tsx for the X-clear buttons) pass either a time value or null, both handled correctly by the ternary.
  • reset() bypasses the setters entirely (direct set(...)) so it won't auto-enable loop — verified.
  • Tests: 14 new tests covering set/clear/non-finite/overlap/auto-enable/preserve-on-clear, all passing (44/44 in the player store suite).

One optional nit: setOutPoint omits the 2-line comment that setInPoint has for the loopEnabled line — could add a brief // see setInPoint for symmetry, but not blocking.

@miguel-heygen miguel-heygen merged commit 83c29fa into heygen-com:main May 15, 2026
24 checks passed
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