Cleanup #24181
Cleanup #24181
Conversation
|
Also, is CameraControls even used anymore? It seems to be very similar to OrbitControls. |
|
Thanks! |
I always thought it was over-engineered... I think we can remove it. |
| Material.prototype.copy.call( pointsMaterial, material ); | ||
| pointsMaterial.color.copy( material.color ); | ||
| pointsMaterial.map = material.map; | ||
| pointsMaterial.copy( material ); |
There was a problem hiding this comment.
I don't think this line of code is right. Material.prototype.copy.call( pointsMaterial, material ); only executes the copy() method of the Material class whereas pointsMaterial.copy( material ); executes PointsMaterial.copy(). However, this will lead to undefined material properties if the given material is not of type PointsMaterial.
So using Material.prototype.copy() was done on purpose. /cc @donmccurdy
There was a problem hiding this comment.
Remove prototype accesses
Like I said before, prototype access is not evil per se. At some places, it does make sense and a refactoring is not really required. I think we should only do such changes if they generate an additional value.
There was a problem hiding this comment.
Yes – I'm using Material.prototype.copy as the common denominator to copy properties that can be shared between PointsMaterial and other material types here. Errors can be thrown if the copy method tries to access methods on a Color property that isn't there.
There was a problem hiding this comment.
OK, reverted this change:
#24190
* Cleanup * Fix one super() * Fix another super() * Cleanup webgpu_sprites * Return lost lint line
* Cleanup * Fix one super() * Fix another super() * Cleanup webgpu_sprites * Return lost lint line
Related issue: #24047 (comment)
Description
This PR does many cleanup:
npm run lint-fix(and fix it),