leopard-js / leopard

Library for making Scratch-like projects with JavaScript.
https://leopardjs.com
MIT License
139 stars 26 forks source link

Best API for setting pen color? #22

Closed PullJosh closed 4 years ago

PullJosh commented 4 years ago

Right now pen color can be set to any valid CSS color string (like red, #f00, rgb(255, 0, 0), or others).

This isn't great because it doesn't leave much wiggle room for setting color, saturation, brightness, or transparency independently (which is possible in Scratch).

What's the best API for dealing with colors? Color strings are handy, but underpowered.

towerofnix commented 4 years ago

I'd do something like these functions...

this.setPenHue(n)
this.setPenSaturation(n)
this.setPenBrightness(n)
this.setPenTransparency(n)

You could do stuff like this.setPenAttribute('hue', n) but that's difficult to remember and IMO it's better to avoid special "keyword strings" like that.

(Oh, or you could just do this.penHue = n, of course. I forgot that's like, something JS can do. :P)

When actually drawing, just compute the drawing color the same way Scratch does; you might want to expose that as a utility function too. (You can do this in reaction to the pen values updating too, so you don't recompute it every time. Or cache the values at render time, and if they haven't changed since last render, don't recompute. Then again that's probably a very small optimization anyway, but hey! :P)

Another idea is to make the pen its own object (i.e. class), and let a sprite easily switch between pens - this doesn't correspond to any existing Scratch feature, but it might be useful for learning developers to use, as a way to quickly swap between configurations defined earlier in the class or elsewhere in the project.

One more question is if the ranges should match the Scratch values or if they should match CSS; it would probably be easier to do Scratch values because 1) it's more familiar to developers, 2) you don't have to embed math right into the project code (ala (255/100 * <n>)), and 3) the code to convert it into what's rendered would be simpler and more understandable. But CSS values would help users get familiar with more typical color schemes for the web. I don't think there's an especially wrong answer here, but it'd probably help to take a look at the Scratch rendering code and consider that in judging :P

PullJosh commented 4 years ago

Another idea is to make the pen its own object (i.e. class)

Interesting. Something like this?

this.pen = new Pen(true, 4, "#f00"); // Down, size, color
this.pen.size = 8;
this.pen.setBlue(255);
this.pen.changeHue(25);
console.log(this.pen);
/*
Pen {
  down: true,
  size: 8,
  color: { r: 255, g: 0, b: 149, h: 325, s: 100, v: 100 }
}
*/

I like that this sort of "namespaces" off the color methods so that the only property name being used on the sprite itself is this.pen. It does require importing another class from scratch-js, but only if you want to set everything at once like in the first line of the example. If you're content to just update existing values (we'll start with some default), you don't need to import the class at all!


Another option is to take heavy advantage of JS magic. We can do something like this and make it work:

this.penColor = "red";
this.penColor.b = 255;
this.penColor.h += 25;
console.log(this.penColor); // { r: 255, b: 0, b: 149, h: 325, s: 100, v: 100 }

I don't like this kind of magic because nobody knows what's going on.

PullJosh commented 4 years ago

I guess it never really occurred to me, but we technically don't need to support rgb at all! It could be as simple as

this.penColor = { h: 0, s: 100, v: 100, a: 100 };
this.penColor.h = 120;
this.penColor.s -= 25;
this.penColor.a /= 2;

HSV is the same as HSB (I think), and it's what Scratch uses. If we forfeit any sort of syncing between different systems, things become dead simple. That's a very real option.


Of course, there's also value in performing the syncing and allowing users to deal in whichever color system they prefer (or mixing and matching throughout their code!)

If we find a sync system for dealing with both hsv and rgb, the following question becomes less of a problem:

One more question is if the ranges should match the Scratch values or if they should match CSS

Instead of just two systems (rgb and hsv), add a third (scratch hsv)!

// These are the same:
this.penColor.setColor(100);
this.penColor.setHue(255);

(Also, I don't think CSS has hsv. Just hsl. 😉)

bates64 commented 4 years ago

Add a Color type? This is somewhat similar to a special Pen class, but unspecialised.

this.penColor = Color.rgb(1.0, 0.0, 0.0)
console.log(this.penColor == Color.RED) // true
console.log(this.penColor == Color.rgba(1.0, 0.0, 0.0, 1.0)) // true
console.log(this.penColor == Color.hsv(0.0, 1.0, 0.5)) // true (I think)

this.penColor.b += 0.5

I personally like RGB(A) much more than HSV(A), so providing the option would be nice :)

PullJosh commented 4 years ago

@nanaian Is it possible to make equality work that way? Surely they would be different instances. :P In any case, I get the idea and I like it the best of any of the options I've seen.

towerofnix commented 4 years ago

@PullJosh == works for comparing them if you implement the toString function to return a color string (e.g. hex) — it'd be comparing those instead of the actual objects (which is what === usually does).

PullJosh commented 4 years ago

Okay, I think I've got a pretty decent system in the color branch. I'm going to make the needed updates to sb-edit (so that the automatic converter on the website continues to work) and then, unless anybody has concerns, I'll merge into master.

PullJosh commented 4 years ago

Fixed in #30 🎉