Refactored the createGraphics function in rendering.js #6557
Conversation
|
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
| */ | ||
| if (args[0] instanceof HTMLCanvasElement) { | ||
| args[0] = constants.P2D; | ||
| args[1] = args[0]; |
There was a problem hiding this comment.
I believe this is a bug: args[1] will also be constants.P2D because you are setting it equal to args[0] after resetting that to be constants.P2D.
There was a problem hiding this comment.
Absolutely! I was thinking the same. Maybe using a temporary variable could do the trick? What do you reckon?
There was a problem hiding this comment.
That would work, or alternatively just setting args[1] before args[0].
Made suggested changes
|
I have made the changes as you suggested @davepagurek |
src/core/rendering.js
Outdated
| } | ||
| p5._validateParameters('createGraphics', arguments); | ||
| return new p5.Graphics(w, h, renderer, this, canvas); | ||
| p5._validateParameters('createGraphics', [w, h, ...args]); |
There was a problem hiding this comment.
One last thing, I think we might want to leave this one as arguments. I think the parameter validation method is intended to make sure what the user passes in matches the signatures in the docs. If something doesn't match, we want to give an error that corresponds to what the user has actually typed, not how we rearrange things, to avoid confusion.
|
I've changed it back to |
|
@all-contributors please add @Jaivignesh-afk for code |
|
I've put up a pull request to add @Jaivignesh-afk! 🎉 |
|
I appreciate the feedback. It was a pleasure working on this contribution. |
Resolves #6551
Changes:
Make some refactoring in createGraphics Function of rendering.js
Screenshots of the change:
PR Checklist
npm run lintpasses