Pen Colors repeat each 101 numbers rather than each 100 numbers

[helderman]

Expected Behavior

Pen colors 0, 100, 200, 300 ... should all be red.

Actual Behavior

Instead, pen colors 0, 101, 202, 303 ... are red.
Full explanation (by Scratch-Minion) can be found in the Scratch forum:
https://scratch.mit.edu/discuss/topic/369220/

Steps to Reproduce

Draw a line with a pen color of 5000. Notice it is not red.
Demonstrated in this project (by Scratch-Minion):
https://scratch.mit.edu/projects/327014325

Operating System and Browser

Windows NT 10.0, Chrome 76.0.3809.132

[benjiwheeler]

I checked out Scratch-Minion's project ( https://scratch.mit.edu/projects/327014325 ) and verified this. If you continuously increment pen color by 101, it's always red. If you use 100, the color cycles through the rainbow.

[benjiwheeler]

The current 3.0 behavior also appears to be inconsistent with Scratch 2.0.

Note that from 2.0 to 3.0, we changed the pen color range size from 200 to 100; still, you can see that continually incrementing by 200 used to keep the color red:

So in 2.0, the period size for pen colors to cycle back to equivalent values was 200 -- not 201.

But currently in 3.0, the period size is 101.

Seems like we should change 3.0 to use a period size of 100.

[benjiwheeler]

Looking into MathUtil.wrapClamp, it seems to me like we might want to reconsider how it works.

Its current code is:

    static wrapClamp (n, min, max) {
        const range = (max - min) + 1;
        return n - (Math.floor((n - min) / range) * range);
    }

I think that's more understandable by rewriting it like this: (note I'm not proposing this change, just understanding it)

    static wrapClamp (n, min, max) {
        const range = (max - min) + 1;
        const timesOverRange = Math.floor((n - min) / range);
        const adjustedN = n - timesOverRange * range
        return adjustedN;
    }

I think this is built around the assumption that if you kept incrementing n by 1, you'd want max and min to be distinct stopping points.

E.g.:

If you're using MathUtil.wrapClamp(n, 0, 10), and repeatedly doing n = n + 1, you'd get values of 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 0, 1, 2 etc.

If you're using MathUtil.wrapClamp(n, 1, 2), and repeatedly doing n = n + 1, you'd get values of 1, 2, 1, 2, 1, 2, etc.

And if you're using MathUtil.wrapClamp(n, 0, 4), and repeatedly doing n = n + .5, you'd get values of 0, .5, 1, 1.5, 2, 2.5, 3, 3.5, 4, 4.5, 0, .5, etc.

Whereas if we instead did this:

        const range = max - min;

...then:

If you're using MathUtil.wrapClamp(n, 0, 10), and repeatedly doing n = n + 1, you'd get values of 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2 etc.

If you're using MathUtil.wrapClamp(n, 1, 2), and repeatedly doing n = n + 1, you'd get values of 1, 1, 1, 1, 1 etc.

And if you're using MathUtil.wrapClamp(n, 0, 4), and repeatedly doing n = n + .5, you'd get values of 0, .5, 1, 1.5, 2, 2.5, 3, 3.5, 0, .5, 1, etc.

[helderman]

One more thing: I see a similar effect with the legacy block 'set pen color to (number)' - which should only exist in projects that were migrated from Scratch 2.0 to 3.0, not in new Scratch 3.0 projects. Colors cycle at a period of 202 instead of 200.

[thisandagain]

/cc @kchadha @cwillisf