PhET-iO instrumentation of dynamic elements

[pixelzoom]

Related to phetsims/tandem#192 (https://github.com/phetsims/equality-explorer/issues/192). @amanda-phet @catherinecarter FYI.

Here's a summary of my current understanding of this sim's dynamic elements.

IOTypes that will be needed:

• TermIO and its subclasses (ConstantTermIO, ObjectTermIO, VariableTermIO)
• SnapshotIO and its components (PlateSnapshotIO, TermSnapshotIO, TermIO)

PhetioGroups that will be needed, each wrapped in a Collection (the pattern I used in natural-selection):

• TermGroup and its subclasses (ConstantTermGroup, ObjectTermGroup, VariableTermGroup)
• TermNodeGroup and its subclasses (ConstantTermNodeGroup, ObjectTermNodeGroup, VariableTermNodeGroup)
• SnapshotGroup
• UniversalOperationGroup (or convert to static instances and mutate?)
• UniversalOperationNodeGroup (or convert to static instances and mutate?)

If animation needs to be stateful for phetsims/tandem#197, these related things need to be examined:

• SumToZeroNode and it’s twixt.Animation
• TranslateThenFade (chained twixt.Animations) in UniversalOperationControl
• faceOpacity animation in SolveItLevelNode
• RewardNode animation in SolveItLevelNode
• twixt.TransitionNode is SolveItScreenView

[pixelzoom]

There are no plans to work on this until the initial instrumentation has been reviewed, and the initial PhET-iO design has been completed. Reminder that I have not commited to completing dynamic instrumentation.

This issue is also blocked by https://github.com/phetsims/equality-explorer/issues/204 (PhetioGroup does not support Prototype pattern).

[pixelzoom]

There are currently 31 TODOs in the code that reference this issue:

//TODO https://github.com/phetsims/equality-explorer/issues/200

[pixelzoom]

@amanda-phet @catherinecarter Before reviewing the Studio tree, read through model.md and implementation-notes.md. The essential elements to understand are:

• Scene
• Term
• TermCreator
• Snapshot
• UniversalOperator

[pixelzoom]

@kathy-phet @amanda-phet FYI.

Before commiting to finish instrumentation of the equality-explorer suite, I'd like to investigate a new approach to managing dynamic elements. I ran this past @samreid, and he gave it his blessing. A sprint of 3-4 days would allow me to do a proper investigation. The goal of this approach is to minimize the code changes required to manage dynamic elements via PhetioGroup, a major pain point for retrofitting existing sims.

Here's a sketch of the approach...

For each class that requires dynamic elements, create a class that manages a "group" of those instances. Internally, a group will use a PhetioGroup to handle object creation (via multiple patterns, like new and copy, see https://github.com/phetsims/equality-explorer/issues/204) and disposal. (Note that we're using composition, not inheritance, so that we can hide the API of PhetioGroup.)

Here's an example for the ContantTerm model class in equality-explorer. (These are the elements that can be dragged around that have a constant numeric value, like "1".)

/**
 * ConstantTermGroup managers creation and disposal of ConstantTerm instances.
 */
class ConstantTermGroup {

  // PhetioGroup is an internal detail, not visible outside the group.
  private readonly phetioGroup: PhetioGroup;

  public constructor( ... ) {

    // The function that PhetioGroup uses to instantiate a ConstantTerm.
    const createElement = ( tandem: Tandem, ... ) => new ConstantTerm( ... );

    this.phetioGroup = new PhetioGroup( createElement, ... );
  }

  // Replace new ContantTerm( ... ) with constantTermGroup.newElement( ... )
  public newElement( ... ): ConstantTerm {
     return this.phetioGroup.createNextElement( ... );
  }

  // Replace term.copy( ... ) with constantTermGroup.copyElement( ... )
  public copyElement( term: ConstantTerm, ... ): ConstantTerm {
     return this.phetioGroup.createNextElement( ... );
  }

  // Replace term.dispose() with constantTermGroup.disposeElement( term )
  public disposeElement( term: ConstantTerm ) {
    this.phetioGroup.disposeElement( term );
  }

  // etc.
}

The goal would be for newElement to have the same signature as ConstantTerm's constructor, so that replacing new ConstantTerm would look like this, requiring minimal disturbance, and minimal changes to arguments:

-     const term = new ConstantTerm( {
+     const term = constantTermGroup.newConstantTerm( {
        constantValue: Fraction.fromInteger( i )
      } );

Ditto for other methods. Here's an example of how to replace term.copy:

-         const termCopy = term.copy( {
+         const termCopy = constantTermGroup.copy( this.term, {
            diameter: EqualityExplorerConstants.BIG_TERM_DIAMETER
          } );

After creating a group class for each dynamic element class, I would start by using 1 global instance of each group class -- that will be most expedient for testing this approach.

The primary advantage of using globals is that it will minimize changes to code; we don't need to pass around group instances.

The primary disadvantage of using globals is that instances for all screens will be managed by the same group. That could be a problem for sims that (for example) need per-screen notification when an instance is created/disposed. If that proves to be problematic/unacceptable for equality-explorer, then we can create a group per screen, and pass them to where they are needed -- which will involve touching a lot more code. The group class implementation should be the same, regardless of whether we use globals or per-screen instantances.