REVIEW comments, #32

js/common/PDLConstants.ts

@@ -11,6 +11,7 @@ import projectileDataLab from '../projectileDataLab.js';
 import PhetFont from '../../../scenery-phet/js/PhetFont.js';
 import { BooleanProperty } from '../../../axon/js/imports.js';
 
+//REVIEW Constants should be uppercase.
 const maxFieldDistance = 100;
 const fieldWidth = 850;
 const pixelsToDistance = fieldWidth / maxFieldDistance;

js/common/PDLUtils.ts

@@ -15,6 +15,8 @@ import { VSMFieldIdentifierValues } from '../common-vsm/model/VSMFieldIdentifier
 import { SAMPLE_SIZES } from '../sampling/model/SamplingModel.js';
 
 export default class PDLUtils {
+
+  //REVIEW document
   public static transformField( point: Vector2 ): Vector2 {
     const pointY = point.y + PDLConstants.FIELD_SCALING_FACTOR_VERTICAL * Math.abs( point.y );
     const horizontalDistanceFactor = point.x / ( 0.5 * PDLConstants.FIELD_WIDTH );
@@ -24,11 +26,13 @@ export default class PDLUtils {
       pointY );
   }
 
+  //REVIEW document
   public static colorForFieldIndex( index: number ): Color {
     return PDLColors.fieldLightFillProperty.value.blend(
       PDLColors.fieldDarkFillProperty.value, index / ( VSMFieldIdentifierValues.length - 1 ) );
   }
 
+  //REVIEW document
   public static colorForSampleSize( sampleSize: number ): Color {
     const index = SAMPLE_SIZES.indexOf( sampleSize );
 

js/common/view/LaunchButton.ts

@@ -1,5 +1,6 @@
 // Copyright 2024, University of Colorado Boulder
 
+//REVIEW This class feels under-documented. There's a lot going on here with how this button changes based on Properties.
 /**
  * Launch button for the projectile data lab.
  *

js/common/view/LauncherIconNode.ts

@@ -1,5 +1,6 @@
 // Copyright 2024, University of Colorado Boulder
 
+//REVIEW This doc looks seems sort of incorrect, looks like it was copied from LauncherNode and sort of changed.
 /**
  * The LauncherIconNode is the visual representation of the projectile launcher used in the UI of Projectile Data Lab.
  *
@@ -25,7 +26,7 @@ type LauncherIconNodeOptions = SelfOptions & NodeOptions;
 
 export default class LauncherIconNode extends Node {
 
-  // Create adapters for that will reflect the values for the selected launcher.
+  // Create adapters that will reflect the values for the selected launcher.
   private launcherConfigurationProperty = new Property<LauncherConfiguration>( 'angle30' );
   private launchAngleProperty = new NumberProperty( 30 );
   private launchHeightProperty = new NumberProperty( 0 );

js/common/view/LauncherNode.ts

@@ -199,13 +199,15 @@ export default class LauncherNode extends Node {
     this.y = this.modelViewTransform.modelToViewY( height );
   }
 
+  //REVIEW Unnecessary method. No members are accessed, could be moved outside class definition.
   private getCachedDarkerColorProperty( colorProperty: TReadOnlyProperty<Color> ): TReadOnlyProperty<Color> {
     if ( !darkerColorCacheMap.has( colorProperty ) ) {
       darkerColorCacheMap.set( colorProperty, new DerivedProperty( [ colorProperty ], color => color.darkerColor( 0.8 ) ) );
     }
     return darkerColorCacheMap.get( colorProperty )!;
   }
 
+  //REVIEW document
   private launcherBarrelGraphicsForType( mysteryLauncherNumber: number, isIcon: boolean ): Node[] {
     const barrelColorProperty = PDLColors.mysteryLauncherColorProfiles[ mysteryLauncherNumber - 1 ].barrelFillProperty;
     const barrelDarkColorProperty = this.getCachedDarkerColorProperty( barrelColorProperty );
@@ -282,6 +284,7 @@ export default class LauncherNode extends Node {
     return [ barrel, ...( patternImage ? [ patternImage ] : [] ), barrelBorder, ...( isIcon ? [] : [ this.labelNode ] ), launcherEndRect ];
   }
 
+  //REVIEW document
   private launcherFrameBackGraphicsForType( mysteryLauncher: number, isIcon: boolean ): Node[] {
     const frameBackground = new Path( this.guideRailInnerShape(), {
       fill: PDLColors.launcherBackFillProperty
@@ -331,6 +334,7 @@ export default class LauncherNode extends Node {
     return [ supportBar, frameBackground, frameBarTop, frameBarBottom ];
   }
 
+  //REVIEW document
   protected launcherFrameFrontGraphicsForType( mysteryLauncher: number, outerRadiusCutoff = 0 ): Node[] {
 
     const fillColorProperty = PDLColors.mysteryLauncherColorProfiles[ mysteryLauncher - 1 ].frameFillProperty;
@@ -354,11 +358,13 @@ export default class LauncherNode extends Node {
     return [ guideRail ];
   }
 
+  //REVIEW Unnecessary method. No members are accessed, could be moved outside class definition.
   private guideRailInnerShape(): Shape {
     return new Shape().arc( 0, 0, GUIDE_RAIL_INNER_RADIUS, GUIDE_RAIL_MIN_ANGLE, GUIDE_RAIL_MAX_ANGLE )
       .lineTo( 0, 0 ).close();
   }
 
+  //REVIEW Unnecessary method. No members are accessed, could be moved outside class definition.
   private guideRailOuterShape( outerRadiusCutoff = 0 ): Shape {
     return new Shape().arc( 0, 0, GUIDE_RAIL_OUTER_RADIUS - outerRadiusCutoff, GUIDE_RAIL_MIN_ANGLE, GUIDE_RAIL_MAX_ANGLE )
       .lineTo( 0, 0 ).close();

js/common/view/MysteryLauncherIcon.ts

@@ -19,7 +19,7 @@ export class MysteryLauncherIcon extends Node {
 
     const mysteryLauncherIcon = new LauncherNode( ModelViewTransform2.createIdentity(), new Property( 45 ), new Property( 0 ),
       new Property( mysteryLauncher ), null, { isIcon: true } ).rasterized( {
-      resolution: 2.22
+      resolution: 2.22 //REVIEW NodeOptions.resolution is undocumented, this was new to me, so I can't grok 2.22.
     } );
     super( {
       children: [ mysteryLauncherIcon ],

js/common/view/MysteryLauncherRadioButtonGroupWrapper.ts

@@ -1,5 +1,7 @@
 // Copyright 2023-2024, University of Colorado Boulder
 
+//REVIEW MysteryLauncherControl might be a better name for this.
+//REVIEW Not clear how you achieved a grid layout, which is presumably the reason for this 'wrapper', elaborate with comments.
 /**
  * MysteryLauncherRadioButtonGroupWrapper is a control that shows the radio buttons that choose between the different
  * mystery launchers. A wrapper is needed to help the layout wrap.

js/common/view/PDLCanvasNode.ts

@@ -77,6 +77,7 @@ export default abstract class PDLCanvasNode<T extends Field> extends CanvasNode
     this.modelViewTransform = modelViewTransform;
   }
 
+  //REVIEW document
   protected drawOutlierGraphicsForLandedProjectiles( landedProjectiles: Projectile[], context: CanvasRenderingContext2D ): void {
 
     // Force field graphics for landed outliers
@@ -92,6 +93,7 @@ export default abstract class PDLCanvasNode<T extends Field> extends CanvasNode
     } );
   }
 
+  //REVIEW document
   protected drawPathForProjectile( context: CanvasRenderingContext2D, projectile: Projectile ): void {
     context.beginPath();
     let pathStarted = false;
@@ -117,6 +119,7 @@ export default abstract class PDLCanvasNode<T extends Field> extends CanvasNode
     context.stroke();
   }
 
+  //REVIEW document
   protected drawProjectile( context: CanvasRenderingContext2D, projectile: Projectile, isLanded: boolean, isSelected: boolean ): void {
 
     const viewPoint = this.modelViewTransform.modelToViewXY( projectile.x, projectile.y );

js/common/view/PDLLaunchPanel.ts

@@ -1,5 +1,6 @@
 // Copyright 2023-2024, University of Colorado Boulder
 
+//REVIEW Incorrect doc, copy-pasted from PDLPanel.
 /**
  * The PDLPanel defines the default look and feel for panels in the Projectile Data Lab.
  *

js/common/view/PDLPanelSection.ts

@@ -1,8 +1,8 @@
 // Copyright 2023-2024, University of Colorado Boulder
 
 /**
- * Shows a title and content in a VBox. For the first 3 screens, the content is rectangular radio buttons.
- * For the Sampling screen, there are other content types.
+ * PDLPanelSection is a base class for showing a title and content in a VBox. For the first 3 screens, the content is
+ * rectangular radio buttons. For the Sampling screen, there are other content types.
  *
  * @author Matthew Blackman (PhET Interactive Simulations)
  * @author Sam Reid (PhET Interactive Simulations)

js/common/view/PDLRectangularRadioButtonGroup.ts

@@ -1,8 +1,8 @@
 // Copyright 2024, University of Colorado Boulder
 
 /**
- * PDLRectangularRadioButtonGroup is a RectangularRadioButtonGroup with a default look that is used in the majority of radio buttons
- * within Projectile Data Lab.
+ * PDLRectangularRadioButtonGroup is a RectangularRadioButtonGroup with a default look that is used in the majority of
+ * radio buttons within Projectile Data Lab.
  *
  * @author Sam Reid (PhET Interactive Simulations)
  */

js/common/view/PDLScreenView.ts

@@ -45,8 +45,9 @@ export type PDLScreenViewOptions = SelfOptions & WithRequired<ScreenViewOptions,
 export default abstract class PDLScreenView<T extends Field> extends ScreenView {
 
   protected readonly modelViewTransform;
-  protected readonly canvasBounds: Bounds2;
+  protected readonly canvasBounds: Bounds2;//REVIEW document - what canvas?
 
+  //REVIEW launcherLayer is no longer needed. Now that PDLScreenView creates launcherNode, you just put launcherNode in the correct rendering order.
   protected readonly launcherLayer = new Node();
   protected readonly behindProjectilesLayer = new Node();
   protected readonly projectileLayer = new Node();