phetsims / molecules-and-light

"Molecules and Light" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 5 forks source link

instrument with tandem #112

Closed jbphet closed 9 years ago

jbphet commented 9 years ago

add together/tandem support

jbphet commented 9 years ago

forget to mention this issue in another relevant commit: 8f588c49f5dd7a2a0ac691b64194df6c01748bcf

jbphet commented 9 years ago

Spent about 1 hour on this so far, which has been enough to show much of the basic functionality in the "mirror test". This was done with @samreid. This was much of the low hanging fruit, handling photon emission, molecule changes, and atom motion will be the trickier bits.

samreid commented 9 years ago

I instrumented the molecule radio buttons above, it took 15 minutes.

samreid commented 9 years ago

I instrumented the sliders above. This took approximately 30 minutes, which was more than I anticipated. The complications were:

  1. There are actually 4 different sliders, one for each emitter
  2. Previous sliders needed to be disposed() of
  3. Previous slider thumb needed a dispose method
  4. The model is powered by wavelength, not by emitter type and I needed a way to associate tandem UI component names with wavelengths. Perhaps a good long term solution for this would be to change the entire model to use emitter enums (emitter instances, not wavelength numbers). Instead I opted for this reverse-lookup table. If it is too unmaintainable-looking then we can rewrite the model to accommodate the tandem names.
    // Given a wavelength, look up the tandem name for an emitter
    // This is required because the simulation is driven by the wavelength value.  If this code is too instances
    // unmaintainable, we could rewrite the sim to use Emitter instances, each of which has a wavelength and a tandem name
    // See, for example: PhotonEmitterNode
    getTandemName: function( wavelength ) {
      return wavelength === this.SUNLIGHT_WAVELENGTH ? 'sunlight' :
             wavelength === this.MICRO_WAVELENGTH ? 'microwave' :
             wavelength === this.IR_WAVELENGTH ? 'infrared' :
             wavelength === this.VISIBLE_WAVELENGTH ? 'visible' :
             wavelength === this.UV_WAVELENGTH ? 'ultraviolet' :
             assert( false, 'unknown' );
    }
samreid commented 9 years ago

I instrumented play/pause and step buttons above. It took about 15 minutes, most of which was instrumenting ToggleButton and its model in sun (and adding dispose methods), which will be reused for other sims.

samreid commented 9 years ago

I instrumented the reset all button above. It was very straightforward and just (rounding up) 5 minutes.

samreid commented 9 years ago

I instrumented the show/close spectrum window buttons above in about 10 minutes. Will take more work to detect clicks within the spectrum window (which closes it) and even more work to detect clicks outside the spectrum window, which also closes it.

EDIT: Whatever solution we use for this dialog should also apply to the AboutDialog and other dialogs.

samreid commented 9 years ago

By the way, we are trying to keep good time records for this sim since we were very curious how long it takes to instrument a simulation. This seemed like a good candidate since I was unfamiliar with the codebase (@jbphet may be more familiar) and it hasn't previously been instrumented. But note these time estimates only apply to @jbphet and @samreid, since others wanting to instrument sims would have to climb a learning curve.

samreid commented 9 years ago

Took 5 minutes to fix tandem support for joist in the above commit.

samreid commented 9 years ago

In the above commit, I instrumented photons for playback. Took about an hour. Much of this time was split into two categories: (a) in common code for supporting ObservableArray for playback and (b) identifying #114. Instrumenting the observable array and adding to/from JSON for Photons was straightforward. Though I would like to discuss this pattern with someone:

In Photon.js

  // We must make Photon available to together.js for deserializing instances
  window.phet = window.phet || {};
  window.phet.moleculesAndLight = window.phet.moleculesAndLight || {};
  window.phet.moleculesAndLight.Photon = Photon;
jbphet commented 9 years ago

This work is largely complete, but there are a couple of known deficiencies. Right now, no user message is sent if the user opens up the spectrum window and then dismisses it by clicking on some portion of the window other than the close button, or by clicking outside of the spectrum window. This will require some work in Joist. I discussed this with the organization that is currently integrating this simulation into their environment, and they don't see this as a problem at all, since the state of the window (i.e. shown, not shown) is still conveyed.

jbphet commented 9 years ago

The deficiencies mentioned in the previous comments have been partially addressed by adding some additional support for tandem/together to Joist, see https://github.com/phetsims/together/issues/99. The one remaining problem is that no user message is sent when dismissing the spectrum window, or any other popup, by clicking on the window itself in a place other than the close button. We have decided to defer handling of this until the next round of interoperability planning and implementation. That means that for now at least, our work here is done. Closing.