processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.51k stars 3.29k forks source link

Incorrect FES message for normal() #6982

Closed nickmcintyre closed 5 months ago

nickmcintyre commented 5 months ago

Most appropriate sub-area of p5.js?

p5.js version

1.9.2

Web browser and version

Chrome 124.0.6367.60

Operating system

macOS 14.4.1

Steps to reproduce this

Steps:

  1. Create a p5.Vector object.
  2. Pass the object to normal().

Snippet:

// Create a p5.Vector object.
let n = createVector(-0.4, -0.4, 0.8);

// Draw the shape.
// Use normal() to set vertex normals.
beginShape();
normal(n);
vertex(-30, -30, 0);
// ...rest of the shape.
endShape();

I noticed this while working on the reference. Passing a p5.Vector to normal() generates the following FES message:

🌸 p5.js says: [sketch.js, line 29] normal() was expecting Vector
for the first parameter, received object instead. (http://p5js.org/reference/#/p5/normal)

See the full sketch.

welcome[bot] commented 5 months ago

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

kaiserarg commented 5 months ago

Hey,

I was able to replicate this issue and possibly found a fix. It seems that in src\core\shape\vertex.js for the normal() method the documentation was written as @param {Vector} vector and since Vector isn't a native JS class when the FES couldn't use instanceOf to check and typeOf checked the e.g. in your example let n = createVector(-0.4, -0.4, 0.8); which would return object since Vector is not a native JS class.

A quick and easy fix would be to change @param {Vector} vector to @param {p5.Vector} vector so FES can correctly check the parameter.

I linked a PR below that would fix this issue. Let me know if there is any issue with it.

Thanks!