Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

non-standard energy colors #343

Closed
pixelzoom opened this issue Jan 9, 2019 · 3 comments
Closed

non-standard energy colors #343

pixelzoom opened this issue Jan 9, 2019 · 3 comments
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 9, 2019

Noted while investigating phetsims/scenery-phet#455.

In MAS/EnergyGraphNode:

  var GREEN_COLOR = '#39d74e';
  var BLUE_COLOR = '#5798de';
  var CYAN_COLOR = '#29d4ff';
  var ORANGE_COLOR = '#ee6f3e';

First of all, these constants have totally inappropriate names. They are apparently colors for KE, PE (gravity), PE (elastic), and Thermal Energy respectively. These names should be changed.

Total Energy color looks like it's not specified, defaulting to 'black'. That should be changed.

Second, they should presumable be to to the standard energy colors from PhetColorScheme:

var RED_COLORBLIND = new Color( 255, 85, 0 );
var TAN_ORANGE = new Color( 236, 153, 55 );

HEAT_THERMAL_ENERGY: RED_COLORBLIND,
...
KINETIC_ENERGY: Color.GREEN,  // 0,255,0
...
POTENTIAL_ENERGY: Color.BLUE, // 0,0,255
...
TOTAL_ENERGY: TAN_ORANGE,

@arouinfar is there any reason why this sim shouldn't be using PhetColorScheme?

@arouinfar
Copy link

Thanks @pixelzoom. All PhET sims should use the same colors for the various energies, but PhetColorScheme is a bit antiquated. Changes to PhetColorScheme will be discussed in phetsims/scenery-phet#456, so marking this as status:on-hold until that issue is resolved.

@arouinfar
Copy link

arouinfar commented Jan 10, 2019

@Denz1994, in phetsims/scenery-phet#456 the colors used for energy were updated in PhetColorScheme.js. MAS should be using PhetColorScheme for gravitational PE, elastic PE, KE, and thermal energy.

The total energy is represented by a stacked bar comprised of the other energies, rather than a solid bar such as the one in ESPB. I do not believe the total energy needs to use PhetColorScheme, and can remain black in the graph label.

@Denz1994
Copy link
Contributor

PhetColorScheme is now being used for Colors for the Energy Graph view. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants