p5.Vector reflect() unexpectedly modifies surface normal argument

[nbogie]

Most appropriate sub-area of p5.js?

• [ ] Accessibility
• [ ] Color
• [ ] Core/Environment/Rendering
• [ ] Data
• [ ] DOM
• [ ] Events
• [ ] Image
• [ ] IO
• [X] Math
• [ ] Typography
• [ ] Utilities
• [ ] WebGL
• [ ] Build process
• [ ] Unit testing
• [ ] Internationalization
• [ ] Friendly errors
• [ ] Other (specify if possible)

p5.js version

1.9.4

Web browser and version

all

Operating system

all

Steps to reproduce this

Snippet:

function setup() {
    instanceDemo();
    staticDemo();
}

function instanceDemo() {
    const surfaceNormal = createVector(1, 0);
    const incidentVector = createVector(1, 1);

    incidentVector.reflect(surfaceNormal);

    //We'd hope surfaceNormal remains [1, 0, 0], but it'll be [2, 0, 0];
    console.log(surfaceNormal.toString());
}

function staticDemo() {
    const surfaceNormal = createVector(1, 0);
    const incidentVector = createVector(1, 1);

    p5.Vector.reflect(incidentVector, surfaceNormal);

    //We'd hope surfaceNormal remains [1, 0, 0], but it'll be [2, 0, 0];
    console.log(surfaceNormal.toString());
}

Expected behaviour:

I'd expect the surface normal argument to be unchanged by the calculation, in both instance and static cases.

Actual behaviour:

The surface normal argument is mutated during the calculation, in both instance and static cases.

Misc:

Here's a (simplification of a) real usage where the bug caught me out: https://openprocessing.org/sketch/2295422

[welcome[bot]]

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!

[nbogie]

I'd like to contribute a fix and regression tests, if it turns out that that's wanted!

Looks like the code has been this way since addition of the functions in v1.0.0

[limzykenneth]

It seems like on this line the surface normal is multiplied by two using the instance method which modifies the vector where it probably should use a static method that does not modify the original instead.

@nbogie You can go ahead with a fix. Thanks!