Skip to content

Commit

Permalink
REVIEW comments, #32
Browse files Browse the repository at this point in the history
  • Loading branch information
pixelzoom committed Mar 4, 2024
1 parent ce4fe63 commit 7c8143c
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 3 deletions.
2 changes: 2 additions & 0 deletions js/common-vsm/model/VSMModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,14 @@ export default class VSMModel<T extends VSMField> extends PDLModel<T> {

const measuringTapeTandem = providedOptions.tandem.createTandem( 'measuringTape' );

//README Why is this not Vector2Property?
this.measuringTapeBasePositionProperty = new Property<Vector2>( new Vector2( 0, 0 ), {
tandem: measuringTapeTandem.createTandem( 'basePositionProperty' ),
phetioValueType: Vector2.Vector2IO,
phetioReadOnly: true
} );

//README Why is this not Vector2Property?
this.measuringTapeTipPositionProperty = new Property<Vector2>( new Vector2( 50, 0 ), {
tandem: measuringTapeTandem.createTandem( 'tipPositionProperty' ),
phetioValueType: Vector2.Vector2IO,
Expand Down
3 changes: 3 additions & 0 deletions js/common/model/Field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ export default abstract class Field extends PhetioObject {
} : { validValues: [ 'angle45' ] } as const;

const initialAngle = options.isLauncherConfigurationPhetioInstrumented ? 'angle30' : 'angle45';

//REVIEW should be StringUnionProperty<LauncherConfiguration>
this.launcherConfigurationProperty = new Property<LauncherConfiguration>( initialAngle, launcherConfigurationOptions );

const projectileTypeOptions = options.isProjectileTypePhetioInstrumented ? {
Expand All @@ -183,6 +185,7 @@ export default abstract class Field extends PhetioObject {
phetioValueType: ProjectileType.ProjectileTypeIO
} : { validValues: [ CANNONBALL ] } as const;

//REVIEW should be StringUnionProperty<ProjectileType>
this.projectileTypeProperty = new Property<ProjectileType>( CANNONBALL, projectileTypeOptions );

this.meanAngleProperty = new DerivedProperty( [ this.launcherConfigurationProperty ],
Expand Down
2 changes: 1 addition & 1 deletion js/common/model/HistogramSonifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const MEAN_HIGHLIGHT_DURATION = 0.4;

export default class HistogramSonifier {

//REVIEW document
//REVIEW document, should be StringUnionProperty<HistogramSonifierPhase>
public readonly histogramSonifierPhaseProperty = new Property<HistogramSonifierPhase>( { phaseName: 'idlePhase' } );

// The sorted data from the left to the rightmost bin
Expand Down
1 change: 1 addition & 0 deletions js/common/model/Launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export default class Launcher extends PhetioObject {

super( options );

//REVIEW should be StringUnionProperty<LauncherMechanism>
this.launcherMechanismProperty = new Property( launcherMechanism, {
tandem: options.tandem.createTandem( 'launcherMechanismProperty' ),
phetioFeatured: true,
Expand Down
1 change: 1 addition & 0 deletions js/common/model/PDLModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export default abstract class PDLModel<T extends Field> implements TModel {

this.histogram = new Histogram( () => this.shouldPlayMeanTone(), () => this.playMeanTone(), { tandem: providedOptions.tandem.createTandem( 'histogram' ) } );

//REVIEW should be StringUnionProperty<SingleOrContinuous>
this.singleOrContinuousProperty = new Property<SingleOrContinuous>( 'single', {
validValues: SingleOrContinuousValues,
tandem: providedOptions.tandem.createTandem( 'singleOrContinuousProperty' ),
Expand Down
2 changes: 1 addition & 1 deletion js/common/view/LauncherIconNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default class LauncherIconNode extends Node {
private launcherConfigurationProperty = new Property<LauncherConfiguration>( 'angle30' );
private launchAngleProperty = new NumberProperty( 30 );
private launchHeightProperty = new NumberProperty( 0 );
private mysteryOrCustomProperty = new Property<MysteryOrCustom>( 'mystery' );
private mysteryOrCustomProperty = new Property<MysteryOrCustom>( 'mystery' ); //REVIEW should be StringUnionProperty<MysteryOrCustom>
private mysteryLauncherProperty = new Property( MYSTERY_LAUNCHERS[ 0 ] );
private launcherMechanismProperty = new Property<LauncherMechanism>( SPRING );
private standardDeviationAngleProperty = new Property( 0 );
Expand Down
2 changes: 1 addition & 1 deletion js/sources/view/SourcesScreenView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default class SourcesScreenView extends VSMScreenView<SMField> {
model.launcherConfigurationProperty,
model.meanLaunchAngleProperty,
model.launcherHeightProperty,
new Property( 'custom' ),
new Property( 'custom' ), //REVIEW should be new StringUnionProperty<MysteryOrCustom>( 'custom' )
new Property( launcher ),
model.customLauncherMechanismProperty,
model.standardDeviationAngleProperty,
Expand Down

1 comment on commit 7c8143c

@pixelzoom
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.