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

IOType problems #325

Closed
pixelzoom opened this issue May 4, 2024 · 2 comments
Closed

IOType problems #325

pixelzoom opened this issue May 4, 2024 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented May 4, 2024

I was trying to use this sim as an exemplar for instrumenting particle systems in phetsims/gas-properties#231. I noted the following problems:

(1) In Projectile.ts, ProjectileIO does not identify which type of serialization it uses, as described in https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization.

(2) In Field.ts, type FieldStateObject is missing, and the result is a lack of type checking:

  • toStateObject(): object is almost as bad as using any.
  • applyState: ( field: Field, stateObject ) results in stateObject: any.

(3) Also in Field.ts, the documentation for FieldIO does not link to https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization.

(4) All of the IOTypes are currently public static. They should be public static readonly, e.g.:

-  public static FieldIO = new IOType<Field>( 'FieldIO', {
+  public static readonly FieldIO = new IOType<Field>( 'FieldIO', {
@pixelzoom
Copy link
Contributor Author

pixelzoom commented May 9, 2024

Reopening. This is important to get right because I'm using PDL as an exemplar for instrumenting parts of GP.

ProjectileIO and FieldIO are documented as "value-based serialization". That is not one of the 3 types of serialization at https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization. Do you mean data-type serialzation?

While ProjectileIO certainly looks like data-type serialization, I'm not convinced that FieldIO is data-type serialization. Data-type serialization does not typically use applyState, and I do not see Field instances being instantiated as the result of deserialization. FieldIO also does not seem like reference-type or dynamic-element serialization. Is it possible that this is a 4th serialization pattern that is not identified in the Technical Guide?

ProjectileIO doc is also missing the link to https://github.com/phetsims/phet-io/blob/main/doc/phet-io-instrumentation-technical-guide.md#serialization.

@pixelzoom pixelzoom reopened this May 9, 2024
@pixelzoom pixelzoom assigned samreid and unassigned pixelzoom May 9, 2024
samreid added a commit that referenced this issue May 9, 2024
@samreid
Copy link
Member

samreid commented May 9, 2024

I updated the documentation to answer the questions above, please review.

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

3 participants