p5.js
p5.js copied to clipboard
Add list support for `lerpColor` like other color functions
Resolves #6953
Changes:
Adds list support to the lerpColor like other color functions.
(I understand why this function doesn't support string based colors since you should know the actual value of the color when lerping between them, but adding list support is still definitely something that would be useful as seen in the referenced issue. However, if anyone thinks that adding string based color support to lerpColor as well would be preferred I'll add that to the PR.)
Screenshots of the change:
const red = [255, 0, 0];
const blue = [0, 255, 0];
function setup() {
// These always worked
fill(red);
fill(blue);
// This works now!
lerpColor(red, blue, 0.5);
}
PR Checklist
- [x]
npm run lintpasses - [x] Inline documentation is included / updated
- [x] Unit tests are included / updated
Thanks, looks good!
I understand why this function doesn't support string based colors
Do you mean this as in, we don't want to support this, or the color() function doesn't handle it? Just looking at your code, it seems like the code may actually already support string based colors. I haven't tested this, but if it does work, I'm ok adding |String to the types (and maybe mentioning in the docs that any argument to color() also works as an input) if you agree.
Thanks, looks good!
I understand why this function doesn't support string based colors
Do you mean this as in, we don't want to support this, or the
color()function doesn't handle it? Just looking at your code, it seems like the code may actually already support string based colors. I haven't tested this, but if it does work, I'm ok adding|Stringto the types (and maybe mentioning in the docs that any argument tocolor()also works as an input) if you agree.
I mean this as in I understand why you wouldn't support it, but I personally would prefer if it were supported. I'd be fine with adding the String type as well and yes, the function already does support String types.
although I can see that heavy lifting is being done by the Color constructor underneath, will it still be valuable to add examples and unit tests for this update in behavior?
although I can see that heavy lifting is being done by the Color constructor underneath, will it still be valuable to add examples and unit tests for this update in behavior?
Probably not. It's pretty expected behavior.
Agreed that it's probably OK to leave it as is. Maybe we can just add |String to the types and then merge?
Agreed that it's probably OK to leave it as is. Maybe we can just add
|Stringto the types and then merge?
Should we add lists as well and all other types that can be converted into p5.Color? Because for background() for instance, it says "p5.Color: any value created by the color() function" and doesn't list out all the types that can be converted into p5.Color, and including all those types could actually lead to more confusion. I think we should convert it to be the same as the rest of the documentation that involves colors like this and copy it from background().
I've added that instead for now to correspond better to the rest of p5.js's documentation instead of include all types (that would create inconsistencies although it could be nicer for some stuff).
That works, thanks!