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

[p5.js 2.0 Beta Bug Report]: Confusing log when loading font with callback #7541

Open
2 of 17 tasks
aferriss opened this issue Feb 12, 2025 · 8 comments
Open
2 of 17 tasks

Comments

@aferriss
Copy link
Contributor

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
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

2.0.0 beta 2

Web browser and version

Chrome 132.0.6834.111

Operating system

MacOSX Sequoia 15.2

Steps to reproduce this

When loading a font with a callback in 2.0, a message is generated to the console that says:
🌸 p5.js says: Expected at most 1 argument, but received more in loadFont(). For more information, see https://p5js.org/reference/p5/loadFont..

Unless the docs are out of date, I thought callbacks were still supported, so we probably shouldn't be logging this message. If I'm wrong about the api changes, then maybe we should log a message about the callback api being no longer supported. I'm not super up to date on all the changes in 2.0 so feel free to close if this is a non-issue!

Steps:

  1. Call loadFont("myfont.ttf", font => {})
  2. Observe the warning in the console

Snippet:

https://editor.p5js.org/aferriss/sketches/dEb0fiahL

function setup() {
  createCanvas(400, 400);
  
  loadFont("Arial.ttf", font => {
    textFont(font);
    // Check console
  })
}

function draw() {
  background(220);
}
@ImRAJAS-SAMSE
Copy link
Contributor

Hi p5.js team,

I’ve been looking into this issue and found that loadFont() still technically supports callbacks, but the validation system incorrectly throws a warning when they are used. This seems to be the root cause of the confusion.

Proposed Fix:

Modify validate_params.js to allow up to 3 arguments for loadFont() (path, successCallback, and failureCallback), instead of triggering a warning.
Ensure that loadFont() continues supporting callbacks without issues.
No changes to the loadFont function itself—only adjusting validation to prevent misleading errors.

Would you like me to proceed with this fix? Or is there an intentional plan to deprecate callbacks in loadFont()?

Let me know your thoughts. Thank you :)

@aferriss
Copy link
Contributor Author

@Harshit-7373 I believe the issue is with the internals of the loadFont and not my example. Happy to be wrong, but I'm pretty sure 2.0 also won't support the preload() function so your solution may not work either.

@ImRAJAS-SAMSE thanks for looking into it! Sounds like you've identified a good potential solve. I can't speak to the support of callbacks in general or in loadFont for 2.0 so I'll leave that for @ksen0 to chime in. Personally I would hope we can leave them, as I think they're useful still.

@ImRAJAS-SAMSE
Copy link
Contributor

Thanks for your response, @aferriss! I agree—callbacks are still quite useful. I’ll wait for @ksen0’s input before proceeding. If callbacks are meant to be supported, I’ll work on fixing the validation issue so they don’t trigger an unnecessary warning.

@limzykenneth
Copy link
Member

It probably is the documentation not being up to date, the function signature should be the same and callback is still supported. From what I can see it might be that either documentation.js or our parsing script is not parsing the way that the parameters are defined in the inline docs. The inline docs will need to be updated to match actual behavior.

@ImRAJAS-SAMSE
Copy link
Contributor

Thanks for the clarification, @limzykenneth! Just to confirm, should I proceed with updating the inline docs in loading_displaying.js to correctly reflect callback support and check documentation.js to ensure proper parsing?

@limzykenneth
Copy link
Member

Thanks @ImRAJAS-SAMSE, not at the moment, we have some contributors working on the documentation already and they'll be going through all the references including this case. Feel free to look around the p5.js 2.0 beta and report any bugs you found.

@ImRAJAS-SAMSE
Copy link
Contributor

Got it, @limzykenneth! Thanks for the update. I'll keep exploring p5.js 2.0 beta.

@Jatin24062005
Copy link

Hi @aferriss , @dhowe , @limzykenneth , @ImRAJAS-SAMSE , @osteele ,

I've made changes to the p5.js main repository and want to test whether the issue is resolved using the p5.js Web Editor with the same code snippet mentioned above.

I cloned and ran the Web Editor locally, but I'm unable to use my modified p5.js library within it. Could anyone guide me on how to achieve this? Any help would be greatly appreciated!

Thanks in advance!

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

No branches or pull requests

5 participants