Skip to content

fix: add splineVertex to p5.Graphics prototype and fix unit test error #7757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 21, 2025

Conversation

VANSH3104
Copy link
Contributor

Resolves #7755

This pull request introduces a fix to the p5.Graphics prototype by adding the splineVertex method, which allows for more flexible spline drawing in the graphics context. Additionally, the corresponding unit test has been updated to reflect this change, addressing a previously encountered error in the test suite.

Changes:

  • Added splineVertex method to p5.Graphics prototype.

  • Fixed unit test error related to the splineVertex method.

  • testing using this

suite('p5.Graphics.prototype.splineVertex', function() {
    test('should be a function in graphics', function() {
      let g = myp5.createGraphics(10, 10);
      assert.ok(g.splineVertex);
      assert.typeOf(g.splineVertex, 'function');
    });
  });

PR Checklist

@bojidar-bg
Copy link

I wonder if it might be possible to call customShapes similar to how primitives2D and the other addon functions are called in p5.Graphics.js - that might save a lot of repetition 😅

@VANSH3104
Copy link
Contributor Author

VANSH3104 commented Apr 20, 2025

@bojidar-bg Thanks a lot for the suggestion — that makes perfect sense!
I’ve added customShapes in Graphics.js and registered it just like you suggested.
Could you please take a look now and let me know if anything else needs tweaking?

@bojidar-bg
Copy link

Well.. you mention adding a unit test in the PR description and the commit message, but there's no unit test added in the changes themselves? (:'

@VANSH3104
Copy link
Contributor Author

@bojidar-bg Ah, you're right — I mistakenly mentioned the unit test in the commit/PR but didn't include it in the actual changes 😅
I’ll add the unit test right away and push an update. Thanks for pointing that out!

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this!

@davepagurek davepagurek merged commit 398df4b into processing:dev-2.0 Apr 21, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants