phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Document IOTypes, identify type of serialization. #224

Closed pixelzoom closed 5 months ago

pixelzoom commented 5 months ago

For code review #32 ...

There is currently no documentation for IOTypes other than the documentation metadata, which is for Studio consumption. I consequently didn't spend much time looking at IOTypes, and (other than ProjectileIO) I don't have a good feel for the types of serialization used in this sim. At a mimimun, you should document which of the 3 types of serialization is used and why. This will at least get the reader (and your future self) pointed in the right direction, and accelerate understanding of what is one of the most complicated/confusing PhET-iO topics.

Some examples...

BunnyIO.ts:

  /**
   * BunnyIO implements 'Dynamic element serialization', as described in the Serialization section of
   * https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization
   * Dynamic element serialization is appropriate because Bunny instances are created dynamically - either as a
   * "generation-zero" bunny, or via mating. See BunnyCollection.createBunny.
   */
  public static readonly BunnyIO = new IOType<Bunny, BunnyStateObject>( 'BunnyIO', {

CurrentSource.ts:

  /**
   * CurrentSourceIO handles PhET-iO serialization of CurrentSource. Since all CurrentSource are created at startup and
   * exist for the lifetime of the simulation, it implements 'Reference type serialization', as described in the
   * Serialization section of https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization
   */
  public static readonly CurrentSourceIO = new IOType( 'CurrentSourceIO', {
    valueType: CurrentSource,
    supertype: ReferenceIO( IOType.ObjectIO ),
    documentation: 'A device that acts as the current source for an electromagnet'
  } );
samreid commented 5 months ago

@matthew-blackman and I addressed this, thanks. Closing.