vertexNormal() is not defined

[GregStanton]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build Process
• Unit Testing
• Internalization
• Friendly Errors
• Other (specify if possible)

p5.js version

1.9.0

Steps to reproduce this

Run the example code from the normal() reference page, replacing calls to normal() with calls to vertexNormal().

Snippet:

function setup() {
  createCanvas(100, 100, WEBGL);
  noStroke();
}

function draw() {
  background(255);
  rotateY(frameCount / 100);
  normalMaterial();
  beginShape(TRIANGLE_STRIP);
  vertexNormal(-0.4, 0.4, 0.8);
  vertex(-30, 30, 0);

  vertexNormal(0, 0, 1);
  vertex(-30, -30, 30);
  vertex(30, 30, 30);

  vertexNormal(0.4, -0.4, 0.8);
  vertex(30, -30, 0);
  endShape();
}

Console error

ReferenceError: vertexNormal is not defined

Suggested solution

Based on the reference, the two functions vertexNormal() and normal() serve the same purpose. Inspecting the source reveals that there's actually only one function; the reference page for the other function appears to have been generated by inline docs that use the wrong function name.

Specifically, there's a reference page for vertexNormal(), but that function is never defined in the p5.js codebase. The reference page is a stub that points to a file in the source that names vertexNormal() in the inline docs but implements the documented function with the name normal(). The naming mismatch between the inline docs and the implementation seems to have been present since this functionality was first introduced in commit 072fac9. Fortunately, the mismatch is only present internally, in the version of the feature defined on p5.RendererGL.

Like the internal version, the user-facing version is implemented with normal() as the function name. Its reference page is complete and uses the correct function name. So, this bug seems to have an easy fix: Replace vertexNormal in the documentation on lines 170 and 176 of webgl/p5.RendererGL.Immediate.js with normal. Since this is the only mention of vertexNormal() in the inline docs, I'm guessing this should effectively delete the associated page from the p5.js website's reference.

[diyaayay]

I would like to work on this issue. Thanks:)

[diyaayay]

@GregStanton, the changes that are suggested above produced the following warning on running npm test:

which I think indicates the way function overloads are documented and defined across different files in the p5.js codebase.
It suggests that all the overloads of the p5.normal() function should be defined in the same file, but currently, they are spread across different files. I tried running npm test --force to look into how this can be fixed but that aborts too. The files mentioned in the warning are:

src\webgl\p5.RendererGL.Immediate.js
src\core\shape\vertex.js

So, going about the same solution that you provided or should we be looking into other ways?

[GregStanton]

Hi @diyaayay! Thanks for working on this.

My guess is that we just need to include * @private (as in the YUIDoc syntax reference) on a separate line between lines 169 and 170 in the current version of p5.RendererGL.Immediate.js. I think this will prevent a conflict between the docs for the user-facing normal() function and the docs for the internal normal() function. This is how the docs work for vertex() in vertex.js and vertex() in p5.RendererGL.Immediate.js, for example.

Maybe @limzykenneth can confirm that this is all we need to do?

[diyaayay]

@GregStanton Thanks for providing the document for syntax reference. It explains a lot and will be helpful for my future contributions. The 'npm test' runs successfully after making the changes you suggested. I'll raise a PR and will make any necessary adjustments if somebody suggests. Thanks again:)

[GregStanton]

@diyaayay I'm so glad you found the syntax reference helpful! Thanks for letting me know, and for working on the PR!

[GregStanton]

Hey @nickmcintyre, do you happen to know why vertexNormal() is still showing up in the p5.js reference? I figured it might take a few days, but it's been three weeks since we removed vertexNormal() from the inline docs... so I figured I'd check in to make sure we didn't miss something when completing this issue.

I hope you don't mind me pinging you; I thought you might know since you've been working on the reference.

[limzykenneth]

@GregStanton The reference is only updated when there is a release created for p5.js (since we don't want to release updated reference when the relevant p5.js version is not yet released), although this do create a bit of a lag in cases like this.

We plan to create a new release in a few weeks time which should have this updated. The new website will probably address this issue in due time.

[GregStanton]

Thanks @limzykenneth! That makes sense. The contributor docs suggest following up if the change isn't visible on the website after a few days, so I wasn't sure. I appreciate the clarification.