From 8212576d601db6b84dc287ae7e35e817e9e0583c Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Fri, 12 Jun 2026 17:27:41 +0200 Subject: [PATCH] fix: don't warn with 'Invalid repetitionCount value: 1. Using default: Infinity' for valid modelViewer.appendAnimation(animName, { repetitions: 1 }) It actually just emitted the warning and kept repetitionCount to 1 anyway, the else branch was added in 305e07c120900e63f8d42ff9cf5f2b9647f8241d --- .../test/three-components/ModelScene-spec.ts | 47 ++++++++++++++++++- .../src/three-components/ModelScene.ts | 12 +++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/model-viewer/src/test/three-components/ModelScene-spec.ts b/packages/model-viewer/src/test/three-components/ModelScene-spec.ts index 4b3f36be71..1073df8212 100644 --- a/packages/model-viewer/src/test/three-components/ModelScene-spec.ts +++ b/packages/model-viewer/src/test/three-components/ModelScene-spec.ts @@ -16,7 +16,7 @@ import '../renderer-gate.js'; import {expect} from 'chai'; -import {Matrix4, Mesh, SphereGeometry, Vector3} from 'three'; +import {LoopRepeat, Matrix4, Mesh, SphereGeometry, Vector3} from 'three'; import {$scene} from '../../model-viewer-base.js'; import {ModelViewerElement} from '../../model-viewer.js'; @@ -81,6 +81,51 @@ suite('ModelScene', () => { scene.detachAnimation(animationName, false); expect(scene.appendedAnimations).to.not.include(animationName); }); + + suite('appendAnimation repetitionCount validation', () => { + // Returns the repetitionCount-related warnings emitted while appending the + // first animation with the given repetitionCount. + const repetitionWarnings = (repetitionCount: any): string[] => { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: any[]) => void warnings.push(args.join(' ')); + try { + scene.appendAnimation( + scene.animationNames[0], LoopRepeat, repetitionCount); + } finally { + console.warn = originalWarn; + } + return warnings.filter((w) => w.includes('repetitionCount')); + }; + + test('a valid numeric value does not warn', () => { + expect(repetitionWarnings(1)).to.be.empty; + }); + + test('Infinity (the default) does not warn', () => { + expect(repetitionWarnings(Infinity)).to.be.empty; + }); + + test('a numeric value < 1 warns', () => { + expect(repetitionWarnings(0)).to.have.lengthOf(1); + }); + + test('NaN warns', () => { + expect(repetitionWarnings(NaN)).to.have.lengthOf(1); + }); + + test('a numeric string >= 1 does not warn', () => { + expect(repetitionWarnings('3')).to.be.empty; + }); + + test('a numeric string < 1 warns', () => { + expect(repetitionWarnings('0')).to.have.lengthOf(1); + }); + + test('a non-numeric string warns', () => { + expect(repetitionWarnings('abc')).to.have.lengthOf(1); + }); + }); }); suite('setSize', () => { diff --git a/packages/model-viewer/src/three-components/ModelScene.ts b/packages/model-viewer/src/three-components/ModelScene.ts index 9201d174eb..3cefe8c1cf 100644 --- a/packages/model-viewer/src/three-components/ModelScene.ts +++ b/packages/model-viewer/src/three-components/ModelScene.ts @@ -990,13 +990,17 @@ export class ModelScene extends Scene { } repetitionCount = Math.max(parseInt(repetitionCount), 1); } - } else if (typeof repetitionCount === 'number' && repetitionCount < 1) { - repetitionCount = 1; - console.warn(`Invalid repetitionCount value: ${ - repetitionCount}. Using 1 value as minimum.`); + } else if (typeof repetitionCount === 'number' && !isNaN(repetitionCount)) { + // A valid number >= 1 (including Infinity) is left untouched. + if (repetitionCount < 1) { + console.warn(`Invalid repetitionCount value: ${ + repetitionCount}. Using 1 as minimum.`); + repetitionCount = 1; + } } else { console.warn(`Invalid repetitionCount value: ${ repetitionCount}. Using default: Infinity`); + repetitionCount = Infinity; } if (repetitionCount === 1 && loopMode !== LoopOnce) {