Skip to content

Commit 8b230af

Browse files
committed
Fix double aggregation when seeking playback
1 parent bc02a54 commit 8b230af

File tree

1 file changed

+29
-30
lines changed

1 file changed

+29
-30
lines changed

ui/src/components/Sim/modules/Playback.tsx

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ export const Playback: FC = () => {
1313
// Timeline playback refs
1414
const intervalRef = useRef<number | null>(null);
1515
const lastUpdateRef = useRef<number>(0);
16-
const currentTimeRef = useRef<number>(currentTime);
1716

18-
// Refs for seeking functionality
17+
// Refs for stable seeking callbacks
18+
const currentTimeRef = useRef(currentTime);
1919
const eventsRef = useRef(events);
2020
const maxTimeRef = useRef(maxTime);
2121

@@ -47,15 +47,28 @@ export const Playback: FC = () => {
4747
}, []);
4848

4949
const handleStep = useCallback(
50+
(stepAmount: number) => {
51+
const maxEventTime =
52+
events.length > 0 ? events[events.length - 1].time_s : maxTime;
53+
const newTime = Math.max(
54+
0,
55+
Math.min(currentTime + stepAmount, maxEventTime),
56+
);
57+
dispatch({ type: "SET_TIMELINE_TIME", payload: newTime });
58+
},
59+
[dispatch, events, maxTime, currentTime],
60+
);
61+
62+
// Stable version for seeking intervals (uses refs)
63+
const handleStepForSeeking = useCallback(
5064
(stepAmount: number) => {
5165
const maxEventTime =
5266
eventsRef.current.length > 0
5367
? eventsRef.current[eventsRef.current.length - 1].time_s
5468
: maxTimeRef.current;
55-
const currentTime = currentTimeRef.current;
5669
const newTime = Math.max(
5770
0,
58-
Math.min(currentTime + stepAmount, maxEventTime),
71+
Math.min(currentTimeRef.current + stepAmount, maxEventTime),
5972
);
6073
currentTimeRef.current = newTime;
6174
dispatch({ type: "SET_TIMELINE_TIME", payload: newTime });
@@ -68,17 +81,17 @@ export const Playback: FC = () => {
6881
// Clear any existing seeking first
6982
stopSeeking();
7083

71-
// Initial step using current ref values
84+
// Initial step using current context values
7285
handleStep(stepAmount);
7386

74-
// Start continuous seeking after delay
87+
// Start continuous seeking after delay using stable callback
7588
stepTimeoutRef.current = window.setTimeout(() => {
7689
stepIntervalRef.current = window.setInterval(() => {
77-
handleStep(stepAmount);
90+
handleStepForSeeking(stepAmount);
7891
}, 33); // ~30 FPS smooth seeking
7992
}, 300); // initial delay
8093
},
81-
[handleStep, stopSeeking],
94+
[handleStep, handleStepForSeeking, stopSeeking],
8295
);
8396

8497
// Timeline playback effect - handles automatic advancement when playing
@@ -91,6 +104,9 @@ export const Playback: FC = () => {
91104
clearInterval(intervalRef.current);
92105
}
93106

107+
// Capture current time at interval start to avoid stale closure
108+
let localCurrentTime = currentTime;
109+
94110
// Start playback interval
95111
intervalRef.current = window.setInterval(() => {
96112
const now = performance.now();
@@ -99,16 +115,10 @@ export const Playback: FC = () => {
99115
((now - lastUpdateRef.current) / 1000) * speedMultiplier;
100116
lastUpdateRef.current = now;
101117

102-
const newTime = Math.min(
103-
currentTimeRef.current + deltaTime,
104-
maxEventTime,
105-
);
106-
currentTimeRef.current = newTime;
118+
const newTime = Math.min(localCurrentTime + deltaTime, maxEventTime);
119+
localCurrentTime = newTime;
107120

108-
dispatch({
109-
type: "SET_TIMELINE_TIME",
110-
payload: newTime,
111-
});
121+
dispatch({ type: "SET_TIMELINE_TIME", payload: newTime });
112122

113123
// Auto-pause at the end
114124
if (newTime >= maxEventTime) {
@@ -124,16 +134,9 @@ export const Playback: FC = () => {
124134
intervalRef.current = null;
125135
}
126136
}
127-
}, [
128-
isPlaying,
129-
events.length,
130-
currentTime,
131-
speedMultiplier,
132-
dispatch,
133-
stopSeeking,
134-
]);
137+
}, [isPlaying, events.length, speedMultiplier, dispatch]);
135138

136-
// Keep refs in sync when values change externally
139+
// Keep refs in sync with context values
137140
useEffect(() => {
138141
currentTimeRef.current = currentTime;
139142
lastUpdateRef.current = performance.now();
@@ -245,7 +248,6 @@ export const Playback: FC = () => {
245248

246249
<Button
247250
ref={stepBigBackwardRef}
248-
onClick={() => handleStep(-1.0 * speedMultiplier)}
249251
onMouseDown={(e) => {
250252
e.preventDefault();
251253
startSeeking(-1.0 * speedMultiplier);
@@ -264,7 +266,6 @@ export const Playback: FC = () => {
264266

265267
<Button
266268
ref={stepSmallBackwardRef}
267-
onClick={() => handleStep(-0.1 * speedMultiplier)}
268269
onMouseDown={(e) => {
269270
e.preventDefault();
270271
startSeeking(-0.1 * speedMultiplier);
@@ -283,7 +284,6 @@ export const Playback: FC = () => {
283284

284285
<Button
285286
ref={stepSmallForwardRef}
286-
onClick={() => handleStep(0.1 * speedMultiplier)}
287287
onMouseDown={(e) => {
288288
e.preventDefault();
289289
startSeeking(0.1 * speedMultiplier);
@@ -302,7 +302,6 @@ export const Playback: FC = () => {
302302

303303
<Button
304304
ref={stepBigForwardRef}
305-
onClick={() => handleStep(1.0 * speedMultiplier)}
306305
onMouseDown={(e) => {
307306
e.preventDefault();
308307
startSeeking(1.0 * speedMultiplier);

0 commit comments

Comments
 (0)