search

source-academy / modules

Modules that can be imported by programs in Source Academy, an online experiential environment for computational thinking
Apache License 2.0
8 stars 28 forks source link

🐛 fix: incorrect rgb colors #281

Closed srj31 closed 6 months ago

srj31 commented 6 months ago

Plotly treats decimal numbers for rgb value weirdly, where if rgb values are (0.9, 0, 0) it uses 0.9 as a percentage to display bright red

Fixes #279

Type of change

Please delete options that are not relevant.

martin-henz commented 6 months ago

Well, the description is a bit strange. It's clear that the original implemention intended to scale the color values by a factor of 255. The original implementation assumed that ?? has higher precedence than . However, ?? has lower precedence than . So 

pt.color[1] ?? 0 * 255;

is parsed like

pt.color[1] ?? (0 * 255);

which was obviously wrong because the color value isn't scaled at all. The PR fixes this bug.

RichDom2185 commented 6 months ago

Well, the description is a bit strange.

Yeah, it is. But I agree, the operation ordering is definitely incorrect. I guess we can merge this first and reopen the issue if it still persists.

srj31 commented 6 months ago

I wanted to explain why the error was not caught before when tested with different inputs. For e.g. pt.color[0] ?? 0 * 255 would return 0.9 which plotly would convert it to the 0...255 range value and it looked as if it worked correctly.

martin-henz commented 6 months ago

Ah, makes sense. Thanks!

martin-henz commented 6 months ago

I wanted to explain why the error was not caught before when tested with different inputs. For e.g. pt.color[0] ?? 0 * 255 would return 0.9 which plotly would convert it to the 0...255 range value and it looked as if it worked correctly.

It's fixed.