Skip to content
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

TEmitter declarations are confusing #460

Open
AgustinVallejo opened this issue Feb 20, 2025 · 1 comment
Open

TEmitter declarations are confusing #460

AgustinVallejo opened this issue Feb 20, 2025 · 1 comment

Comments

@AgustinVallejo
Copy link
Contributor

As part of phetsims/models-of-the-hydrogen-atom#125 I came across this:

  public readonly photonEmittedEmitter: TEmitter<[ number, Vector2, number, Color ]>;

  ...

    this.photonEmittedEmitter = new Emitter( {
      parameters: [
        { name: 'wavelength', valueType: 'number', phetioType: NumberIO },
        { name: 'position', valueType: Vector2, phetioType: Vector2.Vector2IO },
        { name: 'direction', valueType: 'number', phetioType: NumberIO },
        { name: 'debugHalo', valueType: Color || null, phetioType: NullableIO( Color.ColorIO ) }
      ],
    ...
    } );

Which is a bit odd, since you have to look at the definition to understand what those parameters mean. Very different to Property<> to which you can just pass a type. I asked @pixelzoom and he agreed on finding Emitter confusing. So we were wondering if there's something better we can do.

@zepumph
Copy link
Member

zepumph commented Feb 20, 2025

Very different to Property<> to which you can just pass a type.

Property takes a single value, and allows you to provide a Validator and phetioType to it. Emitter can take N values, and allows you to provide a validator and phetioType to each. The Validator is only for runtime assertion validation, and so you do not need to provide the valueType if it doesn't seem helpful to you (just as we do for Property).

The name attribute is a bit unfortunate, but it is for better data stream output, instead of an ordered list, they get a name for context:

  {
      "index": 819,
      "time": 1740071114277,
      "type": "model",
      "phetioID": "ohmsLaw.general.model.stepSimulationAction.executedEmitter",
      "name": "emitted",
      "componentType": "EmitterIO<NumberIO>",
      "data": {
        "dt": 0.016  <------ here, instead of just an array!
      }
    }

Much of the runtime validation that we originally developed is obsolete now because of Typescript, and so doesn't need to be duplicated. For example, I rarely use valueType in Property now, because I can just say Property<number>() instead of Property( {valueType: 'number'}). So for Emitter it is the same way. The name and phetioType are required for PhET-iO instrumentation.

    this.photonEmittedEmitter = new Emitter( {
      parameters: [
        { name: 'wavelength', phetioType: NumberIO },
        { name: 'position', phetioType: Vector2.Vector2IO },
        { name: 'direction', phetioType: NumberIO },
        { name: 'debugHalo', phetioType: NullableIO( Color.ColorIO ) }
      ],

Do you have any specific suggestions or thoughts about decreasing the confusion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants