Self code review for Balance Point directory, see: #258

js/balance-point/model/BalancePointModel.ts

@@ -26,7 +26,7 @@ const NUMBER_OF_KICKS_RANGE_PROPERTY = new Property( new Range( 0, MeanShareAndB
 
 export default class BalancePointModel extends SoccerModel<BalancePointSceneModel> {
 
-  // This Property determines wether the fulcrum is fixed on the current mean value, or is movable by the user.
+  // This Property determines whether the fulcrum is fixed on the current mean value, or is movable by the user.
   public readonly meanFulcrumFixedProperty: Property<boolean>;
 
   // Visible Properties
@@ -74,6 +74,7 @@ export default class BalancePointModel extends SoccerModel<BalancePointSceneMode
 
     super( [ sceneModel ], options );
 
+    // Create Properties
     this.meanFulcrumFixedProperty = meanFulcrumFixedProperty;
     this.fulcrumWasDraggedProperty = new BooleanProperty( false, {
       tandem: options.tandem.createTandem( 'fulcrumWasDraggedProperty' ),

js/balance-point/model/BalancePointSceneModel.ts

@@ -1,6 +1,7 @@
 // Copyright 2024, University of Colorado Boulder
 /**
- * The BalancePointSceneModel has the soccer ball information for the balance point screen.
+ * The BalancePointSceneModel has the soccer ball information for the balance point screen as well as the information
+ * needed to render the balance beam.
  *
  * @author Marla Schulz (PhET Interactive Simulations)
  *
@@ -93,10 +94,8 @@ export default class BalancePointSceneModel extends SoccerSceneModel {
       createSoccerBall,
       options
     );
-    this.beamSupportsPresentProperty = new BooleanProperty( true, {
-      tandem: options.tandem.createTandem( 'beamSupportsPresentProperty' )
-    } );
 
+    // Create Properties
     const valueDependencies = this.soccerBalls.map( ball => ball.valueProperty );
     const phaseDependencies = this.soccerBalls.map( ball => ball.soccerBallPhaseProperty );
     const positionDependencies = this.soccerBalls.map( ball => ball.positionProperty );
@@ -115,7 +114,24 @@ export default class BalancePointSceneModel extends SoccerSceneModel {
       phetioReadOnly: true,
       tandem: options.tandem.createTandem( 'targetNumberOfBallsProperty' )
     } );
+    this.meanPredictionFulcrumValueProperty = new NumberProperty( MeanShareAndBalanceConstants.FULCRUM_DEFAULT_POSITION, {
+      tandem: options.tandem.createTandem( 'meanPredictionFulcrumValueProperty' ),
+      range: MeanShareAndBalanceConstants.SOCCER_BALL_RANGE
+    } );
+    this.beamSupportsPresentProperty = new BooleanProperty( true, {
+      tandem: options.tandem.createTandem( 'beamSupportsPresentProperty' )
+    } );
+    this.leftBalanceBeamYValueProperty = new NumberProperty( FULCRUM_HEIGHT, {
+      tandem: options.tandem.createTandem( 'leftBalanceBeamYValueProperty' ),
+      phetioReadOnly: true
+    } );
+    this.rightBalanceBeamYValueProperty = new NumberProperty( FULCRUM_HEIGHT, {
+      tandem: options.tandem.createTandem( 'rightBalanceBeamYValueProperty' ),
+      phetioReadOnly: true
+    } );
 
+
+    // Listen to Properties
     this.targetNumberOfBallsProperty.lazyLink( ( newValue, oldValue ) => {
       const delta = newValue - oldValue;
       if ( delta > 0 ) {
@@ -134,20 +150,6 @@ export default class BalancePointSceneModel extends SoccerSceneModel {
       }
     } );
 
-    this.meanPredictionFulcrumValueProperty = new NumberProperty( MeanShareAndBalanceConstants.FULCRUM_DEFAULT_POSITION, {
-      tandem: options.tandem.createTandem( 'meanPredictionFulcrumValueProperty' ),
-      range: MeanShareAndBalanceConstants.SOCCER_BALL_RANGE
-    } );
-
-    this.leftBalanceBeamYValueProperty = new NumberProperty( FULCRUM_HEIGHT, {
-      tandem: options.tandem.createTandem( 'leftBalanceBeamYValueProperty' ),
-      phetioReadOnly: true
-    } );
-    this.rightBalanceBeamYValueProperty = new NumberProperty( FULCRUM_HEIGHT, {
-      tandem: options.tandem.createTandem( 'rightBalanceBeamYValueProperty' ),
-      phetioReadOnly: true
-    } );
-
     // Update the position of the beam as other aspects of the model change.
     Multilink.multilink(
       [
@@ -248,6 +250,9 @@ export default class BalancePointSceneModel extends SoccerSceneModel {
     );
   }
 
+  /**
+   * Regress the kickers line by removing the last kicked ball from the field.
+   */
   private regressLine(): void {
 
     this.activeKickIndexProperty.value = this.targetNumberOfBallsProperty.value;

js/balance-point/view/BalanceBeamNode.ts

@@ -97,7 +97,7 @@ export default class BalanceBeamNode extends Node {
     const triangleHeight = Math.abs( BALANCE_BEAM_TRANSFORM.modelToViewDeltaY( FULCRUM_HEIGHT ) );
     const triangleWidth = BALANCE_BEAM_TRANSFORM.modelToViewDeltaX( fulcrumWidth );
 
-    // The adjustable fulcrum that can be moved by the user.
+    // Create the adjustable fulcrum that can be moved by the user.
     const fulcrumSlider = new MeanPredictionFulcrumSlider(
       sceneModel.meanPredictionFulcrumValueProperty,
       fulcrumWasDraggedProperty,
@@ -112,7 +112,7 @@ export default class BalanceBeamNode extends Node {
       }
     );
 
-    // The fixed, non-movable fulcrum that is always at the mean and is shown in fixed fulcrum mode.
+    // Create the fixed, non-movable fulcrum that is always at the mean and is shown in fixed fulcrum mode.
     const fixedFulcrum = new TriangleNode(
       {
         fill: MeanShareAndBalanceColors.meanColorProperty,
@@ -152,6 +152,7 @@ export default class BalanceBeamNode extends Node {
       } );
     } );
 
+    // Create the line that represents the balance beam.
     const transformedLeftYValue = BALANCE_BEAM_TRANSFORM.modelToViewY( sceneModel.leftBalanceBeamYValueProperty.value );
     const transformedRightYValue = BALANCE_BEAM_TRANSFORM.modelToViewY( sceneModel.rightBalanceBeamYValueProperty.value );
     const beamLineNode = new Line( lineStartX, transformedLeftYValue, lineEndX, transformedRightYValue, {
@@ -191,9 +192,6 @@ export default class BalanceBeamNode extends Node {
     }, options );
     super( superOptions );
 
-    // Make the fulcrum slider available to methods.
-    this.fulcrumSlider = fulcrumSlider;
-
     // Align with the play area number line node, based on the tick mark values.
     const matrixBetweenProperty = new MatrixBetweenProperty(
       playAreaNumberLineNode.tickMarkSet,
@@ -262,7 +260,7 @@ export default class BalanceBeamNode extends Node {
         // Calculate the vectors needed to put the balls in a position such that they are directly above the
         // corresponding spot on the beam and the edge of the ball is touching the beam.  To do this, we calculate two
         // vectors, one for the minimum amount above the beam and one for the point where the edge of the ball touches
-        // the titled edge of the beam, and use the longer of the two.  These vectors are
+        // the titled edge of the beam, and use the longer of the two.
         const minOffsetVector = new Vector2( 0, -( BALL_GRAPHIC_RADIUS + MIN_BEAM_TO_BALL_BOTTOM_SPACING ) );
         const beamAngle = startToEndVector.getAngle();
         const rotatedRadiusVector = new Vector2( 0, -BALL_GRAPHIC_RADIUS ).rotated( beamAngle );
@@ -319,7 +317,7 @@ export default class BalanceBeamNode extends Node {
     } );
     ManualConstraint.create( this, [ needAtLeastOneKickMessage ], messageProxy => {
       messageProxy.centerX = BALANCE_BEAM_TRANSFORM.modelToViewX( MeanShareAndBalanceConstants.SOCCER_BALL_RANGE.getCenter() );
-      messageProxy.centerY = BALANCE_BEAM_TRANSFORM.modelToViewY( 2.1 ); // Y pos empirically determined
+      messageProxy.centerY = BALANCE_BEAM_TRANSFORM.modelToViewY( 2.1 ); // Y position empirically determined
     } );
     this.addChild( needAtLeastOneKickMessage );
 
@@ -359,6 +357,9 @@ export default class BalanceBeamNode extends Node {
         sceneModel.beamSupportsPresentProperty,
         { initialOutputLevel: 0.2 }
       ) );
+
+    // Make the fulcrum slider available to methods.
+    this.fulcrumSlider = fulcrumSlider;
   }
 
   public reset(): void {

js/balance-point/view/BalancePointScreenView.ts

@@ -57,27 +57,20 @@ export default class BalancePointScreenView extends SoccerScreenView<BalancePoin
       options.tandem.createTandem( 'sceneView' )
     );
 
+    // The play area center is calculated based on the layout bounds and the width of the controls.
     const controlsWidthOffset = ( MeanShareAndBalanceConstants.CONTROLS_PREFERRED_WIDTH +
                                   MeanShareAndBalanceConstants.CONTROLS_HORIZONTAL_MARGIN ) / 2;
     this.playAreaCenterX = this.layoutBounds.centerX - controlsWidthOffset;
 
-    // Background
     const backgroundNode = new BackgroundNode( MeanShareAndBalanceConstants.GROUND_POSITION_Y, this.visibleBoundsProperty );
-
     const questionBar = new QuestionBar( this.layoutBounds, this.visibleBoundsProperty, {
       questionString: MeanShareAndBalanceStrings.balancePointQuestionStringProperty,
       barFill: MeanShareAndBalanceColors.balancePointQuestionBarColorProperty
     } );
-
     const playAreaBounds = new Bounds2( this.layoutBounds.minX, this.layoutBounds.minY + questionBar.height,
       this.layoutBounds.maxX, this.layoutBounds.maxY );
 
-    // Controls
-    const controls = new BalancePointControls( model, {
-      tandem: options.tandem.createTandem( 'controls' )
-    } );
-
-    // Notepad
+    // Create the notepad that appears below the questionBar.
     const notepadNode = new BalancePointNotepadNode( sceneModel, this.playAreaNumberLineNode,
       model.fulcrumWasDraggedProperty, model.tickMarksVisibleProperty,
       model.meanFulcrumFixedProperty, {
@@ -87,6 +80,10 @@ export default class BalancePointScreenView extends SoccerScreenView<BalancePoin
       }
     );
 
+    // Create the controls that appear on the right side of the screen.
+    const controls = new BalancePointControls( model, {
+      tandem: options.tandem.createTandem( 'controls' )
+    } );
     const controlsAlignBox = new AlignBox( controls, {
       alignBounds: playAreaBounds,
       xAlign: 'right',
@@ -123,7 +120,7 @@ export default class BalancePointScreenView extends SoccerScreenView<BalancePoin
       leftTop: this.modelViewTransform.modelToViewXY( -2, 0 ).plusXY( 0, 8 )
     } );
 
-
+    // Add children to the scene graph in correct z-order.
     this.addChild( backgroundNode );
     this.addChild( sceneView.backSceneViewLayer );
     this.addChild( kickButton );
@@ -163,16 +160,16 @@ export default class BalancePointScreenView extends SoccerScreenView<BalancePoin
 
     this.addChild( meanCalculationPanel );
 
+    // Set the PDOM order of the nodes in the screen.
     this.pdomPlayAreaNode.setPDOMOrder( [
-      kickButton,
-      ...notepadNode.notepadPDOMOrder,
-      sceneView.backSceneViewLayer,
-      controls.numberSpinner
-      ]
-    );
+        kickButton,
+        ...notepadNode.notepadPDOMOrder,
+        sceneView.backSceneViewLayer,
+        controls.numberSpinner
+      ] );
 
     this.pdomControlAreaNode.setPDOMOrder( [
-        ...controls.controlsPDOMOrder,
+      ...controls.controlsPDOMOrder,
       resetAllButton
     ] );
   }

js/balance-point/view/MeanPredictionFulcrumSlider.ts

@@ -1,6 +1,6 @@
 // Copyright 2024, University of Colorado Boulder
 /**
- * The fulcrum of the balance beam is a slider that can move along the x-axis in increments of a tenth.
+ * The fulcrum of the balance beam is a slider that can move discretely along the x-axis.
  *
  * @author Marla Schulz (PhET Interactive Simulations)
  *
@@ -53,6 +53,7 @@ export default class MeanPredictionFulcrumSlider extends HSlider {
     providedOptions: MeanPredictionFulcrumSliderOptions
   ) {
 
+    // Create the thumb node for the fulcrum slider.
     const fulcrumNode = new TriangleNode( {
       triangleHeight: providedOptions.fulcrumHeight - FULCRUM_LINE_WIDTH,
       triangleWidth: providedOptions.fulcrumWidth,
@@ -61,10 +62,8 @@ export default class MeanPredictionFulcrumSlider extends HSlider {
       lineDash: [ 2, 2 ],
       lineWidth: FULCRUM_LINE_WIDTH
     } );
-
     const leftCueingArrow = new ArrowNode( 0, 0, -CUEING_ARROW_LENGTH, 0, CUEING_ARROW_OPTIONS );
     const rightCueingArrow = new ArrowNode( 0, 0, CUEING_ARROW_LENGTH, 0, CUEING_ARROW_OPTIONS );
-
     const thumbNode = new HBox( {
       children: [
         leftCueingArrow,
@@ -77,10 +76,7 @@ export default class MeanPredictionFulcrumSlider extends HSlider {
       phetioVisiblePropertyInstrumented: false
     } );
 
-    // sound generation
     const sliderSoundGenerator = new FulcrumSliderSoundPlayer( beamSupportsPresentProperty, meanValueProperty );
-
-    // Combine provided and default options.
     const options = optionize<MeanPredictionFulcrumSliderOptions, SelfOptions, HSliderOptions>()( {
       thumbNode: thumbNode,
       thumbYOffset: providedOptions.fulcrumHeight / 2,