From a2e9a96b8a85608cf444d6da7d9176f30eb64a1b Mon Sep 17 00:00:00 2001 From: Jonathan Olson Date: Wed, 30 Jan 2019 13:11:24 -0700 Subject: [PATCH 1/2] A few more REVIEW notes, see https://github.com/phetsims/masses-and-springs-basics/issues/48 --- js/common/model/Spring.js | 5 +++-- js/common/view/EnergyGraphNode.js | 3 +++ js/common/view/GravityAndDampingControlNode.js | 2 ++ js/common/view/MassNode.js | 2 ++ js/common/view/OscillatingSpringNode.js | 1 + js/intro/view/ConstantsControlPanel.js | 1 + 6 files changed, 12 insertions(+), 2 deletions(-) diff --git a/js/common/model/Spring.js b/js/common/model/Spring.js index ae9e4b28..4a533e41 100644 --- a/js/common/model/Spring.js +++ b/js/common/model/Spring.js @@ -59,6 +59,8 @@ define( function( require ) { } ); // Link to manage gravity value for the spring object. + // REVIEW: This would be a leak if springs keep sharing one gravity property (and springs could be created). + // REVIEW: Probably a good place to mention object lifetimes. gravityProperty.link( function( gravity ) { self.gravityProperty.set( gravity ); } ); @@ -620,5 +622,4 @@ define( function( require ) { } } } ); -} ) -; \ No newline at end of file +} ); \ No newline at end of file diff --git a/js/common/view/EnergyGraphNode.js b/js/common/view/EnergyGraphNode.js index 938557e2..42d21245 100644 --- a/js/common/view/EnergyGraphNode.js +++ b/js/common/view/EnergyGraphNode.js @@ -111,6 +111,9 @@ define( function( require ) { }, scale: 0.7 } ); + + // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are + // REVIEW: created. model.firstSpring.thermalEnergyProperty.link( function( value ) { clearThermalButton.enabled = ( value > 0 ); clearThermalButton.pickable = ( value > 0 ); diff --git a/js/common/view/GravityAndDampingControlNode.js b/js/common/view/GravityAndDampingControlNode.js index 66acf135..d2c4fa58 100644 --- a/js/common/view/GravityAndDampingControlNode.js +++ b/js/common/view/GravityAndDampingControlNode.js @@ -254,6 +254,8 @@ define( function( require ) { } // Responsible for managing bodies + // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are + // REVIEW: created. model.bodyProperty.link( function( newBody, oldBody ) { var body = _.find( Body.BODIES, newBody ); diff --git a/js/common/view/MassNode.js b/js/common/view/MassNode.js index 92411285..8809f266 100644 --- a/js/common/view/MassNode.js +++ b/js/common/view/MassNode.js @@ -80,6 +80,8 @@ define( function( require ) { } ); // Update the size of the massNode + // REVIEW: Probably good to mention the lifetime of this, since this would be a memory leak if many of these are + // REVIEW: created for a single mass mass.radiusProperty.link( function( radiusValue ) { self.rect.rectBounds = new Bounds2( diff --git a/js/common/view/OscillatingSpringNode.js b/js/common/view/OscillatingSpringNode.js index 1ef05ff3..9a6bbc9d 100644 --- a/js/common/view/OscillatingSpringNode.js +++ b/js/common/view/OscillatingSpringNode.js @@ -73,6 +73,7 @@ define( function( require ) { self.y = modelViewTransform2.modelToViewY( spring.positionProperty.get().y - spring.lengthProperty.get() ); } + // REVIEW: Probably worth noting that this type creates permanent listeners. spring.naturalRestingLengthProperty.link( function( springLength ) { self.loopsProperty.set( MAP_NUMBER_OF_LOOPS( springLength ) ); updateViewLength(); diff --git a/js/intro/view/ConstantsControlPanel.js b/js/intro/view/ConstantsControlPanel.js index d9c57365..8d9359dd 100644 --- a/js/intro/view/ConstantsControlPanel.js +++ b/js/intro/view/ConstantsControlPanel.js @@ -59,6 +59,7 @@ define( function( require ) { } ); var constantText = new Text( + // REVIEW: Why the fill in with an empty string? Looks suspicious StringUtils.fillIn( springConstantString, { spring: '' } ), _.extend( { font: TITLE_FONT, tandem: tandem.createTandem( 'constantText' ) }, constantsSelectionButtonOptions ) ); From 5fd6d9b80e7651daf393a8243bb6bffc0cabe738 Mon Sep 17 00:00:00 2001 From: Jonathan Olson Date: Wed, 30 Jan 2019 13:12:37 -0700 Subject: [PATCH 2/2] Updating copyright dates, see https://github.com/phetsims/masses-and-springs-basics/issues/48 --- js/common/enum/ForcesModeChoice.js | 2 +- js/common/enum/SimSpeedChoice.js | 2 +- js/common/model/MassesAndSpringsModel.js | 2 +- js/common/model/Spring.js | 2 +- js/common/view/DraggableRulerNode.js | 2 +- js/common/view/DraggableTimerNode.js | 2 +- js/common/view/EnergyGraphNode.js | 2 +- js/common/view/ForceVectorArrow.js | 2 +- js/common/view/GravityAndDampingControlNode.js | 2 +- js/common/view/GravityComboBox.js | 2 +- js/common/view/MassNode.js | 2 +- js/common/view/MassValueControlPanel.js | 2 +- js/common/view/MovableLineNode.js | 2 +- js/common/view/OneSpringScreenView.js | 2 +- js/common/view/OscillatingSpringNode.js | 2 +- js/common/view/ReferenceLineNode.js | 2 +- js/common/view/Shelf.js | 2 +- js/common/view/SpringControlPanel.js | 2 +- js/common/view/SpringHangerNode.js | 2 +- js/common/view/SpringScreenView.js | 2 +- js/common/view/ToolboxPanel.js | 2 +- js/common/view/VectorArrow.js | 2 +- js/energy/model/EnergyModel.js | 2 +- js/intro/enum/SceneModeChoice.js | 2 +- js/intro/model/IntroModel.js | 2 +- js/intro/view/ConstantsControlPanel.js | 2 +- js/intro/view/IntroScreenView.js | 2 +- js/lab/model/LabModel.js | 2 +- js/lab/view/LabScreenView.js | 2 +- js/lab/view/PeriodTraceNode.js | 2 +- js/vectors/view/IndicatorVisibilityControlNode.js | 2 +- js/vectors/view/VectorVisibilityControlNode.js | 2 +- 32 files changed, 32 insertions(+), 32 deletions(-) diff --git a/js/common/enum/ForcesModeChoice.js b/js/common/enum/ForcesModeChoice.js index f341ea32..6b36d0e0 100644 --- a/js/common/enum/ForcesModeChoice.js +++ b/js/common/enum/ForcesModeChoice.js @@ -1,4 +1,4 @@ -// Copyright 2018, University of Colorado Boulder +// Copyright 2018-2019, University of Colorado Boulder /** * Enumeration for sim speed choices. diff --git a/js/common/enum/SimSpeedChoice.js b/js/common/enum/SimSpeedChoice.js index 0b487fcf..7a654826 100644 --- a/js/common/enum/SimSpeedChoice.js +++ b/js/common/enum/SimSpeedChoice.js @@ -1,4 +1,4 @@ -// Copyright 2018, University of Colorado Boulder +// Copyright 2018-2019, University of Colorado Boulder /** * Enumeration for sim speed choices. diff --git a/js/common/model/MassesAndSpringsModel.js b/js/common/model/MassesAndSpringsModel.js index b0cbc5c0..b602e368 100644 --- a/js/common/model/MassesAndSpringsModel.js +++ b/js/common/model/MassesAndSpringsModel.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Common model (base type) for Masses and Springs diff --git a/js/common/model/Spring.js b/js/common/model/Spring.js index 4a533e41..b20ab1bd 100644 --- a/js/common/model/Spring.js +++ b/js/common/model/Spring.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Responsible for the attributes associated with each spring. diff --git a/js/common/view/DraggableRulerNode.js b/js/common/view/DraggableRulerNode.js index 83866610..18782ef2 100644 --- a/js/common/view/DraggableRulerNode.js +++ b/js/common/view/DraggableRulerNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the attributes and drag handlers associated with the ruler node. diff --git a/js/common/view/DraggableTimerNode.js b/js/common/view/DraggableTimerNode.js index 2484b732..addf1c40 100644 --- a/js/common/view/DraggableTimerNode.js +++ b/js/common/view/DraggableTimerNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the attributes and drag handlers associated with the timer node. diff --git a/js/common/view/EnergyGraphNode.js b/js/common/view/EnergyGraphNode.js index 42d21245..54fe8d39 100644 --- a/js/common/view/EnergyGraphNode.js +++ b/js/common/view/EnergyGraphNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Bar graph that represents the kinetic, potential, elastic potential, thermal, and total energy of the mass attached diff --git a/js/common/view/ForceVectorArrow.js b/js/common/view/ForceVectorArrow.js index b1e05476..9597cd9d 100644 --- a/js/common/view/ForceVectorArrow.js +++ b/js/common/view/ForceVectorArrow.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the creation of the arrowNodes associated with the masses and visibility panels. diff --git a/js/common/view/GravityAndDampingControlNode.js b/js/common/view/GravityAndDampingControlNode.js index d2c4fa58..a088eb91 100644 --- a/js/common/view/GravityAndDampingControlNode.js +++ b/js/common/view/GravityAndDampingControlNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Node for the gravity control panel and combo box for planet gravity options. diff --git a/js/common/view/GravityComboBox.js b/js/common/view/GravityComboBox.js index 6bb7115b..0ee011f3 100644 --- a/js/common/view/GravityComboBox.js +++ b/js/common/view/GravityComboBox.js @@ -1,4 +1,4 @@ -// Copyright 2017-2019, University of Colorado Boulder +// Copyright 2019, University of Colorado Boulder /** * ComboBox used for selecting planets. diff --git a/js/common/view/MassNode.js b/js/common/view/MassNode.js index 8809f266..7925b51a 100644 --- a/js/common/view/MassNode.js +++ b/js/common/view/MassNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Node responsible for representing the mass object. diff --git a/js/common/view/MassValueControlPanel.js b/js/common/view/MassValueControlPanel.js index bd51af3f..910a53c4 100644 --- a/js/common/view/MassValueControlPanel.js +++ b/js/common/view/MassValueControlPanel.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Panel that is responsible for adjusting the value of its corresponding mass. diff --git a/js/common/view/MovableLineNode.js b/js/common/view/MovableLineNode.js index 424f6631..954bfdbf 100644 --- a/js/common/view/MovableLineNode.js +++ b/js/common/view/MovableLineNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Node for the reference line and laser pointer node. diff --git a/js/common/view/OneSpringScreenView.js b/js/common/view/OneSpringScreenView.js index 3f45429a..5894eac0 100644 --- a/js/common/view/OneSpringScreenView.js +++ b/js/common/view/OneSpringScreenView.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Common ScreenView for using one mass. diff --git a/js/common/view/OscillatingSpringNode.js b/js/common/view/OscillatingSpringNode.js index 9a6bbc9d..809c16fe 100644 --- a/js/common/view/OscillatingSpringNode.js +++ b/js/common/view/OscillatingSpringNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Node for handling the representation of an oscillating spring. diff --git a/js/common/view/ReferenceLineNode.js b/js/common/view/ReferenceLineNode.js index c46426e8..de698d51 100644 --- a/js/common/view/ReferenceLineNode.js +++ b/js/common/view/ReferenceLineNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the attributes associated with the reference line nodes. diff --git a/js/common/view/Shelf.js b/js/common/view/Shelf.js index 26447dba..55949c70 100644 --- a/js/common/view/Shelf.js +++ b/js/common/view/Shelf.js @@ -1,4 +1,4 @@ -// Copyright 2017, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Shelf used to house the masses when they are not being dragged or attached to a spring diff --git a/js/common/view/SpringControlPanel.js b/js/common/view/SpringControlPanel.js index 19852fba..e80f1591 100644 --- a/js/common/view/SpringControlPanel.js +++ b/js/common/view/SpringControlPanel.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Panel responsible for adjusting the a Property of the spring using an hslider. diff --git a/js/common/view/SpringHangerNode.js b/js/common/view/SpringHangerNode.js index 20c76fb4..d8356a1c 100644 --- a/js/common/view/SpringHangerNode.js +++ b/js/common/view/SpringHangerNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Object that creates the grey bar that the springs hang from and their respective numerical label. diff --git a/js/common/view/SpringScreenView.js b/js/common/view/SpringScreenView.js index 0ac4cca7..19075448 100644 --- a/js/common/view/SpringScreenView.js +++ b/js/common/view/SpringScreenView.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Common ScreenView used for both singular and multispring screen view. diff --git a/js/common/view/ToolboxPanel.js b/js/common/view/ToolboxPanel.js index 788a3715..f1e870d6 100644 --- a/js/common/view/ToolboxPanel.js +++ b/js/common/view/ToolboxPanel.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the toolbox panel and the ruler/timer icons held within diff --git a/js/common/view/VectorArrow.js b/js/common/view/VectorArrow.js index 9fad8d91..1ad96592 100644 --- a/js/common/view/VectorArrow.js +++ b/js/common/view/VectorArrow.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Responsible for the creation of the arrowNodes associated with the masses and visibility panels. diff --git a/js/energy/model/EnergyModel.js b/js/energy/model/EnergyModel.js index f488f85d..fc865357 100644 --- a/js/energy/model/EnergyModel.js +++ b/js/energy/model/EnergyModel.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Energy model (base type) for Masses and Springs diff --git a/js/intro/enum/SceneModeChoice.js b/js/intro/enum/SceneModeChoice.js index 5cabf822..10c1743a 100644 --- a/js/intro/enum/SceneModeChoice.js +++ b/js/intro/enum/SceneModeChoice.js @@ -1,4 +1,4 @@ -// Copyright 2018, University of Colorado Boulder +// Copyright 2018-2019, University of Colorado Boulder /** * Enumeration for scene mode choices intro screen. diff --git a/js/intro/model/IntroModel.js b/js/intro/model/IntroModel.js index beba69b7..b086f0d1 100644 --- a/js/intro/model/IntroModel.js +++ b/js/intro/model/IntroModel.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Intro model (base type) for Masses and Springs diff --git a/js/intro/view/ConstantsControlPanel.js b/js/intro/view/ConstantsControlPanel.js index 8d9359dd..0e4c853d 100644 --- a/js/intro/view/ConstantsControlPanel.js +++ b/js/intro/view/ConstantsControlPanel.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Panel responsible for keeping the spring constant and spring thickness constant. diff --git a/js/intro/view/IntroScreenView.js b/js/intro/view/IntroScreenView.js index 8faa8ac4..eda54e28 100644 --- a/js/intro/view/IntroScreenView.js +++ b/js/intro/view/IntroScreenView.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * ScreenView for the 'Intro' screen diff --git a/js/lab/model/LabModel.js b/js/lab/model/LabModel.js index fe8b7a9b..4567d46b 100644 --- a/js/lab/model/LabModel.js +++ b/js/lab/model/LabModel.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Lab model (base type) for Masses and Springs diff --git a/js/lab/view/LabScreenView.js b/js/lab/view/LabScreenView.js index f449b3ba..92ec85ec 100644 --- a/js/lab/view/LabScreenView.js +++ b/js/lab/view/LabScreenView.js @@ -1,4 +1,4 @@ -// Copyright 2016-2018, University of Colorado Boulder +// Copyright 2016-2019, University of Colorado Boulder /** * Screen view used for lab screen. diff --git a/js/lab/view/PeriodTraceNode.js b/js/lab/view/PeriodTraceNode.js index 88e7ed26..29b1b670 100644 --- a/js/lab/view/PeriodTraceNode.js +++ b/js/lab/view/PeriodTraceNode.js @@ -1,4 +1,4 @@ -// Copyright 2018, University of Colorado Boulder +// Copyright 2018-2019, University of Colorado Boulder /** * View for period trace of mass oscillation. diff --git a/js/vectors/view/IndicatorVisibilityControlNode.js b/js/vectors/view/IndicatorVisibilityControlNode.js index 70450ec7..526a5595 100644 --- a/js/vectors/view/IndicatorVisibilityControlNode.js +++ b/js/vectors/view/IndicatorVisibilityControlNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Panel that manages options for visibility of reference lines on vector screen. diff --git a/js/vectors/view/VectorVisibilityControlNode.js b/js/vectors/view/VectorVisibilityControlNode.js index 8336a438..ce1976af 100644 --- a/js/vectors/view/VectorVisibilityControlNode.js +++ b/js/vectors/view/VectorVisibilityControlNode.js @@ -1,4 +1,4 @@ -// Copyright 2017-2018, University of Colorado Boulder +// Copyright 2017-2019, University of Colorado Boulder /** * Panel that manages options visibility for vectors and period trace in Basics version.