Skip to content

Commit 3df671e

Browse files
committed
fix(studio): an older failed save never settles over a newer edit on the same lane
Live-preview bookkeeping moves to createLiveLanes, one owner for both timeline saves: each lane key carries a generation that every live write and save start advances, and a save settles the preview and store (on its landed value, or on the file after a failure) only while it is still the newest. A paste marks its span only if it is still the latest paste.
1 parent b71748f commit 3df671e

6 files changed

Lines changed: 127 additions & 41 deletions

‎packages/studio/src/hooks/timelineAudioGroupVolume.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { invalidateGroupInfoCache } from "../player/lib/timelineGroupInfo";
1212
import {
1313
buildPatchTarget,
1414
persistElementAttribute,
15-
claimLiveBefore,
15+
createLiveLanes,
1616
readSavedAttribute,
1717
type RecordEditInput,
1818
} from "./timelineEditingHelpers";
@@ -238,14 +238,12 @@ export function useSetAudioGroupAttribute({
238238
) => Promise<TimelineEditOutcome>;
239239
revertLive: (groupId: string, attr: string) => void;
240240
} {
241-
const liveBeforeRef = useRef(new Map<string, string | null>());
241+
const liveLanes = useRef(createLiveLanes());
242242
const setLive = useCallback(
243243
(groupId: string, attr: string, value: string | null) => {
244244
const key = audioGroupAttributeLiveKey(groupId, attr);
245245
const target = previewIframeRef.current?.contentDocument?.getElementById(groupId);
246-
if (!liveBeforeRef.current.has(key)) {
247-
liveBeforeRef.current.set(key, target?.getAttribute(attr) ?? null);
248-
}
246+
liveLanes.current.preview(key, () => target?.getAttribute(attr) ?? null);
249247
patchLiveGroupAttribute(previewIframeRef.current, groupId, attr, value);
250248
// Live too, not just on commit: a fader drag is `setLive` per frame and
251249
// `setQuiet` once on release, so without this the strip's own readout
@@ -256,7 +254,7 @@ export function useSetAudioGroupAttribute({
256254
);
257255
const claimLive = useCallback(
258256
(groupId: string, attr: string) =>
259-
claimLiveBefore(liveBeforeRef.current, audioGroupAttributeLiveKey(groupId, attr), (value) => {
257+
liveLanes.current.claim(audioGroupAttributeLiveKey(groupId, attr), (value) => {
260258
patchLiveGroupAttribute(previewIframeRef.current, groupId, attr, value);
261259
syncStoredGroupAttribute(groupId, attr, value);
262260
}),
@@ -310,7 +308,7 @@ export function useSetAudioGroupAttribute({
310308
});
311309
if (!written)
312310
return unsaved(failedTimelineSave("This group has no id to save it by", showToast));
313-
syncStoredGroupAttribute(groupId, attr, value);
311+
settleLive(value);
314312
return { status: "saved" };
315313
} catch (error) {
316314
console.error("[Timeline] Failed to set group attribute", error);

‎packages/studio/src/hooks/timelineEditingHelpers.ts‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -397,19 +397,32 @@ export async function persistTimelineBatchEdit(
397397
});
398398
}
399399

400-
/** Claim `key`'s before-value; the call settles on `saved` (else the claim), sparing a newer gesture. */
401-
export function claimLiveBefore(
402-
liveBefore: Map<string, string | null>,
403-
key: string,
404-
apply: (value: string | null) => void,
405-
): (saved?: string | null) => void {
406-
const claimed = liveBefore.has(key) ? (liveBefore.get(key) ?? null) : undefined;
407-
liveBefore.delete(key);
408-
return (saved) => {
409-
const value = saved !== undefined ? saved : claimed;
410-
if (value === undefined) return;
411-
if (liveBefore.has(key)) liveBefore.set(key, value);
412-
else apply(value);
400+
/** Live-preview bookkeeping per lane: the value before a gesture, and which gesture is newest. */
401+
export function createLiveLanes() {
402+
const before = new Map<string, string | null>();
403+
const generation = new Map<string, number>();
404+
const bump = (key: string): number => {
405+
const next = (generation.get(key) ?? 0) + 1;
406+
generation.set(key, next);
407+
return next;
408+
};
409+
return {
410+
preview(key: string, readCurrent: () => string | null): void {
411+
if (!before.has(key)) before.set(key, readCurrent());
412+
bump(key);
413+
},
414+
// The returned call settles on `saved` (else the claimed before-value), unless a
415+
// newer gesture or save has touched the lane since.
416+
claim(key: string, apply: (value: string | null) => void): (saved?: string | null) => void {
417+
const claimed = before.has(key) ? (before.get(key) ?? null) : undefined;
418+
before.delete(key);
419+
const mine = bump(key);
420+
return (saved) => {
421+
if (generation.get(key) !== mine) return;
422+
const value = saved !== undefined ? saved : claimed;
423+
if (value !== undefined) apply(value);
424+
};
425+
},
413426
};
414427
}
415428

‎packages/studio/src/hooks/timelineElementFxAttribute.ts‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ import type { TimelineElement } from "../player";
1212
import {
1313
buildPatchTarget,
1414
findTimelineElementInIframe,
15+
createLiveLanes,
1516
persistElementAttribute,
16-
claimLiveBefore,
1717
readSavedAttribute,
1818
} from "./timelineEditingHelpers";
1919
import type {
@@ -116,28 +116,22 @@ export function useSetElementAttribute({
116116
) => Promise<TimelineEditOutcome>;
117117
revertLive: (element: TimelineElement, attr: string) => void;
118118
} {
119-
const liveBeforeRef = useRef(new Map<string, string | null>());
119+
const liveLanes = useRef(createLiveLanes());
120120
const setLive = useCallback(
121121
(element: TimelineElement, attr: string, value: string | null) => {
122122
const key = elementAttributeLiveKey(element, activeCompPath, attr);
123123
const target = findTimelineElementInIframe(previewIframeRef.current, element, activeCompPath);
124-
if (!liveBeforeRef.current.has(key)) {
125-
liveBeforeRef.current.set(key, target?.getAttribute(attr) ?? null);
126-
}
124+
liveLanes.current.preview(key, () => target?.getAttribute(attr) ?? null);
127125
patchLiveElementAttribute(previewIframeRef.current, element, attr, value, activeCompPath);
128126
},
129127
[previewIframeRef, activeCompPath],
130128
);
131129
const claimLive = useCallback(
132130
(element: TimelineElement, attr: string) =>
133-
claimLiveBefore(
134-
liveBeforeRef.current,
135-
elementAttributeLiveKey(element, activeCompPath, attr),
136-
(value) => {
137-
patchLiveElementAttribute(previewIframeRef.current, element, attr, value, activeCompPath);
138-
syncStoredAutomationFromPreview(previewIframeRef.current?.contentDocument);
139-
},
140-
),
131+
liveLanes.current.claim(elementAttributeLiveKey(element, activeCompPath, attr), (value) => {
132+
patchLiveElementAttribute(previewIframeRef.current, element, attr, value, activeCompPath);
133+
syncStoredAutomationFromPreview(previewIframeRef.current?.contentDocument);
134+
}),
141135
[previewIframeRef, activeCompPath],
142136
);
143137
const revertLive = useCallback(
@@ -182,7 +176,7 @@ export function useSetElementAttribute({
182176
});
183177
if (!written)
184178
return unsaved(failedTimelineSave("This clip has no id to save it by", showToast));
185-
syncStoredAutomationFromPreview(previewIframeRef.current?.contentDocument);
179+
settleLive(value);
186180
return { status: "saved" };
187181
} catch (error) {
188182
console.error("[Timeline] Failed to set element attribute", error);

‎packages/studio/src/hooks/useAutomationSelectionKeyboard.test.tsx‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,41 @@ describe("useAutomationSelectionKeyboard", () => {
320320
},
321321
);
322322

323+
it.each(["in order", "reversed"])(
324+
"marks the latest paste when pastes on two clips resolve %s",
325+
async (order) => {
326+
clearAutomationClipboard();
327+
usePlayerStore.setState({
328+
elements: [
329+
{ ...bgmElement, duration: 10 },
330+
{ ...bgmElement, id: "vo", key: "vo", duration: 10 },
331+
],
332+
selectedElementId: "bgm",
333+
currentTime: 1,
334+
});
335+
usePlayerStore
336+
.getState()
337+
.setAutomationSelection(wholeAxis({ elementKey: "bgm", target: "volume", t0: 2, t1: 4 }));
338+
const lands: Array<() => void> = [];
339+
const onCommit = vi.fn(
340+
() =>
341+
new Promise<{ status: "saved" }>((resolve) =>
342+
lands.push(() => resolve({ status: "saved" })),
343+
),
344+
);
345+
setup({ onCommit });
346+
combo("c");
347+
usePlayerStore.getState().clearAutomationSelection();
348+
combo("v");
349+
usePlayerStore.setState({ selectedElementId: "vo" });
350+
combo("v");
351+
const order_ = order === "in order" ? [0, 1] : [1, 0];
352+
for (const i of order_) lands[i]?.();
353+
await act(async () => {});
354+
expect(usePlayerStore.getState().automationSelection?.elementKey).toBe("vo");
355+
},
356+
);
357+
323358
it("chains a second Cmd+V after the first instead of overwriting it", async () => {
324359
// The regression this pins: paste leaves its own span selected, so anchoring
325360
// at sel.t0 unconditionally made every later press recompute the same atT.

‎packages/studio/src/hooks/useAutomationSelectionKeyboard.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ function pasteAnchor(
213213
return clampNumber(raw, 0, element.duration - span);
214214
}
215215

216+
let latestPaste = 0;
217+
216218
/**
217219
* Cmd/Ctrl+V: paste the clipboard onto the selected clip's lane, at the active
218220
* selection or the playhead. Returns false (untouched event) when the chord
@@ -243,6 +245,7 @@ function handlePaste(
243245
const saved = paste.binding.onCommit(
244246
withLane(paste.binding.automation, { target: paste.target, points }),
245247
);
248+
const seq = ++latestPaste;
246249
const mark = {
247250
elementKey: paste.elementKey,
248251
target: paste.target,
@@ -251,13 +254,13 @@ function handlePaste(
251254
v0: paste.range.min,
252255
v1: paste.range.max,
253256
};
254-
// Once it lands, and only over the selection it was pasted at, select and mark the
255-
// full-height span: the feedback that it landed, what a second Cmd+V chains after,
256-
// and what Delete takes back in one press. A refused paste marks nothing.
257+
// Once the latest paste lands, and only over the selection it was pasted at, select
258+
// and mark its full-height span: the feedback that it landed, what a second Cmd+V
259+
// chains after, and what Delete takes back in one press.
257260
void saved.then((outcome) => {
258261
if (outcome && outcome.status !== "saved") return;
259262
const current = usePlayerStore.getState();
260-
if (current.automationSelection !== sel) return;
263+
if (seq !== latestPaste || current.automationSelection !== sel) return;
261264
current.setAutomationSelection(mark);
262265
markLastPaste(mark);
263266
});

‎packages/studio/src/player/components/useAutomationLanes.save.test.tsx‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,21 @@ afterEach(() => {
4141

4242
function mountLanes(target: TimelineElement, canEdit?: CanEdit, recording = false) {
4343
let file = SOURCE;
44+
let reads = 0;
45+
const held = new Map<number, Promise<void>>();
4446
vi.stubGlobal(
4547
"fetch",
46-
vi.fn(async (input: Parameters<typeof fetch>[0]) =>
47-
requestUrl(input).includes("/files/") ? jsonResponse({ content: file }) : jsonResponse({}),
48-
),
48+
vi.fn(async (input: Parameters<typeof fetch>[0]) => {
49+
if (!requestUrl(input).includes("/files/")) return jsonResponse({});
50+
await held.get(++reads);
51+
return jsonResponse({ content: file });
52+
}),
4953
);
54+
const holdRead = (n: number) => {
55+
let release = () => {};
56+
held.set(n, new Promise<void>((resolve) => (release = resolve)));
57+
return release;
58+
};
5059
const iframe = document.createElement("iframe");
5160
document.body.append(iframe);
5261
iframe.contentDocument!.body.innerHTML = SOURCE;
@@ -119,6 +128,9 @@ function mountLanes(target: TimelineElement, canEdit?: CanEdit, recording = fals
119128
selection,
120129
iframe,
121130
setFile: (next: string) => (file = next),
131+
file: () => file,
132+
reads: () => reads,
133+
holdRead,
122134
};
123135
}
124136

@@ -227,6 +239,37 @@ describe("useAutomationLanes saves report what happened", () => {
227239
expect(usePlayerStore.getState().elements[0]?.automation).toBe(saved);
228240
});
229241

242+
it("never lets an older failed save's recovery land over a newer save", async () => {
243+
const { commit, startCommit, preview, writeProjectFile, iframe, file, reads, holdRead } =
244+
mountLanes(music);
245+
const at = (v: number) => ({
246+
version: 1 as const,
247+
lanes: [{ target: "volume", points: [{ t: 0, v }] }],
248+
});
249+
expect(await commit(at(0.5))).toEqual({ status: "saved" });
250+
const releaseRecovery = holdRead(3);
251+
writeProjectFile.mockRejectedValueOnce(new Error("offline"));
252+
preview(at(0.9));
253+
const older = startCommit(at(0.9));
254+
await act(() => vi.waitFor(() => expect(reads()).toBe(3)));
255+
preview(at(0.7));
256+
const newer = startCommit(at(0.7));
257+
await act(() => vi.waitFor(() => expect(reads()).toBe(4)));
258+
releaseRecovery();
259+
let outcomes: unknown[] = [];
260+
await act(async () => {
261+
outcomes = await Promise.all([older, newer]);
262+
});
263+
expect(outcomes).toMatchObject([{ status: "failed" }, { status: "saved" }]);
264+
const landed = serializeAutomation(at(0.7));
265+
const saved = new DOMParser().parseFromString(file(), "text/html");
266+
expect(saved.getElementById("music")?.getAttribute("data-automation")).toBe(landed);
267+
expect(iframe.contentDocument!.getElementById("music")?.getAttribute("data-automation")).toBe(
268+
landed,
269+
);
270+
expect(usePlayerStore.getState().elements[0]?.automation).toBe(landed);
271+
});
272+
230273
it("settles on a queued save that lands after an earlier one fails", async () => {
231274
const { startCommit, preview, writeProjectFile, iframe, setFile } = mountLanes(music);
232275
let failFirst = () => {};

0 commit comments

Comments
 (0)