search

pmndrs / three-stdlib

📚 Stand-alone library of threejs examples designed to run without transpilation in node & browser
https://npmjs.com/three-stdlib
MIT License
676 stars 110 forks source link

`mergeBufferGeometries` give error for empty array #331

Open sylee957 opened 6 months ago

sylee957 commented 6 months ago

Problem description:

mergeBufferGeometries gives error Uncaught TypeError: Cannot read properties of undefined (reading 'index') It is from some sloppy list access

https://github.com/pmndrs/three-stdlib/blame/45e22a1b636241147934e8bd9f22d506ccfc3cf0/src/utils/BufferGeometryUtils.ts#L27

which also exists in three.js upstream

https://github.com/mrdoob/three.js/blob/de6dd45d7e5aa58fed0fbc1dbe53def3402b39cc/examples/jsm/utils/BufferGeometryUtils.js#L109C20-L109C51

Relevant code:

Suggested solution:

I generally would return some reasonable identity object for such cases than raising sloppy errors. Maybe null can be reasonable.

Or otherwise, it may be useful to denote the types as [BufferGeometry, ...BufferGeometry[]] instead of BufferGeometry[], where typescript can just warn users that users should pass array with at least one element.

CodyJasonBennett commented 6 months ago

three.js will tell you to validate your inputs, so this is working as intended there. I do agree with your solution, but would prefer to not change the type (rather than adding a runtime guard).

CodyJasonBennett commented 6 months ago

Related: https://github.com/mrdoob/three.js/issues/27509.