-
Notifications
You must be signed in to change notification settings - Fork 3
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
Allow clients to set Wave Game challenges. #144
Comments
@samreid and I discussed via Zoom. There are 3 options that @samreid recommended, with increasing cost: (1) Write out standard API calls that would set each PhET-iO element's value to configure a challenge. Provide an exemplar in the one of the client guides. This is completely wrapper side. (2) If (1) is too complicated, write client-side code that has the API described above (array of amplitudes, array of boolean slider visibilities) and converts to standard API calls. This is completely wrapper side. (3) If (1) and (2) isn't satisfactory, add a method to an IOType ( For (2) and (3), @samreid suggested a more structured API, rather than 2 parallel arrays. Something like: @typedef ChallengeDescription {Array.<{amplitude:number, [visible:boolean]}>} |
To try options (1) above:
// Set the state to the simulation to customize it.
phetioClient.invoke( 'phetioEngine', 'setState', [ state ], function() {
/* console.log( 'Finished launching with customizations.' ); */
} );
// Set the state to the simulation to customize it.
phetioClient.invoke( 'phetioEngine', 'setState', [ state ], function() {
/* console.log( 'Finished launching with customizations.' ); */
phetioClient.invoke(
'fourierMakingWaves.waveGameScreen.model.level3.answerSeries.harmonics.harmonic1.amplitudeProperty',
'setValue',
[ 1.5 ]
);
} );
|
I got pretty far with options (1), then couldn't figure out where to check it in. So I've included the listener below. A couple of questions:
Code // Set the state to the simulation to customize it.
phetioClient.invoke( 'phetioEngine', 'setState', [ state ], function() {
/* console.log( 'Finished launching with customizations.' ); */
// constants, the sim requires these!
const NUMBER_OF_HARMONICS = 11;
const MAX_AMPLITUDE = 1.5;
// The game level that you're working with.
const level = 3;
// {Array:<{amplitude:number, [controlsVisible:boolean]}
// Data structure that describes the harmonics in a Wave Game challenge.
// The array is ordered by increasing harmonic order, i.e. the fundamental is harmonics[0].
// amplitude is a value in the range [-1.5, 1.5] with at most 1 decimal place.
// controlVisible determines whether the Slider and NumberDisplay for the harmonic are visible,
// defaults to true for non-zero amplitudes, and setting to false for non-zero harmonics is an error.
const harmonics = [
{ amplitude: 0.1 },
{ amplitude: 0, controlsVisible: true },
{ amplitude: 0.3 },
{ amplitude: 0, controlsVisible: true },
{ amplitude: 0.5 },
{ amplitude: 0 },
{ amplitude: 0.7 },
{ amplitude: 0 },
{ amplitude: 0.9 },
{ amplitude: 0 },
{ amplitude: 1.1 },
];
assert && assert( harmonics.length === NUMBER_OF_HARMONICS, `you must provide ${NUMBER_OF_HARMONICS} harmonics` );
// Adjust the range of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControlsRangePropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.numberOfAmplitudeControlsProperty.rangeProperty`;
const minNumberOfAmplitudeControls = _.filter( harmonics, object => object.amplitude !== 0 ).length;
const maxNumberOfAmplitudeControls = harmonics.length;
// phetioClient.invoke( numberOfAmplitudeControlsRangePropertyID, 'setValue', [ TODO Range? ] );
// Adjust the value of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControlsPropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.numberOfAmplitudeControlsProperty`;
const numberOfAmplitudeControls = _.filter( harmonics, object => ( object.amplitude !== 0 || object.controlsVisible === true ) ).length;
phetioClient.invoke( numberOfAmplitudeControlsPropertyID, 'setValue', [ numberOfAmplitudeControls ] );
// Set the amplitude values for the challenge answer.
for ( let i = 0; i < harmonics.length; i++ ) {
const order = i + 1;
const amplitude = harmonics[ i ].amplitude;
assert && assert( amplitude !== undefined, `harmonic ${order} is missing the amplitude field` );
assert && assert( amplitude >= -MAX_AMPLITUDE && amplitude <= MAX_AMPLITUDE, `harmonic ${order} amplitude is out of range: ${amplitude}` );
//TODO verify that amplitude has at most 1 decimal place
const amplitudePropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.answerSeries.harmonics.harmonic${order}.amplitudeProperty`;
phetioClient.invoke( amplitudePropertyID, 'setValue', [ amplitude ] );
}
// Set visibility of amplitude Sliders and NumberDisplays.
//TODO Have to do this after setting all amplitudes in the model, or some of these get made invisible.
for ( let i = 0; i < harmonics.length; i++ ) {
const order = i + 1;
const amplitude = harmonics[ i ].amplitude;
const controlsVisible = harmonics[ i ].controlsVisible;
assert && assert( !( amplitude !== 0 && controlsVisible === false ), `harmonic ${order} has a non-zero amplitude and controlsVisible: false` );
const visible = ( amplitude !== 0 ) || ( controlsVisible === true );
const numberDisplayVisiblePropertyID = `fourierMakingWaves.waveGameScreen.view.level${level}Node.charts.amplitudes.amplitudesChartNode.amplitude${order}NumberDisplay.visibleProperty`;
phetioClient.invoke( numberDisplayVisiblePropertyID, 'setValue', [ visible ] );
const sliderVisiblePropertyID = `fourierMakingWaves.waveGameScreen.view.level${level}Node.charts.amplitudes.amplitudesChartNode.amplitude${order}Slider.visibleProperty`;
phetioClient.invoke( sliderVisiblePropertyID, 'setValue', [ visible ] );
}
} );
} |
I checked my wrapper in as fourier-making-waves/index.html. I figured out that the wrapper will run from the top-level of fourier-making-waves:
But it must be named index.html or it won't run. For example, renaming to set-challenge-example.html does not work:
@samreid @zepumph Why can't I give this wrapper a description name? What do I need to do to be able to move it to someplace like |
I must've done something wrong the first time I tried this, maybe a bad value for
I'd still like to be able to relocate it to a subdirectory in fourier-making-waves, but I guess that's not essential. |
I tried this, thinking that
... but that fails with:
|
Oh, I see. I'm trying to set the rangeProperty of a NumberProperty. But by default NumberProperty, creates its rangeProperty as |
I got this to work:
...by fixing up my rangeProperty like this in WaveGameLevel.js: // @public the number of amplitude controls (sliders) to show in the Amplitudes chart
this.numberOfAmplitudeControlsProperty = new NumberProperty( config.defaultNumberOfAmplitudeControls, {
range: new Range( this.answerSeries.getNumberOfNonZeroHarmonics(), this.answerSeries.harmonics.length ),
rangePropertyOptions: {
phetioDocumentation: 'Determines the range of the Amplitude Controls spinner',
phetioType: Property.PropertyIO( Range.RangeIO ),
phetioReadOnly: false,
phetioStudioControl: false
},
tandem: config.tandem.createTandem( 'numberOfAmplitudeControlsProperty' )
} ); But what a pain to iterate on creating a wrapper. After making the code change, I had to run the sim in Studio, general new HTML, save my custom wrapper code to a temporary document, paste the new HTML into my wrapper, then paste my custom code back into the wrapper. Then realize I had a lint error and start all over again !?!? Passing in |
In our discussion, I neglected to describe our policy that PhET-iO wrapper code should not be checked in to public repos. My mistake, and sorry for the resultant hassle. From a technical standpoint, checking it in to the sim repo is the best option, but this policy was introduced for intellectual property and privacy reasons. I'm not sure where it should be, especially if it has a constraint that it must be a top-level file in a repo (for the paths to work correctly). Perhaps it should be in phet-io-wrappers for now? @zepumph can you please advise? |
I'm kind of surprised that I don't see other sims with test wrappers like this one, and that there's not a procedure in place at this late date.
I definite do not think this belongs at the top-level of phet-io-wrappers. That would deserve a littering violation. |
I see one example in http://localhost/main/phet-io-wrapper-hookes-law-energy/?sim=hookes-law |
Yikes - an entirely new private repo to test wrappers for my sims? |
I see another example in phet-io-wrappers/build-an-atom-game/index.html |
I'd like to understand when the "run it in studio and generate a new wrapper template" steps are necessary. Could those steps be skipped in some cases? |
Yes, I think that can be skipped. Looks like the wrapper template doesn't have any sim or common-code script embedded in it. |
@samreid and I paired on this via Zoom. We complete option (2) above, factoring out a function that handles making the Here's an example of the function call, as it would appear in the wrapper callback for sim customization: // Set the state to the simulation to customize it.
phetioClient.invoke( 'phetioEngine', 'setState', [ state ], function() {
initializeGame(
3,
[ 0.1, 0, 0.3, 0, 0.5, 0, 0.7, 0, 0.9, 0, 1.1 ],
[ true, true, true, true, true, false, true, false, true, false, true ]
);
/* console.log( 'Finished launching with customizations.' ); */
} );
} Here's the definition of the function, as it would appear somewhere in the wrapper, or in a /**
* Initializes the Wave Game with a specific challenge.
* @param {number} level - the game level
* @param {number[]} amplitudes - harmonic amplitudes, in harmonic order, in range [-1.5,1.5]
* @param {boolean[]} controlsVisible - visibility of amplitude controls, in harmonic order
*/
function initializeGame( level, amplitudes, controlsVisible ) {
// constants, the sim requires these!
const NUMBER_OF_GAME_LEVELS = 5;
const NUMBER_OF_HARMONICS = 11;
const MAX_AMPLITUDE = 1.5;
assert && assert( level >= 1 && level <= NUMBER_OF_GAME_LEVELS, `invalid level: ${level}` );
assert && assert( amplitudes.length === NUMBER_OF_HARMONICS, `you must provide ${NUMBER_OF_HARMONICS} amplitudes` );
assert && assert( controlsVisible.length === NUMBER_OF_HARMONICS, `you must provide ${NUMBER_OF_HARMONICS} controlsVisible` );
// Adjust the range of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControlsRangePropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.numberOfAmplitudeControlsProperty.rangeProperty`;
const numberOfAmplitudeControlsRange = {
min: _.filter( amplitudes, amplitude => ( amplitude !== 0 ) ).length,
max: amplitudes.length
};
phetioClient.invoke( numberOfAmplitudeControlsRangePropertyID, 'setValue', [ numberOfAmplitudeControlsRange ] );
// Adjust the value of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControlsPropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.numberOfAmplitudeControlsProperty`;
const numberOfAmplitudeControls = _.filter( controlsVisible, controlVisible => ( controlVisible === true ) ).length;
phetioClient.invoke( numberOfAmplitudeControlsPropertyID, 'setValue', [ numberOfAmplitudeControls ] );
// Set the amplitude values for the challenge answer.
for ( let i = 0; i < amplitudes.length; i++ ) {
const order = i + 1;
const amplitude = amplitudes[ i ];
assert && assert( amplitude >= -MAX_AMPLITUDE && amplitude <= MAX_AMPLITUDE, `amplitude is out of range: ${amplitude}` );
assert && assert( ( amplitude.toFixed( 0 ) === '' + amplitude ) || ( amplitude.toFixed( 1 ) === '' + amplitude ),
`amplitude has more than 1 decimal place: ${amplitude}` );
const amplitudePropertyID = `fourierMakingWaves.waveGameScreen.model.level${level}.answerSeries.harmonics.harmonic${order}.amplitudeProperty`;
phetioClient.invoke( amplitudePropertyID, 'setValue', [ amplitude ] );
}
// Set visibility of amplitude Sliders and NumberDisplays. Do this AFTER changing elements of the model.
// As the model changes, it sends out notifications that result in view changes. If you try to change the
// view before the model, those notifications are likely to undo your view changes.
for ( let i = 0; i < controlsVisible.length; i++ ) {
const order = i + 1;
const amplitude = amplitudes[ i ];
const controlVisible = controlsVisible[ i ];
assert && assert( !( amplitude !== 0 && controlsVisible === false ), `index ${i} has a non-zero amplitude and controlsVisible: false` );
const numberDisplayVisiblePropertyID = `fourierMakingWaves.waveGameScreen.view.level${level}Node.charts.amplitudes.amplitudesChartNode.amplitude${order}NumberDisplay.visibleProperty`;
phetioClient.invoke( numberDisplayVisiblePropertyID, 'setValue', [ controlVisible ] );
const sliderVisiblePropertyID = `fourierMakingWaves.waveGameScreen.view.level${level}Node.charts.amplitudes.amplitudesChartNode.amplitude${order}Slider.visibleProperty`;
phetioClient.invoke( sliderVisiblePropertyID, 'setValue', [ controlVisible ] );
}
} The Wave Game screen looks like this after the example above is run: |
@samreid said:
Because there is currently no standard (secure) place for putting sim-specific wrappers, I have deleted my test wrapper, game-setup-example.html, from fourier-making-waves. |
@samreid and I took a look at option (3), adding a custom method to an IOType. We didn't get this to work, just sketched out what it might look like. Since we need to modify elements of the model and view, we added an IOType to WaveGameScreen: WaveGameScreen.WaveGameScreenIO = new IOType( 'WaveGameScreenIO', {
valueType: WaveGameLevel,
supertype: ReferenceIO( IOType.ObjectIO ),
methods: {
initializeGame: {
returnType: VoidIO,
parameterTypes: [ NumberIO, ArrayIO( NumberIO ), ArrayIO( BooleanIO ) ],
implementation: function( level, harmonics ) {
this.initializeGame( level, harmonics );
}
}
}
} );
/**
* Initializes the Wave Game with a specific challenge.
* @param {number} level - the game level
* @param {number[]} amplitudes - harmonic amplitudes, in harmonic order, in range [-1.5,1.5]
* @param {boolean[]} controlsVisible - visibility of amplitude controls, in harmonic order
* @private
*/
initializeGame( level, amplitudes, controlsVisible ) {
assert && assert( amplitudes.length === FMWConstants.MAX_HARMONICS, `you must provide ${FMWConstants.MAX_HARMONICS} amplitudes` );
assert && assert( amplitudes.controlsVisible === FMWConstants.MAX_HARMONICS, `you must provide ${FMWConstants.MAX_HARMONICS} controlsVisible` );
for ( let i = 0; i < controlsVisible.length; i++ ) {
assert && assert( ( controlsVisible[ i ] === true ) || ( controlsVisible[ i ] === false && amplitudes[ i ] === 0 ),
'you cannot set controlsVisible for a non-zero amplitude' );
}
// Adjust the range of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControlsRange = new Range( _.filter( amplitudes, amplitude => ( amplitude !== 0 ) ).length, amplitudes.length );
this.model.setNumberOfAmplitudeControlsRange( level, numberOfAmplitudeControlsRange );
// Adjust the value of the 'Amplitude Controls' spinner.
const numberOfAmplitudeControls = _.filter( controlsVisible, controlVisible => ( controlVisible === true ) ).length;
this.model.setNumberOfAmplitudeControls( level, numberOfAmplitudeControls );
// Set model amplitudes
this.model.setAnswerAmplitudes( level, amplitudes );
// Set visibility of view controls
this.view.setAmplitudeControlsVisible( level, controlsVisible );
} The wrapper call would look something like this in the wrapper: // Set the state to the simulation to customize it.
phetioClient.invoke( 'phetioEngine', 'setState', [ state ], function() {
phetioCient.invoke( 'fourierMakingWaves.waveGameScreen', 'initializeGame', [
3,
[ 0.1, 0, 0.3, 0, 0.5, 0, 0.7, 0, 0.9, 0, 1.1 ],
[ true, true, true, true, true, false, true, false, true, false, true ]
] );
/* console.log( 'Finished launching with customizations.' ); */
} );
} The advantage of this approach is that the wrapper isn't littered with PHET-iO element IDs (tandems). If there are any changes to the API, the are compiled into the sim. |
In addition to the "how to make this work" aspect of this issue, @samreid and I discussed some higher-level issues that have not yet been addressed by PhET-iO. I've summarize those issues in https://github.com/phetsims/phet-io/issues/1814. |
The Fourier team has decided not to include PhET-iO in the 1.0 release. Deferring this issue. |
At 10/14/21 quarterly planning meeting, @kathy-phet asked @JacquiHayes to ask clients what kind of functionality (if any) they would like related to this feature. Assigning to @JacquiHayes. |
@JacquiHayes FYI. At Q1 2022 planning meeting, asking the client about their needs for the game was established as a goal for this quarter. |
Adding @brent-phet to some of the more "product-managery" issues of PhET-iO. Here we are blocked by not knowing what clients want out of our phetsim "games" (pardon the fourier repo-specific issues). This effects many sims, potentially most pertinently Reactants, Products, and Leftovers. |
For what it is worth, of course I did and do ask them about the games. But
they either don’t reply to my emails or they say that they also don’t know
what they want.
This is especially true for a simulation like Fourier Waves, which the
partners were not ready to work with.
For the math sims, however, simple flags on the games are often asked
about.
…On Thu, Mar 30, 2023 at 7:47 PM, Michael Kauzmann ***@***.***> wrote:
Adding @brent-phet <https://github.com/brent-phet> to some of the more
"product-managery" issues of PhET-iO. Here we are blocked by not knowing
what clients want out of our phetsim "games" (pardon the fourier
repo-specific issues). This effects many sims, potentially most pertinently
Reactants, Products, and Leftovers.
—
Reply to this email directly, view it on GitHub
<#144 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOLUG52QNCNXEGML3XSPSO3W6XBLFANCNFSM5CITFDQA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
That is really helpful, thanks. I wasn't trying to insinuate that you hadn't done the work, in our meeting today we were mostly just trying to describe the "chicken and egg" problem with developing valuable features like this in a client-driven way. |
General design issues that need to be discussed:
|
From #4 (comment):
@kathy-phet @arouinfar and I discussed this.
Allow the client to set
amplitudeProperty
for the harmonics in the "answer" FourierSeries. This would require the sim to adapt as amplitude values are changed. It currently does not do this.Provide an API function that allows a developer to set the amplitudes for the "answer" FourierSeries. Provide and array of 11 values in the range [-1.5,1.5], and an optional array of booleans to show which sliders are visible (validated). I'll need to discuss how this might be done with the PhET-iO team. And set
phetioStudioControl: false
foramplitudeProperty
.The text was updated successfully, but these errors were encountered: