phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

Provide a function to create the schema for `gameLevels` query parameter. #100

Closed pixelzoom closed 2 years ago

pixelzoom commented 2 years ago

Designers (@arouinfar @amanda-phet @kathy-phet) requested a gameLevels query parameter. Unfortunately that query parameter can't be supported by the common-code schema in initialize-globals.js, because every sim has a different number of game levels. The downside of this is that every sim is creating its own implementation of gameLevels, and the implementations are not the same. For example fourier-making-waves and number-play have different implementation that can behave differently, see https://github.com/phetsims/number-play/issues/92.

While we can't support gameLevels in initialize-globals.js, perhaps we can provide common code (parameterized with number of levels) to create the schema for gameLevels, so that all sims share the same schema. I'll investigate.

pixelzoom commented 2 years ago

I factored out vegas.getGameLevelsSchema, and it's now used in EqualityExplorerQueryParamaters.js and FourierMakingWaves.js.

@markgammon could you please assign someone to review?

markgammon commented 2 years ago

@pixelzoom do you have recommendations for devs that may be particularly good match for this review? Thanks

pixelzoom commented 2 years ago

@chrisklus perhaps, since this work was motivated by phetsims/number-play#92? By now he's probably familar with the standard design of ?gameLevels. And regardless of how Number Play ends up addressing this query parameter, it would be good for him to be familiar with it.

markgammon commented 2 years ago

Thanks @pixelzoom. @chrisklus will do the review.

chrisklus commented 2 years ago

getGameLevelsSchema and its usages look great. I tested it out in a new sim and made sure validation worked for all expected cases in FMW. Back to @pixelzoom to close if all set.

I'll comment details in phetsims/number-play#92, but it would be pretty unifying for Number Play to use this as well.

pixelzoom commented 2 years ago

Thanks @chrisklus. Closing.