diff --git a/packages/model-viewer/src/features/animation.ts b/packages/model-viewer/src/features/animation.ts index 9ff94831f9..402ccbde9c 100644 --- a/packages/model-viewer/src/features/animation.ts +++ b/packages/model-viewer/src/features/animation.ts @@ -25,6 +25,7 @@ const $changeAnimation = Symbol('changeAnimation'); const $appendAnimation = Symbol('appendAnimation'); const $detachAnimation = Symbol('detachAnimation'); const $paused = Symbol('paused'); +const $currentAnimationOptions = Symbol('currentAnimationOptions'); interface PlayAnimationOptions { repetitions: number; @@ -97,6 +98,13 @@ export const AnimationMixin = >( animationCrossfadeDuration: number = 300; protected[$paused]: boolean = true; + // Remembers the options from the most recent play() call so re-applying the + // current animation (e.g. when animationName changes) preserves them rather + // than resetting to an infinite loop. Without this, `animationName = name` + // followed by `play({repetitions: 1})` loops forever and never emits + // 'finished', because the asynchronous animationName update re-triggers + // playback with the default (infinite) options. + private[$currentAnimationOptions]: PlayAnimationOptions = DEFAULT_PLAY_OPTIONS; constructor(...args: any[]) { super(args); @@ -189,7 +197,8 @@ export const AnimationMixin = >( const modelIndex = options?.modelIndex ?? null; this[$scene].unpauseAnimation(modelIndex); - this[$changeAnimation](options); + this[$currentAnimationOptions] = {...DEFAULT_PLAY_OPTIONS, ...options}; + this[$changeAnimation](this[$currentAnimationOptions]); this.dispatchEvent(new CustomEvent('play')); } @@ -256,9 +265,14 @@ export const AnimationMixin = >( } } - [$changeAnimation](options: PlayAnimationOptions = DEFAULT_PLAY_OPTIONS) { - const repetitions = options.repetitions ?? Infinity; - const mode = options.pingpong ? + [$changeAnimation](options?: PlayAnimationOptions) { + // When re-triggered without explicit options (e.g. from an animationName + // change or onModelLoad), reuse the options from the most recent play() + // so an in-flight `play({repetitions: 1})` is not overwritten with the + // infinite-loop default. + const opts = options ?? this[$currentAnimationOptions]; + const repetitions = opts.repetitions ?? Infinity; + const mode = opts.pingpong ? LoopPingPong : (repetitions === 1 ? LoopOnce : LoopRepeat); this[$scene].playAnimation( @@ -266,7 +280,7 @@ export const AnimationMixin = >( this.animationCrossfadeDuration / MILLISECONDS_PER_SECOND, mode, repetitions, - options.modelIndex); + opts.modelIndex); // If we are currently paused, we need to force a render so that // the scene updates to the first frame of the new animation diff --git a/packages/model-viewer/src/test/features/animation-spec.ts b/packages/model-viewer/src/test/features/animation-spec.ts index d4eb6502bf..4571d696e8 100644 --- a/packages/model-viewer/src/test/features/animation-spec.ts +++ b/packages/model-viewer/src/test/features/animation-spec.ts @@ -81,6 +81,46 @@ suite('Animation', () => { expect(element.paused).to.be.true; }); + test('dispatches "finished" when a repetition-limited animation completes', + async () => { + element.play({repetitions: 1, pingpong: false}); + await element.updateComplete; + + const finished = waitForEvent(element, 'finished'); + // Jump near the end so the LoopOnce action completes quickly (the + // default clip is longer than the test timeout). + element.currentTime = Math.max(element.duration - 0.05, 0); + await timePasses(500); + await finished; + }); + + // Regression test: setting `animationName` schedules an async reactive + // update whose updated() re-triggers playback. It used to do so with the + // default (infinite-loop) options, clobbering a `play({repetitions: 1})` + // issued in the same tick, so the animation looped forever and never + // emitted 'finished'. + test('dispatches "finished" when animationName is set then played once', + async () => { + const target = element.availableAnimations.find( + (name) => name !== element.animationName) ?? + element.availableAnimations[0]; + + element.animationName = target; + // Restart from the first frame, mirroring how a one-time animation is + // typically (re)played after a previous one finished and clamped. + element.currentTime = 0; + element.play({repetitions: 1, pingpong: false}); + + // Flush the animationName update, where the clobber used to happen. + await element.updateComplete; + + const finished = waitForEvent(element, 'finished'); + // Jump near the end so the LoopOnce action completes quickly. + element.currentTime = Math.max(element.duration - 0.05, 0); + await timePasses(500); + await finished; + }); + suite('when play is invoked with no options', () => { setup(async () => { const animationsPlay = waitForEvent(element, 'play'); diff --git a/packages/model-viewer/src/three-components/ModelScene.ts b/packages/model-viewer/src/three-components/ModelScene.ts index 9201d174eb..6fc539d3f6 100644 --- a/packages/model-viewer/src/three-components/ModelScene.ts +++ b/packages/model-viewer/src/three-components/ModelScene.ts @@ -120,6 +120,12 @@ export class ModelScene extends Scene { private boundsAndShadowDirty = false; private mixers: AnimationMixer[] = []; private mixerPausedStates: boolean[] = []; + // Mixer event subscriptions are kept so they can be re-applied to the mixers + // that are recreated on every setSource() (one per loaded glTF). + private mixerEventSubscriptions: Array<{ + event: keyof AnimationMixerEventMap, + callback: (...args: any[]) => void + }> = []; private cancelPendingSourceChange: (() => void)|null = null; private animationsByName: Map = new Map(); private currentAnimationActions: (AnimationAction|null)[] = []; @@ -281,11 +287,11 @@ export class ModelScene extends Scene { if (gltf != null) { this._models.push(gltf.scene); this.target.add(gltf.scene); - this.mixers.push(new AnimationMixer(gltf.scene)); + this.mixers.push(this.createMixer(gltf.scene)); this.mixerPausedStates.push(false); this.currentAnimationActions.push(null); } else { - this.mixers.push(new AnimationMixer(this.target)); + this.mixers.push(this.createMixer(this.target)); this.mixerPausedStates.push(false); this.currentAnimationActions.push(null); } @@ -1282,11 +1288,25 @@ export class ModelScene extends Scene { subscribeMixerEvent( event: keyof AnimationMixerEventMap, callback: (...args: any[]) => void) { + // Remember the subscription so it survives the mixers being recreated on + // the next setSource(), then apply it to the mixers that already exist. + this.mixerEventSubscriptions.push({event, callback}); for (const mixer of this.mixers) { mixer.addEventListener(event, callback); } } + // Creates an AnimationMixer with the currently subscribed events already + // attached, so events (e.g. 'finished', 'loop') keep firing after a new + // model is loaded. + private createMixer(root: Object3D): AnimationMixer { + const mixer = new AnimationMixer(root); + for (const {event, callback} of this.mixerEventSubscriptions) { + mixer.addEventListener(event, callback); + } + return mixer; + } + /** * Call if the object has been changed in such a way that the shadow's * shape has changed (not a rotation about the Y axis). diff --git a/packages/modelviewer.dev/data/examples.json b/packages/modelviewer.dev/data/examples.json index 28e733f1fd..6595e2b2c8 100644 --- a/packages/modelviewer.dev/data/examples.json +++ b/packages/modelviewer.dev/data/examples.json @@ -221,6 +221,10 @@ "htmlId": "controlSpeed", "name": "Control Speed" }, + { + "htmlId": "playOnce", + "name": "Play Once" + }, { "htmlId": "crossFade", "name": "Crossfade" diff --git a/packages/modelviewer.dev/examples/animation/index.html b/packages/modelviewer.dev/examples/animation/index.html index 420b88a616..bcc4cf9005 100644 --- a/packages/modelviewer.dev/examples/animation/index.html +++ b/packages/modelviewer.dev/examples/animation/index.html @@ -161,6 +161,64 @@

+
+
+
+
+
+

Play an animation a single time and react to the finished event

+

+
+ + + +
+
+
+