-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
add getTexture docs and examples for p5.strands #8446
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
add getTexture docs and examples for p5.strands #8446
Conversation
|
I've shared this draft PR to get early feedback before converting the examples to the flat API To-do:
Questions:
current types but these are all getting converted to any and void in type-gen. |
I think so! After merging the flat strands API I believe that should be used now in all the reference examples, if I caught all of them,
For a function like this that only works in strands, I think a boilerplate sentence or two would be great, and then we can paste that into other reference items that are strands-only.
I think I've also converted a bunch of those now that the flat strands PR has been merged too. but yeah, the idea will be to try to not use inline functions.
This is a good question. They're kind of both |
By default it'll use linear interpolation. Currently for a framebuffer you can do |
221fb04 to
2f80c03
Compare
link getTexture docs from buildFilterShader
2f80c03 to
082e85a
Compare
|
davepagurek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well written and the examples look good! Just one question about the parameter/return types when they don't have a real type just yet.
src/strands/p5.strands.js
Outdated
| * @method getTexture | ||
| * @beta | ||
| * | ||
| * @param {*} texture The texture to sample from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does * show up as in the parameter data after running npm run docs, and on when built into the website? I believe generally we can just omit the {} section when we don't have an associated type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I'll omit it, thanks!
I don't know why i latched on to the rare usage (from random(arr), which gets its type patched)
update: checking...
with types omitted:
getTexture(texture: any, coords: any): void;with {*} for params and return:
getTexture(texture: any, coords: any): any;so just that difference in return type.
both show no type info on the website.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that's good to know! We can use {*} for the return type then, and start to document when it makes sense to use that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found it now documented in the JSDocs for @param: https://jsdoc.app/tags-param#:~:text=Hello%20%27%20%2B%20somebody)%3B%0A%7D-,Allows%20any%20type,-/**%0A%20*%20%40param%20%7B (and elsewhere for object properties, though not for @return, I think just through oversight)
Weirdly, in the closure compiler docs around JSDoc it sounds like * should map to unknown in TS, and ? should may to any. I guess if it works and we're consistent...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good, we can add back in whatever works here to get an any return type for this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. If i rebase to squash the commits into one, do we lose these review comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure tbh! But also generally we don't feel the need to squash commits, it's OK if the incremental updates are in there.
don't write {*}
(was omitted - void)
Resolves #8438
Changes:
PR Checklist
npm run lintpasses - No, but my added code passes.npx documentation lint src/strands/p5.strands.jspasses: only one warning new from my code: unknown@betatag.