Skip to content

Commit 12a321a

Browse files
committed
fix: emit finished event for one time animation, regression from 4.3.0 with the multi-model refactor
The multi-model refactor recreates the animation mixers on every setSource(), but the mixer event subscriptions (registered once when the animation feature is constructed, before any model is loaded) were not re-applied to the new mixers, so the 'finished' event was never dispatched.
1 parent 33e5562 commit 12a321a

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

packages/model-viewer/src/test/features/animation-spec.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,15 @@ suite('Animation', () => {
8181
expect(element.paused).to.be.true;
8282
});
8383

84+
test('dispatches "finished" when a repetition-limited animation completes',
85+
async () => {
86+
const finished = waitForEvent(element, 'finished');
87+
element.play({repetitions: 1, pingpong: false});
88+
// Let the LoopOnce action run to completion.
89+
await timePasses((element.duration + TOLERANCE_SEC) * 1000);
90+
await finished;
91+
});
92+
8493
suite('when play is invoked with no options', () => {
8594
setup(async () => {
8695
const animationsPlay = waitForEvent(element, 'play');

packages/model-viewer/src/three-components/ModelScene.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,12 @@ export class ModelScene extends Scene {
120120
private boundsAndShadowDirty = false;
121121
private mixers: AnimationMixer[] = [];
122122
private mixerPausedStates: boolean[] = [];
123+
// Mixer event subscriptions are kept so they can be re-applied to the mixers
124+
// that are recreated on every setSource() (one per loaded glTF).
125+
private mixerEventSubscriptions: Array<{
126+
event: keyof AnimationMixerEventMap,
127+
callback: (...args: any[]) => void
128+
}> = [];
123129
private cancelPendingSourceChange: (() => void)|null = null;
124130
private animationsByName: Map<string, AnimationClip> = new Map();
125131
private currentAnimationActions: (AnimationAction|null)[] = [];
@@ -281,11 +287,11 @@ export class ModelScene extends Scene {
281287
if (gltf != null) {
282288
this._models.push(gltf.scene);
283289
this.target.add(gltf.scene);
284-
this.mixers.push(new AnimationMixer(gltf.scene));
290+
this.mixers.push(this.createMixer(gltf.scene));
285291
this.mixerPausedStates.push(false);
286292
this.currentAnimationActions.push(null);
287293
} else {
288-
this.mixers.push(new AnimationMixer(this.target));
294+
this.mixers.push(this.createMixer(this.target));
289295
this.mixerPausedStates.push(false);
290296
this.currentAnimationActions.push(null);
291297
}
@@ -1282,11 +1288,25 @@ export class ModelScene extends Scene {
12821288

12831289
subscribeMixerEvent(
12841290
event: keyof AnimationMixerEventMap, callback: (...args: any[]) => void) {
1291+
// Remember the subscription so it survives the mixers being recreated on
1292+
// the next setSource(), then apply it to the mixers that already exist.
1293+
this.mixerEventSubscriptions.push({event, callback});
12851294
for (const mixer of this.mixers) {
12861295
mixer.addEventListener(event, callback);
12871296
}
12881297
}
12891298

1299+
// Creates an AnimationMixer with the currently subscribed events already
1300+
// attached, so events (e.g. 'finished', 'loop') keep firing after a new
1301+
// model is loaded.
1302+
private createMixer(root: Object3D): AnimationMixer {
1303+
const mixer = new AnimationMixer(root);
1304+
for (const {event, callback} of this.mixerEventSubscriptions) {
1305+
mixer.addEventListener(event, callback);
1306+
}
1307+
return mixer;
1308+
}
1309+
12901310
/**
12911311
* Call if the object has been changed in such a way that the shadow's
12921312
* shape has changed (not a rotation about the Y axis).

0 commit comments

Comments
 (0)