Skip to content
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

Update examples that use createCamera() to explicitly call setCamera() #7607

Open
wants to merge 2 commits into
base: dev-2.0
Choose a base branch
from

Conversation

webermayank
Copy link

@webermayank webermayank commented Mar 7, 2025

#7602 fix
updated the documentation to call setCamera() in case it hadn't been called, in the examples which uses createCamera().

Copy link

welcome bot commented Mar 7, 2025

🎉 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!

@webermayank
Copy link
Author

@perminder-17

did my PR didnt solve the issue?
Please address me that changes require, i'll do that

The extra lines that were useless at the bottom, i tried to revert those changes but it keeps coming when i staged the files, may its the formatter of my ide, tell me if it will make any issue , i'll remove those unnecessay changes.

@webermayank
Copy link
Author

I correct the unwanted changes, sorry for inconvenience

@GregStanton
Copy link
Collaborator

GregStanton commented Mar 21, 2025

Hi @webermayank! Thanks for working on this. I'm adding it to a list of documentation changes that need to be reviewed. We should be able to review and merge this by the end of the month. In the meantime, in case you'd like to continue helping out, I'll outline some additional steps you can take.

  1. Enter the following search phrase into Google: site:p5js.org/reference -site:*.p5js.org/ "createCamera"
  2. In the description of this PR (at the top), add a list of links to each page in the search results (there should be around 40 results). If you see a message at the end saying similar results have been omitted, click to show the omitted results too.
  3. Tag each link with one of the following: [need to check page], [no change needed], [change needed], [change made], [change reviewed]

Each link can start out with "[need to check page]." Once all the pages are tagged with either "[no change needed]" or "[change reviewed]", we should be able to merge this PR. If you want to do this work (except for the reviewing), just let us know, and at least one of us (@davepagurek, @perminder-17, @holomorfo, or myself) will do a review, so we can merge this. If you don't have time to work on this by the end of next week, we'll help out.

Also, @davepagurek:

I could look into this more, but since you're already familiar with the changes, I thought I'd ask you a couple things first.

  1. Does createCamera on p5.Framebuffer have the new behavior? I guess it probably should, for consistency. If it doesn't, we may need to update the code and then update the documentation. For example, I suppose we might need to update both https://p5js.org/reference/p5/createCamera/ and https://p5js.org/reference/p5.Framebuffer/createCamera/.
  2. Might we need to update the docs for setCamera() too? I'm thinking that in any examples where setCamera() is already called, we should be fine, but maybe the description of setCamera() itself should be updated, since it's no longer just for switching between multiple cameras.

@webermayank
Copy link
Author

@GregStanton, Okay i'll do it, i'll mention all the links related to the reference which include createCamera and create a list with checks.
Tell me if anything else needs to be updated, i would love to address those.

@GregStanton
Copy link
Collaborator

Thanks so much @webermayank!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants