BabylonJS / OldDocumentationSite

Babylon.js documentation page
http://doc.babylonjs.com
Apache License 2.0
66 stars 188 forks source link

ArcRotationCamera.setTarget has different argument type than set target() #2019

Closed valen214 closed 4 years ago

valen214 commented 4 years ago

Quick problem

the two lines below are the implementation arcRotateCamera.ts (line 975): setTarget(target: AbstractMesh | Vector3, ...) takes Vector3 | AbstractMesh as first argument

arcRotateCamera.ts (line 63): set target(value: Vector3) takes only Vector3 as the first argument value while the implementation is literally { this.setTarget(value); }

corresponding typedoc line 42664 for set target()

so it is actually possible to do camera.target = mesh as any

I suggest set target() should include some guidance to notify viewers that camera.target = mesh is possible, or something like "set target() is just an alias tosetTarget()" set target() comes first in the documentation (typedoc), it's reasonable for people skim through and think set target() and setTarget() take the same type then omit the feature setTarget offers. (I think? at least it happened on me :( It took me a day to discover that feature, seeing some PG actually used camera.target = mesh, I was shocked because I thought it can only be Vector3)

edit: oddly setTarget(Vector3) in UniversalCamera has it mentioned, even the parameter type is still the same

valen214 commented 4 years ago

recommend to add incomplete doc label if necessary

deltakosh commented 4 years ago

Well I would not like to complicate things here. The target property is inherited from TargetCamera where only vector3 is supported

And furthermore the set target will be different than the get as the getter only returns vector3

deltakosh commented 4 years ago

I'll add a note though :)

deltakosh commented 4 years ago

"Please note that you can set the target to a mesh and thus the target will be copied from mesh.position"