-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
Description
When doing some optimisations and extracting the vectors to make them reusable, I was left with such sample code:
const empty = new Vector3(); // Use as a default-placeholder for any vector
const up = new Vector3(0, 1, 0); // Vector y-up
const down = new Vector3(0, -1, 0); // Vector y-down
const position = new Vector3(); // Use as a target for world positions
const raycaster = new Raycaster(empty, empty, 0, 20); // Initialize raycaster outside the loop to relieve poor GC, set empty values for now
// Attempt to test each mesh from some arbitrary array
myMeshes.forEach(mesh => {
mesh.getWorldPosition(position); // position is set to (x1, y1, z1)
raycaster.set(position, down); // Since method is called "set", we'd expect the raycaster to be placed at (x1, y1, z1), pointing downward
// Raycasts past this point will ignore mesh position and likely fail, not in an very obvious way 🙈
const hits = raycaster.intersectObject(groundMesh); // Except some specific cases, this will return 0 hits
});Despite using set as a name, Raycaster.set uses Vector3.copy under the hood to update the Ray origin and direction - ie. it preserves the original vectors. In the example above, for each iteration value of empty is updated twice - then assigned as both origin and direction.
Wouldn't it be a bit more intuitive / truthy to use Vector3.set to update origin and direction? Currently docs say the following:
Raycaster.set: Updates the ray with a new origin and direction.While it seems it does something more of (based on Vector.copy):
Raycaster.set: Copies the passed vector values into existing ray origin and direction.It is not really a terrible issue, but docs seem a bit misleading, and it can lead to a bit of debugging confusion - especially since the code above looks perfectly fine, unless one finds their way to these lines.