Skip to content

Commit

Permalink
Clarifications for #42
Browse files Browse the repository at this point in the history
Also related to #11
  • Loading branch information
Saurabh Totey authored and Saurabh Totey committed Jul 18, 2021
1 parent 1adccbb commit 00bb665
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 22 deletions.
19 changes: 9 additions & 10 deletions js/common/model/AbstractNLDBaseModel.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright 2020-2021, University of Colorado Boulder

/**
* Model for common properties/behaviours used by all scenes/screens in the sim.
* This class is incomplete and meant to be subclassed.
* Model for common properties/behaviours used by all scenes/screens in the sim. This class is incomplete and meant to
* be subclassed. It is assumed that all instances of this class are present for the lifetime of the simulation.
*
* @author Saurabh Totey
*/
Expand Down Expand Up @@ -64,19 +64,18 @@ class AbstractNLDBaseModel {

// @public (read-only) {Property.<number|null>} a property that stores the number line point values of the point controllers
// in the order that the point controllers were given to this model. If a point controller doesn't have a number
// line point, then null is recorded in the array instead.
// The stored array is always of length 2:
// index 0 corresponds to the value of the number line point of this.pointControllerOne and
// index 1 corresponds to the value of the number line point of this.pointControllerTwo.
// line point, then null is recorded in the array instead. The stored array is always of length 2:
// index 0 corresponds to the value of the number line point of this.pointControllerOne and index 1 corresponds to
// the value of the number line point of this.pointControllerTwo.
this.pointValuesProperty = new Property( [ null, null ] );
this.pointValuesProperty.link( pointValues => {
assert && assert( pointValues.length === 2, 'There should always be 2 point values.' );
} );

// Listen to the numberLine and its points to make updates to pointsValueProperty when necessary.
// Ideally, we would listen to the residentPoints of this.numberLine so I wouldn't duplicate code per controller, but
// it is necessary to know which point controller each number line point belongs to, and points are seemingly added
// to the number line before they are associated with a point controller.
// Listen to the numberLine and its points to make updates to pointsValueProperty when necessary. Ideally, we would
// listen to the residentPoints of this.numberLine so I wouldn't duplicate code per controller, but it is necessary
// to know which point controller each number line point belongs to, and points can be added to the number line
// before they are associated with a point controller.
this.pointControllerOne.numberLinePoints.addItemAddedListener( numberLinePoint => {
const updatePointValuesProperty = value => {
this.pointValuesProperty.value = [ value, this.pointValuesProperty.value[ 1 ] ];
Expand Down
6 changes: 3 additions & 3 deletions js/common/view/DistanceShadedNumberLineNode.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Copyright 2020-2021, University of Colorado Boulder

/**
* A node that is a number line and that also shades the distance between number line points.
* The space between number line points is only shaded when both point controllers are on the number line.
* A node that is a number line and that also shades the distance between number line points. The space between number
* line points is only shaded when both point controllers are on the number line. No unlinking is required because all
* instances of this number line are present for the lifetime of the simulation.
*
* @author Saurabh Totey
*/
Expand Down Expand Up @@ -78,7 +79,6 @@ class DistanceShadedNumberLineNode extends SpatializedNumberLineNode {
model.pointControllerTwo.isDraggingProperty.link( () => { pointNameBackground1.moveToFront(); } );

// Most of the number line's behaviour is handled in this multilink.
// Unlinking is unnecessary because every instance of this number line is present for the lifetime of the sim.
Property.multilink(
[
model.distanceLabelsVisibleProperty,
Expand Down
1 change: 1 addition & 0 deletions js/common/view/NLDBaseView.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* A view that contains elements that are used in all scenes/screens of the sim.
* This view has all the common controls as well as the common display elements.
* This is a 'base' view because it is meant to be used as the bottom layer in the scene graph for all scenes/screens.
* Nothing needs to be disposed from this view because all instances of NLDBaseView are present for the sim's lifetime.
*
* @author Saurabh Totey
*/
Expand Down
3 changes: 2 additions & 1 deletion js/explore/model/ExplorePointController.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class ExplorePointController extends PointController {
super( options );

// The dropping behaviour for #34: if the point controller is no longer being dragged but is in the relevant drop
// direction, then propose a new position for it into the play area.
// direction, then propose a new position for it into the play area. No unlink necessary because the point controllers
// are present for the sim's lifetime.
this.isDraggingProperty.link( isDragging => {
if ( isDragging ) {
return;
Expand Down
3 changes: 2 additions & 1 deletion js/explore/model/TemperaturePointController.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ class TemperaturePointController extends AreaPointController {

// @public (read-only) {PaintColorProperty} is different from this.color where this.color is the point controller's
// color for the purposes of the other number-line classes; this colorProperty instead is just representative of
// the temperature of this point controller.
// the temperature of this point controller. No unlink necessary since all TemperaturePointControllers are present
// for the sim's lifetime.
this.colorProperty = new PaintColorProperty( NO_TEMPERATURE_COLOR );
this.positionProperty.link( () => {
if ( this.isControllingNumberLinePoint() ) {
Expand Down
7 changes: 4 additions & 3 deletions js/explore/view/ElevationPointControllerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ class ElevationPointControllerNode extends PointControllerNode {
*/
constructor( pointController, seaLevel, belowSeaLevelImage, aboveSeaLevelImage ) {

// dilates each image's touch area
// Dilate each image's touch area.
belowSeaLevelImage.touchArea = belowSeaLevelImage.localBounds.dilated( IMAGE_DILATION );
aboveSeaLevelImage.touchArea = aboveSeaLevelImage.localBounds.dilated( IMAGE_DILATION );

// create a node with all the images that will be used to depict this elevatable item
// Create a node with all the images that will be used to depict this elevatable item.
const compositeImageNode = new Node( { children: [ belowSeaLevelImage, aboveSeaLevelImage ] } );

// update the visibility of the images as the position changes
// Update the visibility of the images as the position changes. No unlink necessary as ElevationPointControllers are
// always present for the sim's lifetime.
pointController.positionProperty.link( position => {
if ( position.y > seaLevel ) {
aboveSeaLevelImage.visible = false;
Expand Down
8 changes: 4 additions & 4 deletions js/explore/view/NLDExploreScreenView.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,22 @@ class NLDExploreScreenView extends ScreenView {
tandem: tandem
} );

// adds scene views as children
// Add scene views as children.
const distanceSceneView = new DistanceSceneView( model.distanceSceneModel );
this.addChild( distanceSceneView );
const temperatureSceneView = new TemperatureSceneView( model.temperatureSceneModel );
this.addChild( temperatureSceneView );
const elevationSceneView = new ElevationSceneView( model.elevationSceneModel );
this.addChild( elevationSceneView );

// links each specific scene view's visibility with whether it is selected in the model
// Link each specific scene view's visibility with whether it is selected in the model.
model.selectedSceneProperty.link( selectedScene => {
distanceSceneView.visible = selectedScene === NLDScene.DISTANCE;
temperatureSceneView.visible = selectedScene === NLDScene.TEMPERATURE;
elevationSceneView.visible = selectedScene === NLDScene.ELEVATION;
} );

// map the scene selection icons to their enum values (used in the radio button group)
// Map the scene selection icons to their enum values (used in the radio button group).
const thermometerSceneIcon = new Rectangle( 0, 0, ICON_SIZE.width, ICON_SIZE.height );
const thermometerNode = new ThermometerNode( 0, 1, new NumberProperty( 0.5 ) );
thermometerNode.setScaleMagnitude( ICON_SIZE.height / thermometerNode.height );
Expand Down Expand Up @@ -90,7 +90,7 @@ class NLDExploreScreenView extends ScreenView {
} );
this.addChild( resetAllButton );

// create scene selector radio buttons
// Create scene selector radio buttons.
const sceneSelectorRadioButtonGroup = new RectangularRadioButtonGroup(
model.selectedSceneProperty,
sceneSelectionButtonsContent,
Expand Down

0 comments on commit 00bb665

Please sign in to comment.