Throw error when passed incorrect color mode instead of silently accepting it?

[gregsadetsky]

trafficstars

How would this new feature help increase access to p5.js?

p5.js could show an error message when colorMode() is passed an invalid color mode, instead of silently ignoring it.

Most appropriate sub-area of p5.js?

• [X] Friendly error system

New feature details:

colorMode accepts any string as its mode

I mistakenly passed "HSL" to it, e.g. a string, instead of the correct constant. I did not realize that colorMode() silently ignored my request to change the color mode, and stayed in RGB mode. It feels to me like this could be quite confusing to others as well.

I would suggest doing the following change: if the passed argument is not any of the known modes, throw an error, i.e.:

p5.prototype.colorMode = function(mode, max1, max2, max3, maxA) {
  p5._validateParameters('colorMode', arguments);
  if (
    mode === constants.RGB ||
    mode === constants.HSB ||
    mode === constants.HSL
  ) {
  // ...
  // ...
  // *** New code below ***
  } else {
    throw new Error('invalid color mode')
  }

I'd be happy to do a commit/PR (with a test) for this. Thanks!

[willmartian]

It looks like an error is thrown when instantiating a new color, perhaps we could use the same message? https://github.com/processing/p5.js/blob/e46fa4d5ad9ba4cad771ac7b7bc01d1f099f3fa3/src/color/p5.Color.js#L41

Additionally, should we just convert the mode argument to lowercase before comparing? I feel like this might be an easy mistake to make.

[gregsadetsky]

I think that this error message is great, and your suggestion to convert to lowercase is really good as well! That would definitely have caught/saved me originally when passing "HSL".

[stampyzfanz]

I believe this suggestion applies to Constants and Friendly Errors in general - maybe this comment should be in a new issue or this one should be renamed if my suggestion should be further discussed.

It would good to rework the constants system. Instead of checking equality with constants and a user inputted mode (ie mode === RGB), there should a method that takes in the mode and the constants it could be and would return whether there is a match. If none did, it would create a Friendly Error and predict typos. It would also convert the inputted mode to lowercase.

This method is superior to what currently exists because of the internationalization built into FES that is already implemented. Additionally, the typo checking is helpful and already exists within FES. Also, if we decided that converting all user inputted modes to lowercase is a good idea (I think it is) using a single method would reduce the amount of duplicated code of converting to lowercase.

Just looking through the code, there is at least 1 exception to the general pattern I outlined that the above method signature would solve: notably https://github.com/processing/p5.js/blob/4640a269c35d90d2b0e1bf4d712149455e5f4766/src/webgl/p5.RendererGL.js#L708-L718 which has a custom error message for common incorrect constant use. This would probably have to be kept the same other than delegating the throwing of the error message to FES.

[vaishnavi192]

Hey @gregsadetsky @stampyzfanz I would like to work on this issue. Can you tell what exactly we have to do?