From f20319be0164011a4103a07ad3acf8224f2e7a4c Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Fri, 15 May 2026 01:24:50 -0300 Subject: [PATCH] fix(studio): auto-enable loop when work-area markers are set 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 #834. Background: PR #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 #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 #834 --- .../src/player/store/playerStore.test.ts | 94 +++++++++++++++++++ .../studio/src/player/store/playerStore.ts | 4 + 2 files changed, 98 insertions(+) diff --git a/packages/studio/src/player/store/playerStore.test.ts b/packages/studio/src/player/store/playerStore.test.ts index 78b43f6d7..0d1750073 100644 --- a/packages/studio/src/player/store/playerStore.test.ts +++ b/packages/studio/src/player/store/playerStore.test.ts @@ -77,6 +77,100 @@ describe("usePlayerStore", () => { }); }); + describe("setInPoint", () => { + it("updates inPoint", () => { + usePlayerStore.getState().setInPoint(1.5); + expect(usePlayerStore.getState().inPoint).toBe(1.5); + }); + + it("clears inPoint when given null", () => { + usePlayerStore.getState().setInPoint(1.5); + usePlayerStore.getState().setInPoint(null); + expect(usePlayerStore.getState().inPoint).toBeNull(); + }); + + it("rejects non-finite values", () => { + usePlayerStore.getState().setInPoint(Number.NaN); + expect(usePlayerStore.getState().inPoint).toBeNull(); + }); + + it("nullifies outPoint when new inPoint is at or past existing outPoint", () => { + usePlayerStore.getState().setOutPoint(2); + usePlayerStore.getState().setInPoint(3); + expect(usePlayerStore.getState().outPoint).toBeNull(); + expect(usePlayerStore.getState().inPoint).toBe(3); + }); + + it("preserves outPoint when new inPoint is before it", () => { + usePlayerStore.getState().setOutPoint(5); + usePlayerStore.getState().setInPoint(2); + expect(usePlayerStore.getState().outPoint).toBe(5); + }); + + it("auto-enables loopEnabled when set to a non-null value", () => { + usePlayerStore.getState().setLoopEnabled(false); + usePlayerStore.getState().setInPoint(1.5); + expect(usePlayerStore.getState().loopEnabled).toBe(true); + }); + + it("preserves loopEnabled when cleared with null", () => { + usePlayerStore.getState().setLoopEnabled(true); + usePlayerStore.getState().setInPoint(null); + expect(usePlayerStore.getState().loopEnabled).toBe(true); + + usePlayerStore.getState().setLoopEnabled(false); + usePlayerStore.getState().setInPoint(null); + expect(usePlayerStore.getState().loopEnabled).toBe(false); + }); + }); + + describe("setOutPoint", () => { + it("updates outPoint", () => { + usePlayerStore.getState().setOutPoint(4.2); + expect(usePlayerStore.getState().outPoint).toBe(4.2); + }); + + it("clears outPoint when given null", () => { + usePlayerStore.getState().setOutPoint(4.2); + usePlayerStore.getState().setOutPoint(null); + expect(usePlayerStore.getState().outPoint).toBeNull(); + }); + + it("rejects non-finite values", () => { + usePlayerStore.getState().setOutPoint(Number.POSITIVE_INFINITY); + expect(usePlayerStore.getState().outPoint).toBeNull(); + }); + + it("nullifies inPoint when new outPoint is at or before existing inPoint", () => { + usePlayerStore.getState().setInPoint(5); + usePlayerStore.getState().setOutPoint(3); + expect(usePlayerStore.getState().inPoint).toBeNull(); + expect(usePlayerStore.getState().outPoint).toBe(3); + }); + + it("preserves inPoint when new outPoint is after it", () => { + usePlayerStore.getState().setInPoint(2); + usePlayerStore.getState().setOutPoint(5); + expect(usePlayerStore.getState().inPoint).toBe(2); + }); + + it("auto-enables loopEnabled when set to a non-null value", () => { + usePlayerStore.getState().setLoopEnabled(false); + usePlayerStore.getState().setOutPoint(4.2); + expect(usePlayerStore.getState().loopEnabled).toBe(true); + }); + + it("preserves loopEnabled when cleared with null", () => { + usePlayerStore.getState().setLoopEnabled(true); + usePlayerStore.getState().setOutPoint(null); + expect(usePlayerStore.getState().loopEnabled).toBe(true); + + usePlayerStore.getState().setLoopEnabled(false); + usePlayerStore.getState().setOutPoint(null); + expect(usePlayerStore.getState().loopEnabled).toBe(false); + }); + }); + describe("setTimelineReady", () => { it("updates timelineReady", () => { usePlayerStore.getState().setTimelineReady(true); diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index 59792d20d..414283dc7 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -127,6 +127,9 @@ export const usePlayerStore = create((set) => ({ inPoint: t, outPoint: t !== null && state.outPoint !== null && t >= state.outPoint ? null : state.outPoint, + // Setting a work-area marker implies the user wants playback bounded by it. + // Auto-enable loop so the playhead respects the marker instead of running past. + loopEnabled: t !== null ? true : state.loopEnabled, }; }), setOutPoint: (time) => @@ -135,6 +138,7 @@ export const usePlayerStore = create((set) => ({ return { outPoint: t, inPoint: t !== null && state.inPoint !== null && t <= state.inPoint ? null : state.inPoint, + loopEnabled: t !== null ? true : state.loopEnabled, }; }), setManualZoomPercent: (percent) =>