Skip to content

Commit

Permalink
add doc for all registration of observers, add TODO where dispose is …
Browse files Browse the repository at this point in the history
…still needed, #78
  • Loading branch information
pixelzoom committed Apr 24, 2017
1 parent 44d5fda commit d7cebf9
Show file tree
Hide file tree
Showing 18 changed files with 42 additions and 3 deletions.
3 changes: 2 additions & 1 deletion js/common/model/LineFormsModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ define( function( require ) {
this.savedLines = new ObservableArray();
this.standardLines = new ObservableArray();

// Update the lines seen by the graph. unmultilink unnecessary because we own these Properties.
// Update the lines seen by the graph.
// unmultilink is unnecessary because we own these Properties, and the model exists for the lifetime of the sim.
Property.multilink( [ this.interactiveLineProperty, this.savedLines.lengthProperty, this.standardLines.lengthProperty ],
function() {
self.graph.lines.clear();
Expand Down
1 change: 1 addition & 0 deletions js/common/view/EquationControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ define( function( require ) {
} );

// Sets the enabled states of the Save and Erase buttons
// unmultilink is unnecessary since EquationControls exists for the lifetime of the sim.
Property.multilink( [ savedLines.lengthProperty, linesVisibleProperty ],
function() {
saveLineButton.enabled = linesVisibleProperty.get();
Expand Down
8 changes: 8 additions & 0 deletions js/common/view/GraphControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,25 @@ define( function( require ) {
Panel.call( this, contentNode, options );

// when lines are not visible, hide related controls
// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
linesVisibleProperty.link( function( visible ) {
notLinesVisibleProperty.set( !visible );
yEqualsXCheckBox.enabled = visible;
yEqualsNegativeXCheckBox.enabled = visible;
slopeCheckBox.enabled = visible;
} );

// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
notLinesVisibleProperty.link( function( visible ) {
linesVisibleProperty.set( !visible );
} );

// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
gridVisibleProperty.link( function( visible ) {
notGridVisibleProperty.set( !visible );
} );

// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
notGridVisibleProperty.link( function( visible ) {
gridVisibleProperty.set( !visible );
} );
Expand All @@ -126,16 +130,19 @@ define( function( require ) {
};

// Add/remove standard line 'y = x'
// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
yEqualsXVisibleProperty.link( function( visible ) {
setStandardLineVisible( visible, Line.Y_EQUALS_X_LINE );
} );

// Add/remove standard line 'y = -x'
// unlink is unnecessary since GraphControls exists for the lifetime of the sim.
yEqualsNegativeXVisibleProperty.link( function( visible ) {
setStandardLineVisible( visible, Line.Y_EQUALS_NEGATIVE_X_LINE );
} );

// Select appropriate check boxes when standard lines are added.
// removeItemAddedListener is unnecessary since GraphControls exists for the lifetime of the sim.
standardLines.addItemAddedListener( function( line ) {
if ( line === Line.Y_EQUALS_X_LINE ) {
yEqualsXVisibleProperty.set( true );
Expand All @@ -146,6 +153,7 @@ define( function( require ) {
} );

// Deselect appropriate check boxes when standard lines are removed.
// removeItemRemovedListener is unnecessary since GraphControls exists for the lifetime of the sim.
standardLines.addItemRemovedListener( function( line ) {
if ( line === Line.Y_EQUALS_X_LINE ) {
yEqualsXVisibleProperty.set( false );
Expand Down
5 changes: 5 additions & 0 deletions js/common/view/LineFormsGraphNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,28 @@ define( function( require ) {
this.addChild( this.slopeToolNode );

// Add/remove standard lines
// remove*Listener not needed because LineFormsGraphNode exists for the lifetime of the sim.
model.standardLines.addItemAddedListener( this.standardLineAdded.bind( this ) );
model.standardLines.addItemRemovedListener( this.standardLineRemoved.bind( this ) );

// Add/remove saved lines
// remove*Listener not needed because LineFormsGraphNode exists for the lifetime of the sim.
model.savedLines.addItemAddedListener( this.savedLineAdded.bind( this ) );
model.savedLines.addItemRemovedListener( this.savedLineRemoved.bind( this ) );

// Visibility of lines
// unmultilink is unnecessary since LineFormsGraphNode exists for the lifetime of the sim.
Property.multilink( [ viewProperties.linesVisibleProperty, viewProperties.slopeToolVisibleProperty ],
this.updateLinesVisibility.bind( this ) );

// Visibility of the grid
// unlink is unnecessary since LineFormsGraphNode exists for the lifetime of the sim.
viewProperties.gridVisibleProperty.link( function( visible ) {
self.setGridVisible( visible );
} );

// Visibility of the equation on the interactive line
// unlink is unnecessary since LineFormsGraphNode exists for the lifetime of the sim.
this.viewProperties.interactiveEquationVisibleProperty.link( function( visible ) {
if ( self.interactiveLineNode ) {
self.interactiveLineNode.setEquationVisible( visible );
Expand Down
1 change: 1 addition & 0 deletions js/linegame/model/BaseGameModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ define( function( require ) {
this.initChallenges();

// Do this after initChallenges, because this will fire immediately and needs to have an initial set of challenges.
// unlink is unnecessary since BaseGameModel exists for the lifetime of the sim.
this.playStateProperty.link( function( playState ) {

var challengeIndex = self.challengeIndexProperty.get();
Expand Down
1 change: 1 addition & 0 deletions js/linegame/model/Challenge.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ define( function( require ) {
this.pointTool2 = new PointTool( new Vector2( 7, -13 ), 'down', this.graph.lines, new Bounds2( -15, -14, 11, 11 ) );

// {Line|NotALine} When the guess changes, update the lines that are 'seen' by the point tools.
// unlink unnecessary because Challenge owns this Property.
this.guessProperty.link( this.updateGraphLines.bind( this ) );
}

Expand Down
1 change: 1 addition & 0 deletions js/linegame/model/PlaceThePoints.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ define( function( require ) {
this.p3Property = new Property( new Vector2( 3, 2 ) );

// update the guess when the points change
// unmultilink unnecessary because PlaceThePoints owns these Properties.
var self = this;
Property.multilink( [ this.p1Property, this.p2Property, this.p3Property ],
function( p1, p2, p3 ) {
Expand Down
1 change: 1 addition & 0 deletions js/linegame/view/BaseGameScreenView.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ define( function( require ) {
this.addChild( this.settingsNode );

// game 'phase' changes
// unlink unnecessary because BaseGameScreenView exists for the lifetime of the sim.
var self = this;
model.gamePhaseProperty.link( function( gamePhase ) {
self.settingsNode.visible = ( gamePhase === GamePhase.SETTINGS );
Expand Down
1 change: 1 addition & 0 deletions js/linegame/view/PlayNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ define( function( require ) {
this.addChild( challengeParent );

// Set up a new challenge
// unlink unnecessary because PlayNode exists for the lifetime of the sim.
model.challengeProperty.link( function( challenge ) {

// dispose of view for previous challenge
Expand Down
4 changes: 3 additions & 1 deletion js/linegame/view/ResultsNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ define( function( require ) {
*/
function ResultsNode( model, layoutBounds, audioPlayer, rewardFactoryFunctions ) {

var self = this;

Node.call( this );

this.rewardNode = null; // @private

// show results when we enter this phase
var self = this;
// unlink unnecessary because ResultsNode exists for the lifetime of the sim.
model.gamePhaseProperty.link( function( gamePhase ) {
if ( gamePhase === GamePhase.RESULTS ) {

Expand Down
4 changes: 3 additions & 1 deletion js/pointslope/model/PointSlopeModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ define( function( require ) {
*/
function PointSlopeModel( interactiveLine, parameterRange ) {

var self = this;

interactiveLine = interactiveLine || Line.createPointSlope( 1, 2, 3, 4, GLColors.INTERACTIVE_LINE );
parameterRange = parameterRange || new PointSlopeParameterRange();

Expand All @@ -36,7 +38,7 @@ define( function( require ) {
this.runRangeProperty = new Property( this.graph.xRange );

// Dynamically adjust ranges so that variables are constrained to the bounds of the graph.
var self = this;
// unlink unnecessary because PointSlopeModel exists for the lifetime of the sim.
this.interactiveLineProperty.link( function( line ) {
self.x1RangeProperty.set( parameterRange.x1( line, self.graph ) );
self.y1RangeProperty.set( parameterRange.y1( line, self.graph ) );
Expand Down
4 changes: 4 additions & 0 deletions js/pointslope/view/PointSlopeEquationNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ define( function( require ) {
}
};

//TODO #78 unmultilink
// sync the model with the controls
Property.lazyMultilink( [ x1Property, y1Property, riseProperty, runProperty ],
function() {
Expand All @@ -363,6 +364,7 @@ define( function( require ) {
}
);

//TODO #78 unlink
// sync the controls and layout with the model
lineProperty.link( function( line ) {

Expand Down Expand Up @@ -391,6 +393,8 @@ define( function( require ) {
self.addChild( undefinedSlopeIndicator );
undefinedSlopeIndicator.centerX = self.centerX;
undefinedSlopeIndicator.centerY = fractionLineNode.centerY - self.undefinedSlopeYFudgeFactor;

//TODO #78 unlink
lineProperty.link( function( line ) {
undefinedSlopeIndicator.visible = line.undefinedSlope();
} );
Expand Down
1 change: 1 addition & 0 deletions js/pointslope/view/PointSlopeGraphNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ define( function( require ) {
this.addChild( slopeManipulator );

// visibility of manipulators
// unlink unnecessary because PointSlopeGraphNode exists for the lifetime of the sim.
viewProperties.linesVisibleProperty.link( function( linesVisible ) {
x1y1Manipulator.visible = slopeManipulator.visible = linesVisible;
} );
Expand Down
1 change: 1 addition & 0 deletions js/slope/model/SlopeModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ define( function( require ) {
this.y2RangeProperty = new Property( this.graph.yRange );

// Dynamically adjust ranges so that variables are constrained to the bounds of the graph.
// unlink unnecessary because SlopeModel exists for the lifetime of the sim.
var parameterRange = new SlopeParameterRange();
this.interactiveLineProperty.link( function( line ) {
self.x1RangeProperty.set( parameterRange.x1( line, self.graph ) );
Expand Down
3 changes: 3 additions & 0 deletions js/slope/view/SlopeEquationNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ define( function( require ) {
parentNode.addChild( unsimplifiedFractionLineNode );
parentNode.addChild( unsimplifiedRunNode );

//TODO #78 unmultilink
// sync the model with the controls
Property.lazyMultilink( [ x1Property, y1Property, x2Property, y2Property ],
function() {
Expand All @@ -156,6 +157,7 @@ define( function( require ) {
}
);

//TODO #78 unlink
// sync the controls and layout with the model
lineProperty.link( function( line ) {

Expand Down Expand Up @@ -420,6 +422,7 @@ define( function( require ) {
}
};

//TODO #78 unlink
lineProperty.link( update.bind( this ) );

return equationNode;
Expand Down
1 change: 1 addition & 0 deletions js/slope/view/SlopeGraphNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ define( function( require ) {
this.addChild( x2y2Manipulator );

// visibility of manipulators
// unlink unnecessary because SlopeGraphNode exists for the lifetime of the sim.
viewProperties.linesVisibleProperty.link( function( linesVisible ) {
x1y1Manipulator.visible = x2y2Manipulator.visible = linesVisible;
} );
Expand Down
4 changes: 4 additions & 0 deletions js/slopeintercept/view/SlopeInterceptEquationNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ define( function( require ) {

//***************************************************************

//TODO #78 unmultilink
// sync the model with the controls
Property.lazyMultilink( [ riseProperty, runProperty, yInterceptProperty ],
function() {
Expand All @@ -373,6 +374,7 @@ define( function( require ) {
}
);

//TODO #78 unlink
// sync the controls and layout with the model
lineProperty.link( function( line ) {

Expand Down Expand Up @@ -411,6 +413,8 @@ define( function( require ) {
self.addChild( undefinedSlopeIndicator );
undefinedSlopeIndicator.centerX = self.centerX;
undefinedSlopeIndicator.centerY = slopeFractionLineNode.centerY - self.undefinedSlopeYFudgeFactor;

//TODO #78 unlink
lineProperty.link( function( line ) {
undefinedSlopeIndicator.visible = line.undefinedSlope();
} );
Expand Down
1 change: 1 addition & 0 deletions js/slopeintercept/view/SlopeInterceptGraphNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ define( function( require ) {
this.addChild( yInterceptManipulator );

// visibility of manipulators
// unlink unnecessary because SlopeInterceptGraphNode exists for the lifetime of the sim.
viewProperties.linesVisibleProperty.link( function( linesVisible ) {
slopeManipulator.visible = yInterceptManipulator.visible = linesVisible;
} );
Expand Down

0 comments on commit d7cebf9

Please sign in to comment.