phetsims / energy-skate-park

"Energy Skate Park" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
2 stars 11 forks source link

Add support for swapping out skater images #337

Closed jessegreenberg closed 2 years ago

jessegreenberg commented 2 years ago

We are adding the ability to swap out a set of skater images for localization. The UI for this is going to be worked on in https://github.com/phetsims/joist/issues/814. But swapping out the images will be handled in the sim.

jessegreenberg commented 2 years ago

This is in place, we should be able to add or replace sets of skater images now as needed. To exercise the feature I added a Carousel to the Preferences dialog to select a set. Im sure that will change in https://github.com/phetsims/joist/issues/814.

One behavior question was how this feature should behave with reset. I opted to set skater back to the first of the selected set and not reset the character set. @arouinfar let me know if that is not expected.

@arouinfar I think I have done all I can to get support ready for this. Feel free to comment next steps here or close if we will continue in https://github.com/phetsims/joist/issues/814.

arouinfar commented 2 years ago

This is working really nicely @jessegreenberg. I noticed that the headshots could use a bit of top padding, but the artwork isn't finalized, so I can update that when delivering the final assets.

One behavior question was how this feature should behave with reset. I opted to set skater back to the first of the selected set and not reset the character set. @arouinfar let me know if that is not expected.

You made the right call. I don't think that the ResetAllButton should undo changes made via the Preferences dialog. There's already precedence for this, such as enabling Voicing or using non-default circuit symbols in CCK (currently done in the Options menu, but conceptually similar).

@arouinfar I think I have done all I can to get support ready for this. Feel free to comment next steps here or close if we will continue in https://github.com/phetsims/joist/issues/814.

Aside from https://github.com/phetsims/joist/issues/814, I think the only thing remaining is to expand the skater selection to 8 skaters, but that can probably wait until we have the final assets.

Here's a mockup showing 8 skaters. The skater radio buttons have been reduced to 40x40 px and are aligned with the increment/decrement buttons:

image
jessegreenberg commented 2 years ago

Great thanks. In the above commit I added two more skaters per set (recycling characters from each set for now) and adjusted the size of the radio buttons.

arouinfar commented 2 years ago

Awesome, thanks @jessegreenberg. It looks great in master. Since the infrastructure is all there, I think we can close and continue elsewhere.