From 3c8b4f920742bc99efd78a3a3a12b82f16ed18e3 Mon Sep 17 00:00:00 2001 From: zepumph Date: Sat, 24 Jun 2017 18:09:42 -0800 Subject: [PATCH] consolidate parameters to use model. Document why no dispose, https://github.com/phetsims/ohms-law/issues/51 --- js/ohms-law/model/OhmsLawModel.js | 1 - js/ohms-law/view/BatteriesView.js | 2 +- js/ohms-law/view/FormulaNode.js | 16 ++++++---------- js/ohms-law/view/OhmsLawScreenView.js | 5 +++-- js/ohms-law/view/ReadoutPanel.js | 2 +- js/ohms-law/view/ResistorNode.js | 2 +- js/ohms-law/view/SliderUnit.js | 3 +-- js/ohms-law/view/WireBox.js | 16 +++++++--------- 8 files changed, 20 insertions(+), 27 deletions(-) diff --git a/js/ohms-law/model/OhmsLawModel.js b/js/ohms-law/model/OhmsLawModel.js index 9b7ddc9..6b38dbf 100644 --- a/js/ohms-law/model/OhmsLawModel.js +++ b/js/ohms-law/model/OhmsLawModel.js @@ -44,7 +44,6 @@ define( function( require ) { reset: function() { this.voltageProperty.reset(); this.resistanceProperty.reset(); - this.soundActiveProperty.reset(); } } ); } ); diff --git a/js/ohms-law/view/BatteriesView.js b/js/ohms-law/view/BatteriesView.js index d2c58f3..bf7829a 100644 --- a/js/ohms-law/view/BatteriesView.js +++ b/js/ohms-law/view/BatteriesView.js @@ -39,7 +39,7 @@ define( function( require ) { batteries.push( battery ); } - // Present for the lifetime of the simulation + // Present for the lifetime of the simulation; no need to unlink. voltageProperty.link( function( voltage ) { batteries.forEach( function( battery, index ) { diff --git a/js/ohms-law/view/FormulaNode.js b/js/ohms-law/view/FormulaNode.js index 48884cb..2169879 100644 --- a/js/ohms-law/view/FormulaNode.js +++ b/js/ohms-law/view/FormulaNode.js @@ -30,13 +30,11 @@ define( function( require ) { /** - * @param {Property.} currentProperty - * @param {Property.} voltageProperty - * @param {Property.} resistanceProperty + * @param {OhmsLawModel} model * @param {Object} options * @constructor */ - function FormulaNode( currentProperty, voltageProperty, resistanceProperty, options ) { + function FormulaNode( model, options ) { Node.call( this ); @@ -47,7 +45,7 @@ define( function( require ) { scaleA: 0.2, scaleB: 0.84, x: 380, - property: currentProperty, + property: model.currentProperty, color: PhetColorScheme.RED_COLORBLIND, maxInitialWidth: 20 }, @@ -56,7 +54,7 @@ define( function( require ) { scaleA: 4.5, scaleB: 2, x: 150, - property: voltageProperty, + property: model.voltageProperty, color: OhmsLawConstants.BLUE_COLOR, maxInitialWidth: 180 }, @@ -65,7 +63,7 @@ define( function( require ) { scaleA: 0.04, scaleB: 2, x: 560, - property: resistanceProperty, + property: model.resistanceProperty, color: OhmsLawConstants.BLUE_COLOR, maxInitialWidth: 175 } @@ -113,14 +111,12 @@ define( function( require ) { var letterNode = new Node( { children: [ antiArtifactRectangle, textNode ] } ); lettersNode.addChild( letterNode ); - // Scale the text as the associated value changes + // Scale the text as the associated value changes. Present for the lifetime of the sim; no need to dispose. textData.property.link( function updateProperty( value ) { // Since it would potentially reduce the area of SVG that gets repainted (may be browser-specific) letterNode.matrix = Matrix3.translation( textData.x, CENTER_Y ) .timesMatrix( Matrix3.scale( textData.scaleA * value + textData.scaleB ) ); - - // TODO: Performance: consider not updating the matrix if it hasn't changed (if textData.x, textData.scaleA, and textData.scaleB haven't changed) } ); } ); diff --git a/js/ohms-law/view/OhmsLawScreenView.js b/js/ohms-law/view/OhmsLawScreenView.js index 09805a9..2c61bec 100644 --- a/js/ohms-law/view/OhmsLawScreenView.js +++ b/js/ohms-law/view/OhmsLawScreenView.js @@ -39,7 +39,7 @@ define( function( require ) { ScreenView.call( this ); // Node of ohm's law equation. Layout is hardwired, see FormulaNode. - var formulaNode = new FormulaNode( model.currentProperty, model.voltageProperty, model.resistanceProperty, { + var formulaNode = new FormulaNode( model, { pickable: false } ); @@ -48,7 +48,7 @@ define( function( require ) { // Circuit node with readout node - var wireBox = new WireBox( model.voltageProperty, model.resistanceProperty, model.currentProperty, { + var wireBox = new WireBox( model, { pickable: false, x: 70, // Layout of the WireBox y: 380 @@ -80,6 +80,7 @@ define( function( require ) { centerY: controlPanel.bottom + buttonCenterYOffset, listener: function() { model.reset(); + soundActiveProperty.reset(); } } ) ); diff --git a/js/ohms-law/view/ReadoutPanel.js b/js/ohms-law/view/ReadoutPanel.js index 2f0432f..db282f5 100644 --- a/js/ohms-law/view/ReadoutPanel.js +++ b/js/ohms-law/view/ReadoutPanel.js @@ -78,7 +78,7 @@ define( function( require ) { resize: false } ); - // Present for the lifetime of the simulation, no need to unlink + // Present for the lifetime of the simulation, no need to unlink. currentProperty.link( function( current ) { var rightEdgePosition = currentValue.right; currentValue.text = Util.toFixed( current, 1 ); diff --git a/js/ohms-law/view/ResistorNode.js b/js/ohms-law/view/ResistorNode.js index fd5626c..222f1c5 100644 --- a/js/ohms-law/view/ResistorNode.js +++ b/js/ohms-law/view/ResistorNode.js @@ -119,7 +119,7 @@ define( function( require ) { Math.PI / 2, true ); - // Set the number of visible dots based on the resistivity + // Set the number of visible dots based on the resistivity. Present for the lifetime of the simulation; no need to unlink. resistanceProperty.link( function( resistance ) { var numDotsToShow = RESISTANCE_TO_NUM_DOTS( resistance ); dotGroup.children.forEach( function( dot, index ) { diff --git a/js/ohms-law/view/SliderUnit.js b/js/ohms-law/view/SliderUnit.js index c61537b..596a458 100644 --- a/js/ohms-law/view/SliderUnit.js +++ b/js/ohms-law/view/SliderUnit.js @@ -98,8 +98,7 @@ define( function( require ) { this.addChild( readout ); this.addChild( unitText ); - // No need to unlink, present for the lifetime of the simulation - // update value of the readout + // Update value of the readout. Present for the lifetime of the simulation; no need to unlink. property.link( function( value ) { readout.text = Util.toFixed( value, options.numberDecimalPlaces ); readout.right = unitText.left - 10; diff --git a/js/ohms-law/view/WireBox.js b/js/ohms-law/view/WireBox.js index 0b14040..72dccbb 100644 --- a/js/ohms-law/view/WireBox.js +++ b/js/ohms-law/view/WireBox.js @@ -26,13 +26,11 @@ define( function( require ) { var OFFSET = 10; // position offset for the RightAngleArrow /** - * @param {Property.} voltageProperty - * @param {Property.} resistanceProperty - * @param {Property.} currentProperty + * @param {OhmsLawModel} model * @param {Object} options * @constructor */ - function WireBox( voltageProperty, resistanceProperty, currentProperty, options ) { + function WireBox( model, options ) { Node.call( this ); @@ -40,33 +38,33 @@ define( function( require ) { var wireFrame = new Rectangle( 0, 0, WIDTH, HEIGHT, 4, 4, { stroke: '#000', lineWidth: THICKNESS } ); this.addChild( wireFrame ); - var batteriesView = new BatteriesView( voltageProperty, { + var batteriesView = new BatteriesView( model.voltageProperty, { left: 30, // Slightly to the right of the wire centerY: 0 } ); this.addChild( batteriesView ); - var resistorNode = new ResistorNode( resistanceProperty, { + var resistorNode = new ResistorNode( model.resistanceProperty, { centerX: WIDTH / 2, centerY: HEIGHT } ); this.addChild( resistorNode ); - var bottomLeftArrow = new RightAngleArrow( currentProperty, { + var bottomLeftArrow = new RightAngleArrow( model.currentProperty, { x: -OFFSET, y: HEIGHT + OFFSET, rotation: Math.PI / 2 } ); this.addChild( bottomLeftArrow ); - var bottomRightArrow = new RightAngleArrow( currentProperty, { + var bottomRightArrow = new RightAngleArrow( model.currentProperty, { x: WIDTH + OFFSET, y: HEIGHT + OFFSET, rotation: 0 } ); this.addChild( bottomRightArrow ); - var currentReadoutPanel = new ReadoutPanel( currentProperty, { + var currentReadoutPanel = new ReadoutPanel( model.currentProperty, { centerY: HEIGHT / 2, centerX: WIDTH / 2 } );