Skip to content

Commit 83c29fa

Browse files
fix(studio): auto-enable loop when work-area markers are set (#859)
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 Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>
1 parent 808b10c commit 83c29fa

2 files changed

Lines changed: 98 additions & 0 deletions

File tree

‎packages/studio/src/player/store/playerStore.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,100 @@ describe("usePlayerStore", () => {
7777
});
7878
});
7979

80+
describe("setInPoint", () => {
81+
it("updates inPoint", () => {
82+
usePlayerStore.getState().setInPoint(1.5);
83+
expect(usePlayerStore.getState().inPoint).toBe(1.5);
84+
});
85+
86+
it("clears inPoint when given null", () => {
87+
usePlayerStore.getState().setInPoint(1.5);
88+
usePlayerStore.getState().setInPoint(null);
89+
expect(usePlayerStore.getState().inPoint).toBeNull();
90+
});
91+
92+
it("rejects non-finite values", () => {
93+
usePlayerStore.getState().setInPoint(Number.NaN);
94+
expect(usePlayerStore.getState().inPoint).toBeNull();
95+
});
96+
97+
it("nullifies outPoint when new inPoint is at or past existing outPoint", () => {
98+
usePlayerStore.getState().setOutPoint(2);
99+
usePlayerStore.getState().setInPoint(3);
100+
expect(usePlayerStore.getState().outPoint).toBeNull();
101+
expect(usePlayerStore.getState().inPoint).toBe(3);
102+
});
103+
104+
it("preserves outPoint when new inPoint is before it", () => {
105+
usePlayerStore.getState().setOutPoint(5);
106+
usePlayerStore.getState().setInPoint(2);
107+
expect(usePlayerStore.getState().outPoint).toBe(5);
108+
});
109+
110+
it("auto-enables loopEnabled when set to a non-null value", () => {
111+
usePlayerStore.getState().setLoopEnabled(false);
112+
usePlayerStore.getState().setInPoint(1.5);
113+
expect(usePlayerStore.getState().loopEnabled).toBe(true);
114+
});
115+
116+
it("preserves loopEnabled when cleared with null", () => {
117+
usePlayerStore.getState().setLoopEnabled(true);
118+
usePlayerStore.getState().setInPoint(null);
119+
expect(usePlayerStore.getState().loopEnabled).toBe(true);
120+
121+
usePlayerStore.getState().setLoopEnabled(false);
122+
usePlayerStore.getState().setInPoint(null);
123+
expect(usePlayerStore.getState().loopEnabled).toBe(false);
124+
});
125+
});
126+
127+
describe("setOutPoint", () => {
128+
it("updates outPoint", () => {
129+
usePlayerStore.getState().setOutPoint(4.2);
130+
expect(usePlayerStore.getState().outPoint).toBe(4.2);
131+
});
132+
133+
it("clears outPoint when given null", () => {
134+
usePlayerStore.getState().setOutPoint(4.2);
135+
usePlayerStore.getState().setOutPoint(null);
136+
expect(usePlayerStore.getState().outPoint).toBeNull();
137+
});
138+
139+
it("rejects non-finite values", () => {
140+
usePlayerStore.getState().setOutPoint(Number.POSITIVE_INFINITY);
141+
expect(usePlayerStore.getState().outPoint).toBeNull();
142+
});
143+
144+
it("nullifies inPoint when new outPoint is at or before existing inPoint", () => {
145+
usePlayerStore.getState().setInPoint(5);
146+
usePlayerStore.getState().setOutPoint(3);
147+
expect(usePlayerStore.getState().inPoint).toBeNull();
148+
expect(usePlayerStore.getState().outPoint).toBe(3);
149+
});
150+
151+
it("preserves inPoint when new outPoint is after it", () => {
152+
usePlayerStore.getState().setInPoint(2);
153+
usePlayerStore.getState().setOutPoint(5);
154+
expect(usePlayerStore.getState().inPoint).toBe(2);
155+
});
156+
157+
it("auto-enables loopEnabled when set to a non-null value", () => {
158+
usePlayerStore.getState().setLoopEnabled(false);
159+
usePlayerStore.getState().setOutPoint(4.2);
160+
expect(usePlayerStore.getState().loopEnabled).toBe(true);
161+
});
162+
163+
it("preserves loopEnabled when cleared with null", () => {
164+
usePlayerStore.getState().setLoopEnabled(true);
165+
usePlayerStore.getState().setOutPoint(null);
166+
expect(usePlayerStore.getState().loopEnabled).toBe(true);
167+
168+
usePlayerStore.getState().setLoopEnabled(false);
169+
usePlayerStore.getState().setOutPoint(null);
170+
expect(usePlayerStore.getState().loopEnabled).toBe(false);
171+
});
172+
});
173+
80174
describe("setTimelineReady", () => {
81175
it("updates timelineReady", () => {
82176
usePlayerStore.getState().setTimelineReady(true);

‎packages/studio/src/player/store/playerStore.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ export const usePlayerStore = create<PlayerState>((set) => ({
127127
inPoint: t,
128128
outPoint:
129129
t !== null && state.outPoint !== null && t >= state.outPoint ? null : state.outPoint,
130+
// Setting a work-area marker implies the user wants playback bounded by it.
131+
// Auto-enable loop so the playhead respects the marker instead of running past.
132+
loopEnabled: t !== null ? true : state.loopEnabled,
130133
};
131134
}),
132135
setOutPoint: (time) =>
@@ -135,6 +138,7 @@ export const usePlayerStore = create<PlayerState>((set) => ({
135138
return {
136139
outPoint: t,
137140
inPoint: t !== null && state.inPoint !== null && t <= state.inPoint ? null : state.inPoint,
141+
loopEnabled: t !== null ? true : state.loopEnabled,
138142
};
139143
}),
140144
setManualZoomPercent: (percent) =>

0 commit comments

Comments
 (0)