search

phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

Allow clients to set Wave Game challenges. #144

Open pixelzoom opened 3 years ago

pixelzoom commented 3 years ago

From https://github.com/phetsims/fourier-making-waves/issues/4#issuecomment-846256591:

Specify their own challenge, or at least have some way of ensuring all students get the same sequence of challenges:

@kathy-phet @arouinfar and I discussed this.

pixelzoom commented 3 years ago

I'll need to discuss how this might be done with the PhET-iO team.

@samreid @zepumph when are you available to discuss? High priority because this sim needs to be in dev testing in < 2 weeks.

pixelzoom commented 3 years ago

@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 (WaveGameLevelIO?) This is not wrapper side, requires PhET-side programming, and a new release of the sim to add later.

For (2) and (3), @samreid suggested a more structured API, rather than 2 parallel arrays. Something like:

@typedef ChallengeDescription {Array.<{amplitude:number, [visible:boolean]}>}
pixelzoom commented 3 years ago

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 ]
          );
        } );
pixelzoom commented 3 years ago

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 ```js // 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 ] ); } } ); } ```
pixelzoom commented 3 years ago

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:

http://localhost/~cmalley/GitHub/fourier-making-waves/?sim=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:

http://localhost/~cmalley/GitHub/fourier-making-waves/set-challenge-example.html?sim=fourier-making-waves

@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 fourier-making-waves/phet-io-wrappers/ ?

pixelzoom commented 3 years ago

Why can't I give this wrapper a description name?

I must've done something wrong the first time I tried this, maybe a bad value for ?sim. I tried again, renamed the wrapper to game-setup-example.html, and this works:

http://localhost/~cmalley/GitHub/fourier-making-waves/game-setup-example.html?sim=fourier-making-waves

I'd still like to be able to relocate it to a subdirectory in fourier-making-waves, but I guess that's not essential.

pixelzoom commented 3 years ago
  • How do I setValue for a {Property.}?

I tried this, thinking that setValue probably took a state object, and trying to fake a state object:

phetioClient.invoke( elementID, 'setValue', [ { min: 0, max: 100 } ] );

... but that fails with:

Cannot invoke method 'setValue' on read-only element

pixelzoom commented 3 years ago

Oh, I see. I'm trying to set the rangeProperty of a NumberProperty. But by default NumberProperty, creates its rangeProperty as phetioReadOnly: false. Guess I need to customize that.

pixelzoom commented 3 years ago

I got this to work:

phetioClient.invoke( elementID, 'setValue', [ { min: 0, max: 100 } ] );

...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 [ { min: 0, max: 100 } ] to setValue also feels unsafe and unwise. Much more robust would be to create a Range instance, then use RangeIO to convert it to a stateObject. How do I do that?

samreid commented 3 years ago

I checked my wrapper in as fourier-making-waves/index.html.

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?

pixelzoom commented 3 years ago

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?

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.

Perhaps it should be in phet-io-wrappers for now?

I definite do not think this belongs at the top-level of phet-io-wrappers. That would deserve a littering violation.

samreid commented 3 years ago

I see one example in http://localhost/main/phet-io-wrapper-hookes-law-energy/?sim=hookes-law

pixelzoom commented 3 years ago

Yikes - an entirely new private repo to test wrappers for my sims?

samreid commented 3 years ago

I see another example in phet-io-wrappers/build-an-atom-game/index.html

samreid commented 3 years ago

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 !?!?

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?

pixelzoom commented 3 years ago

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.

pixelzoom commented 3 years ago

@samreid and I paired on this via Zoom. We complete option (2) above, factoring out a function that handles making the phetioClient.invoke calls. This was working as of commit https://github.com/phetsims/fourier-making-waves/commit/39de74478331fbfb3747b738af5734e6d9e87d35.

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 script loaded by the wrapper.

  /**
   * 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:

screenshot_1197
pixelzoom commented 3 years ago

@samreid said:

In our discussion, I neglected to describe our policy that PhET-iO wrapper code should not be checked in to public repos. ...

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.

pixelzoom commented 3 years ago

@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 );
      }
    }
  }
} );

WaveGameScreenIO delegates to WaveGameScreen for the implementation of the initializeGame method. Here's what that method might look like:

  /**
   * 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.

pixelzoom commented 3 years ago

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.

pixelzoom commented 3 years ago

The Fourier team has decided not to include PhET-iO in the 1.0 release. Deferring this issue.

pixelzoom commented 3 years ago

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.

pixelzoom commented 2 years ago

@JacquiHayes FYI. At Q1 2022 planning meeting, asking the client about their needs for the game was established as a goal for this quarter.

zepumph commented 1 year ago

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.

JacquiHayes commented 1 year ago

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 https://github.com/phetsims/fourier-making-waves/issues/144#issuecomment-1490691616, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOLUG52QNCNXEGML3XSPSO3W6XBLFANCNFSM5CITFDQA . You are receiving this because you were mentioned.Message ID: @.***>

zepumph commented 1 year ago

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.

pixelzoom commented 1 year ago

General design issues that need to be discussed: