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

Documentation Update:Webgl - .lib files not supported #7454

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

atmajaa
Copy link

@atmajaa atmajaa commented Jan 3, 2025

Resolves #7381

Changes:

  • Added documentation note on .lib files not being supported for WebGL.

Screenshots of the change:

  • (Attach screenshots if applicable)

PR Checklist

  • npm run lint passes
  • [Inline documentation] is included / updated
  • [Unit tests] are included / updated

Copy link

welcome bot commented Jan 3, 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!

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 making these changes! I just left a few comments about how we might make the text a bit more easily understandable to users.

@@ -26,6 +26,8 @@ import './p5.Geometry';
* `loadModel('assets/model.obj')`. URLs such as
* `'https://example.com/model.obj'` may be blocked due to browser security.
*
* Note: Currently there is no support for `.lib` files. Only `.mtl` files are supported for defining materials.
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than just mentioning formats we don't support that users may not have heard of, maybe we can phrase this like, "if you are loading a .obj file that references materials stored in .mtl files, p5.js will try to load those materials." and maybe mention something about making sure you upload them alongside your obj if you want them to be read.

Copy link
Author

Choose a reason for hiding this comment

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

@davepagurek Thankyou for the feedback!
I have made the required changes. Let me know if any further modifications are required.

@@ -143,5 +143,8 @@
"hooks": {
"pre-commit": "lint-staged"
}
},
"dependencies": {
"p5": "file:"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need these changes just to change the documentation. You can reset them with git checkout main -- package-lock.json package.json and then commit the results.

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.

expand support for .lib materials in obj
2 participants