phetsims / masses-and-springs

"Masses and Springs" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 5 forks source link

Class constructor OscillatingSpringNode cannot be invoked without 'new' #363

Open pixelzoom opened 4 years ago

pixelzoom commented 4 years ago

For https://github.com/phetsims/tasks/issues/1044, I converted scenery-phet's ParametricSpringNode to ES6 class. Here's the patch:

ParametricSpringNode ```diff Index: js/ParametricSpringNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/ParametricSpringNode.js (revision 04211f946a86b6ccbd85c1faa2fe7c9d66b03eb3) +++ js/ParametricSpringNode.js (date 1599777255956) @@ -24,7 +24,6 @@ import Vector2 from '../../dot/js/Vector2.js'; import Shape from '../../kite/js/Shape.js'; import InstanceRegistry from '../../phet-core/js/documentation/InstanceRegistry.js'; -import inherit from '../../phet-core/js/inherit.js'; import merge from '../../phet-core/js/merge.js'; import Circle from '../../scenery/js/nodes/Circle.js'; import Node from '../../scenery/js/nodes/Node.js'; @@ -36,287 +35,273 @@ // constants const SHOW_ORIGIN = false; // {boolean} draws a red circle at the origin, for layout debugging -/** - * @param {Object} [options] - * @constructor - */ -function ParametricSpringNode( options ) { +class ParametricSpringNode extends Node { + + /** + * @param {Object} [options] + */ + constructor( options ) { - options = merge( { + options = merge( { - // {Color|string} colors used for the gradient strokes. middleColor is the dominant color. - frontColor: 'lightGray', - middleColor: 'gray', - backColor: 'black', + // {Color|string} colors used for the gradient strokes. middleColor is the dominant color. + frontColor: 'lightGray', + middleColor: 'gray', + backColor: 'black', - // {number} length of the horizontal line added to the left end of the coil - leftEndLength: 15, + // {number} length of the horizontal line added to the left end of the coil + leftEndLength: 15, - // {number} length of the horizontal line added to the right end of the coil - rightEndLength: 25, + // {number} length of the horizontal line added to the right end of the coil + rightEndLength: 25, - // {number} number of loops in the coil - loops: 10, + // {number} number of loops in the coil + loops: 10, - // {number} number of points used to approximate 1 loop of the coil - pointsPerLoop: 40, + // {number} number of points used to approximate 1 loop of the coil + pointsPerLoop: 40, - // {number} radius of a loop with aspect ratio of 1:1 - radius: 10, + // {number} radius of a loop with aspect ratio of 1:1 + radius: 10, - // {number} y:x aspect ratio of the loop radius - aspectRatio: 4, + // {number} y:x aspect ratio of the loop radius + aspectRatio: 4, - // {number} lineWidth used to stroke the Paths - lineWidth: 3, + // {number} lineWidth used to stroke the Paths + lineWidth: 3, - // {number} phase angle of where the loop starts, period is (0,2*PI) radians, counterclockwise - phase: Math.PI, + // {number} phase angle of where the loop starts, period is (0,2*PI) radians, counterclockwise + phase: Math.PI, - // {number} responsible for the leaning of the coil, variation on a Lissjoue curve, period is (0,2*PI) radians - deltaPhase: Math.PI / 2, + // {number} responsible for the leaning of the coil, variation on a Lissjoue curve, period is (0,2*PI) radians + deltaPhase: Math.PI / 2, - // {number} multiplier for radius in the x dimensions, makes the coil appear to get longer - xScale: 2.5, + // {number} multiplier for radius in the x dimensions, makes the coil appear to get longer + xScale: 2.5, - // {string} method used to compute bounds for scenery.Path components, see Path.boundsMethod - pathBoundsMethod: 'accurate', + // {string} method used to compute bounds for scenery.Path components, see Path.boundsMethod + pathBoundsMethod: 'accurate', - // phet-io - tandem: Tandem.OPTIONAL - }, options ); + // phet-io + tandem: Tandem.OPTIONAL + }, options ); - const self = this; + super(); - // @public - this.loopsProperty = new NumberProperty( options.loops, { - tandem: options.tandem.createTandem( 'loopsProperty' ), - numberType: 'Integer', - range: new Range( 1, Number.POSITIVE_INFINITY ) - } ); - this.radiusProperty = new NumberProperty( options.radius, { - tandem: options.tandem.createTandem( 'radiusProperty' ), - range: new Range( 0, Number.POSITIVE_INFINITY ) - } ); - this.aspectRatioProperty = new NumberProperty( options.aspectRatio, { - tandem: options.tandem.createTandem( 'aspectRatioProperty' ), - range: new Range( 0, Number.POSITIVE_INFINITY ) - } ); - this.pointsPerLoopProperty = new NumberProperty( options.pointsPerLoop, { - tandem: options.tandem.createTandem( 'pointsPerLoopProperty' ), - numberType: 'Integer', - range: new Range( 0, Number.POSITIVE_INFINITY ) - } ); - this.lineWidthProperty = new NumberProperty( options.lineWidth, { - tandem: options.tandem.createTandem( 'lineWidthProperty' ), - range: new Range( 0, Number.POSITIVE_INFINITY ) - } ); - this.phaseProperty = new NumberProperty( options.phase, { - tandem: options.tandem.createTandem( 'phaseProperty' ), - range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) - } ); - this.deltaPhaseProperty = new NumberProperty( options.deltaPhase, { - tandem: options.tandem.createTandem( 'deltaPhaseProperty' ), - range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) - } ); - this.xScaleProperty = new NumberProperty( options.xScale, { - tandem: options.tandem.createTandem( 'xScaleProperty' ), - range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) - } ); + // @public + this.loopsProperty = new NumberProperty( options.loops, { + tandem: options.tandem.createTandem( 'loopsProperty' ), + numberType: 'Integer', + range: new Range( 1, Number.POSITIVE_INFINITY ) + } ); + this.radiusProperty = new NumberProperty( options.radius, { + tandem: options.tandem.createTandem( 'radiusProperty' ), + range: new Range( 0, Number.POSITIVE_INFINITY ) + } ); + this.aspectRatioProperty = new NumberProperty( options.aspectRatio, { + tandem: options.tandem.createTandem( 'aspectRatioProperty' ), + range: new Range( 0, Number.POSITIVE_INFINITY ) + } ); + this.pointsPerLoopProperty = new NumberProperty( options.pointsPerLoop, { + tandem: options.tandem.createTandem( 'pointsPerLoopProperty' ), + numberType: 'Integer', + range: new Range( 0, Number.POSITIVE_INFINITY ) + } ); + this.lineWidthProperty = new NumberProperty( options.lineWidth, { + tandem: options.tandem.createTandem( 'lineWidthProperty' ), + range: new Range( 0, Number.POSITIVE_INFINITY ) + } ); + this.phaseProperty = new NumberProperty( options.phase, { + tandem: options.tandem.createTandem( 'phaseProperty' ), + range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) + } ); + this.deltaPhaseProperty = new NumberProperty( options.deltaPhase, { + tandem: options.tandem.createTandem( 'deltaPhaseProperty' ), + range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) + } ); + this.xScaleProperty = new NumberProperty( options.xScale, { + tandem: options.tandem.createTandem( 'xScaleProperty' ), + range: new Range( Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY ) + } ); - // Paths for the front (foreground) and back (background) parts of the spring - const pathOptions = { - boundsMethod: options.pathBoundsMethod, - lineCap: 'round', - lineJoin: 'round' - }; - const frontPath = new Path( null, merge( { tandem: options.tandem.createTandem( 'frontPath' ) }, pathOptions ) ); - const backPath = new Path( null, merge( { tandem: options.tandem.createTandem( 'backPath' ) }, pathOptions ) ); + // Paths for the front (foreground) and back (background) parts of the spring + const pathOptions = { + boundsMethod: options.pathBoundsMethod, + lineCap: 'round', + lineJoin: 'round' + }; + const frontPath = new Path( null, merge( { tandem: options.tandem.createTandem( 'frontPath' ) }, pathOptions ) ); + const backPath = new Path( null, merge( { tandem: options.tandem.createTandem( 'backPath' ) }, pathOptions ) ); - // Update the line width - this.lineWidthProperty.link( function( lineWidth ) { - frontPath.lineWidth = backPath.lineWidth = lineWidth; - } ); + // Update the line width + this.lineWidthProperty.link( function( lineWidth ) { + frontPath.lineWidth = backPath.lineWidth = lineWidth; + } ); - // Mutate these to improve performance - const springPoints = []; // {Vector2[]} points in the spring (includes the horizontal ends) - let frontShape; // {Shape} - let backShape; // {Shape} + // Mutate these to improve performance + const springPoints = []; // {Vector2[]} points in the spring (includes the horizontal ends) + let frontShape; // {Shape} + let backShape; // {Shape} - // Changes to these properties require new points (Vector2) and Shapes, because they change - // the number of points and/or how the points are allocated to frontShape and backShape. - Property.multilink( [ - this.loopsProperty, this.pointsPerLoopProperty, - this.aspectRatioProperty, this.phaseProperty, this.deltaPhaseProperty - ], - function( loops, pointsPerLoop, aspectRatio, phase, deltaPhase ) { + // Changes to these Properties require new points (Vector2) and Shapes, because they change + // the number of points and/or how the points are allocated to frontShape and backShape. + Property.multilink( [ + this.loopsProperty, this.pointsPerLoopProperty, + this.aspectRatioProperty, this.phaseProperty, this.deltaPhaseProperty + ], + ( loops, pointsPerLoop, aspectRatio, phase, deltaPhase ) => { - // new points and Shapes - springPoints.length = 0; - frontShape = new Shape(); - backShape = new Shape(); + // new points and Shapes + springPoints.length = 0; + frontShape = new Shape(); + backShape = new Shape(); - // Values of other properties, to improve readability - const radius = self.radiusProperty.get(); - const xScale = self.xScaleProperty.get(); + // Values of other Properties, to improve readability + const radius = this.radiusProperty.get(); + const xScale = this.xScaleProperty.get(); - // compute the points for the coil - const coilPoints = []; // {Vector2[]} - const numberOfCoilPoints = computeNumberOfCoilPoints( loops, pointsPerLoop ); - let index; - for ( index = 0; index < numberOfCoilPoints; index++ ) { - const coilX = computeCoilX( index, radius, pointsPerLoop, phase, xScale, options.leftEndLength ); - const coilY = computeCoilY( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ); - coilPoints.push( new Vector2( coilX, coilY ) ); - } + // compute the points for the coil + const coilPoints = []; // {Vector2[]} + const numberOfCoilPoints = computeNumberOfCoilPoints( loops, pointsPerLoop ); + let index; + for ( index = 0; index < numberOfCoilPoints; index++ ) { + const coilX = computeCoilX( index, radius, pointsPerLoop, phase, xScale, options.leftEndLength ); + const coilY = computeCoilY( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ); + coilPoints.push( new Vector2( coilX, coilY ) ); + } - let p; // {Vector2} reusable point, hoisted explicitly - let wasFront = true; // was the previous point on the front path? + let p; // {Vector2} reusable point, hoisted explicitly + let wasFront = true; // was the previous point on the front path? - // Add points to Shapes - for ( index = 0; index < numberOfCoilPoints; index++ ) { + // Add points to Shapes + for ( index = 0; index < numberOfCoilPoints; index++ ) { - // is the current point on the front path? - const isFront = ( ( 2 * Math.PI * index / pointsPerLoop + phase + deltaPhase ) % ( 2 * Math.PI ) > Math.PI ); + // is the current point on the front path? + const isFront = ( ( 2 * Math.PI * index / pointsPerLoop + phase + deltaPhase ) % ( 2 * Math.PI ) > Math.PI ); - // horizontal line at left end - if ( index === 0 ) { - p = new Vector2( 0, coilPoints[ 0 ].y ); - springPoints.push( p ); - if ( isFront ) { - frontShape.moveToPoint( p ); - } - else { - backShape.moveToPoint( p ); - } - } + // horizontal line at left end + if ( index === 0 ) { + p = new Vector2( 0, coilPoints[ 0 ].y ); + springPoints.push( p ); + if ( isFront ) { + frontShape.moveToPoint( p ); + } + else { + backShape.moveToPoint( p ); + } + } - // coil point - springPoints.push( coilPoints[ index ] ); - if ( isFront ) { - // we're in the front - if ( !wasFront && index !== 0 ) { - // ... and we've just moved to the front - frontShape.moveToPoint( coilPoints[ index - 1 ] ); - } - frontShape.lineToPoint( coilPoints[ index ] ); - } - else { - // we're in the back - if ( wasFront && index !== 0 ) { - // ... and we've just moved to the back - backShape.moveToPoint( coilPoints[ index - 1 ] ); - } - backShape.lineToPoint( coilPoints[ index ] ); - } + // coil point + springPoints.push( coilPoints[ index ] ); + if ( isFront ) { + // we're in the front + if ( !wasFront && index !== 0 ) { + // ... and we've just moved to the front + frontShape.moveToPoint( coilPoints[ index - 1 ] ); + } + frontShape.lineToPoint( coilPoints[ index ] ); + } + else { + // we're in the back + if ( wasFront && index !== 0 ) { + // ... and we've just moved to the back + backShape.moveToPoint( coilPoints[ index - 1 ] ); + } + backShape.lineToPoint( coilPoints[ index ] ); + } - wasFront = isFront; - } + wasFront = isFront; + } - // horizontal line at right end - const lastCoilPoint = coilPoints[ numberOfCoilPoints - 1 ]; - p = new Vector2( lastCoilPoint.x + options.rightEndLength, lastCoilPoint.y ); - springPoints.push( p ); - if ( wasFront ) { - frontShape.lineToPoint( p ); - } - else { - backShape.lineToPoint( p ); - } - assert && assert( springPoints.length === coilPoints.length + 2, - 'missing some points, have ' + springPoints.length + ', expected ' + coilPoints.length + 2 ); // +2 for horizontal ends + // horizontal line at right end + const lastCoilPoint = coilPoints[ numberOfCoilPoints - 1 ]; + p = new Vector2( lastCoilPoint.x + options.rightEndLength, lastCoilPoint.y ); + springPoints.push( p ); + if ( wasFront ) { + frontShape.lineToPoint( p ); + } + else { + backShape.lineToPoint( p ); + } + assert && assert( springPoints.length === coilPoints.length + 2, + 'missing some points, have ' + springPoints.length + ', expected ' + coilPoints.length + 2 ); // +2 for horizontal ends - frontPath.shape = frontShape; - backPath.shape = backShape; - } ); + frontPath.shape = frontShape; + backPath.shape = backShape; + } ); - // Changes to these properties can be accomplished by mutating existing points (Vector2) and Shapes, - // because the number of points remains the same, as does their allocation to frontShape and backShape. - Property.lazyMultilink( [ this.radiusProperty, this.xScaleProperty ], - function( radius, xScale ) { + // Changes to these Properties can be accomplished by mutating existing points (Vector2) and Shapes, + // because the number of points remains the same, as does their allocation to frontShape and backShape. + Property.lazyMultilink( + [ this.radiusProperty, this.xScaleProperty ], + ( radius, xScale ) => { - // Values of other properties, to improve readability - const loops = self.loopsProperty.get(); - const pointsPerLoop = self.pointsPerLoopProperty.get(); - const aspectRatio = self.aspectRatioProperty.get(); - const phase = self.phaseProperty.get(); - const deltaPhase = self.deltaPhaseProperty.get(); + // Values of other Properties, to improve readability + const loops = this.loopsProperty.get(); + const pointsPerLoop = this.pointsPerLoopProperty.get(); + const aspectRatio = this.aspectRatioProperty.get(); + const phase = this.phaseProperty.get(); + const deltaPhase = this.deltaPhaseProperty.get(); - // number of points in the coil - const numberOfCoilPoints = computeNumberOfCoilPoints( loops, pointsPerLoop ); - assert && assert( numberOfCoilPoints === springPoints.length - 2, - 'unexpected number of coil points: ' + numberOfCoilPoints + ', expected ' + ( springPoints.length - 2 ) ); // -2 for horizontal ends + // number of points in the coil + const numberOfCoilPoints = computeNumberOfCoilPoints( loops, pointsPerLoop ); + assert && assert( numberOfCoilPoints === springPoints.length - 2, + 'unexpected number of coil points: ' + numberOfCoilPoints + ', expected ' + ( springPoints.length - 2 ) ); // -2 for horizontal ends - // mutate the coil points - for ( let index = 0; index < numberOfCoilPoints; index++ ) { - const coilX = computeCoilX( index, radius, pointsPerLoop, phase, xScale, options.leftEndLength ); - const coilY = computeCoilY( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ); - springPoints[ index + 1 ].setXY( coilX, coilY ); - } + // mutate the coil points + for ( let index = 0; index < numberOfCoilPoints; index++ ) { + const coilX = computeCoilX( index, radius, pointsPerLoop, phase, xScale, options.leftEndLength ); + const coilY = computeCoilY( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ); + springPoints[ index + 1 ].setXY( coilX, coilY ); + } - // mutate horizontal line at left end - const firstCoilPoint = springPoints[ 1 ]; - springPoints[ 0 ].setXY( 0, firstCoilPoint.y ); + // mutate horizontal line at left end + const firstCoilPoint = springPoints[ 1 ]; + springPoints[ 0 ].setXY( 0, firstCoilPoint.y ); - // mutate horizontal line at right end - const lastCoilPoint = springPoints[ springPoints.length - 2 ]; - springPoints[ springPoints.length - 1 ].setXY( lastCoilPoint.x + options.rightEndLength, lastCoilPoint.y ); + // mutate horizontal line at right end + const lastCoilPoint = springPoints[ springPoints.length - 2 ]; + springPoints[ springPoints.length - 1 ].setXY( lastCoilPoint.x + options.rightEndLength, lastCoilPoint.y ); - // Tell shapes that their points have changed. - frontShape.invalidatePoints(); - backShape.invalidatePoints(); - } ); + // Tell shapes that their points have changed. + frontShape.invalidatePoints(); + backShape.invalidatePoints(); + } ); - // Update the stroke gradients - Property.multilink( [ this.radiusProperty, this.aspectRatioProperty ], - function( radius, aspectRatio ) { + // Update the stroke gradients + Property.multilink( + [ this.radiusProperty, this.aspectRatioProperty ], + ( radius, aspectRatio ) => { - const yRadius = radius * aspectRatio; + const yRadius = radius * aspectRatio; - frontPath.stroke = new LinearGradient( 0, -yRadius, 0, yRadius ) - .addColorStop( 0, options.middleColor ) - .addColorStop( 0.35, options.frontColor ) - .addColorStop( 0.65, options.frontColor ) - .addColorStop( 1, options.middleColor ); + frontPath.stroke = new LinearGradient( 0, -yRadius, 0, yRadius ) + .addColorStop( 0, options.middleColor ) + .addColorStop( 0.35, options.frontColor ) + .addColorStop( 0.65, options.frontColor ) + .addColorStop( 1, options.middleColor ); - backPath.stroke = new LinearGradient( 0, -yRadius, 0, yRadius ) - .addColorStop( 0, options.middleColor ) - .addColorStop( 0.5, options.backColor ) - .addColorStop( 1, options.middleColor ); - } ); + backPath.stroke = new LinearGradient( 0, -yRadius, 0, yRadius ) + .addColorStop( 0, options.middleColor ) + .addColorStop( 0.5, options.backColor ) + .addColorStop( 1, options.middleColor ); + } ); - options.children = [ backPath, frontPath ]; - Node.call( this, options ); + assert && assert( !options.children, 'ParametricSpringNode sets children' ); + options.children = [ backPath, frontPath ]; + + this.mutate( options ); - if ( SHOW_ORIGIN ) { - this.addChild( new Circle( 3, { fill: 'red' } ) ); - } + if ( SHOW_ORIGIN ) { + this.addChild( new Circle( 3, { fill: 'red' } ) ); + } - // support for binder documentation, stripped out in builds and only runs when ?binder is specified - assert && phet.chipper.queryParameters.binder && InstanceRegistry.registerDataURL( 'scenery-phet', 'ParametricSpringNode', this ); -} + // support for binder documentation, stripped out in builds and only runs when ?binder is specified + assert && phet.chipper.queryParameters.binder && InstanceRegistry.registerDataURL( 'scenery-phet', 'ParametricSpringNode', this ); + } -sceneryPhet.register( 'ParametricSpringNode', ParametricSpringNode ); - -// Gets the number of points in the coil part of the spring. -var computeNumberOfCoilPoints = function( loops, pointsPerLoop ) { - return loops * pointsPerLoop + 1; -}; - -// Computes the x coordinate for a point on the coil. -var computeCoilX = function( index, radius, pointsPerLoop, phase, xScale, leftEndLength ) { - return ( leftEndLength + radius ) + radius * Math.cos( 2 * Math.PI * index / pointsPerLoop + phase ) + xScale * ( index / pointsPerLoop ) * radius; -}; - -// Computes the y coordinate for a point on the coil. -var computeCoilY = function( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ) { - return aspectRatio * radius * Math.cos( 2 * Math.PI * index / pointsPerLoop + deltaPhase + phase ); -}; - -inherit( Node, ParametricSpringNode, { - // @public - reset: function() { + reset() { this.loopsProperty.reset(); this.radiusProperty.reset(); this.aspectRatioProperty.reset(); @@ -326,6 +311,45 @@ this.deltaPhaseProperty.reset(); this.xScaleProperty.reset(); } -} ); +} + +/** + * Gets the number of points in the coil part of the spring. + * @param {number} loops + * @param {number} pointsPerLoop + * @returns {number} + */ +function computeNumberOfCoilPoints( loops, pointsPerLoop ) { + return loops * pointsPerLoop + 1; +} +/** + * Computes the x coordinate for a point on the coil. + * @param {number} index + * @param {number} radius + * @param {number} pointsPerLoop + * @param {number} phase + * @param {number} xScale + * @param {number} leftEndLength + * @returns {number} + */ +function computeCoilX( index, radius, pointsPerLoop, phase, xScale, leftEndLength ) { + return ( leftEndLength + radius ) + radius * Math.cos( 2 * Math.PI * index / pointsPerLoop + phase ) + xScale * ( index / pointsPerLoop ) * radius; +} + +/** + * Computes the y coordinate for a point on the coil. + * @param {number} index + * @param {number} radius + * @param {number} pointsPerLoop + * @param {number} phase + * @param {number} deltaPhase + * @param {number} aspectRatio + * @returns {number} + */ +function computeCoilY( index, radius, pointsPerLoop, phase, deltaPhase, aspectRatio ) { + return aspectRatio * radius * Math.cos( 2 * Math.PI * index / pointsPerLoop + deltaPhase + phase ); +} + +sceneryPhet.register( 'ParametricSpringNode', ParametricSpringNode ); export default ParametricSpringNode; \ No newline at end of file ```

I also converted masses-and-springs OscillatingSpringNode to ES6 class. It's currently the only subclass of OscillatingSpringNode. Here's the patch:

OscillatingSpringNode patch ```diff Index: js/common/view/OscillatingSpringNode.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- js/common/view/OscillatingSpringNode.js (revision 248c196bea6bcfddf2de92bd8fcdce2504434899) +++ js/common/view/OscillatingSpringNode.js (date 1599777255961) @@ -10,97 +10,87 @@ import LinearFunction from '../../../../dot/js/LinearFunction.js'; import Utils from '../../../../dot/js/Utils.js'; import Vector2 from '../../../../dot/js/Vector2.js'; -import inherit from '../../../../phet-core/js/inherit.js'; import merge from '../../../../phet-core/js/merge.js'; import ParametricSpringNode from '../../../../scenery-phet/js/ParametricSpringNode.js'; import massesAndSprings from '../../massesAndSprings.js'; // constants const LINEAR_LOOP_MAPPING = new LinearFunction( 0.1, 0.5, 2, 12 ); -const MAP_NUMBER_OF_LOOPS = function( springLength ) { - return Utils.roundSymmetric( LINEAR_LOOP_MAPPING( springLength ) ); -}; +const MAP_NUMBER_OF_LOOPS = springLength => Utils.roundSymmetric( LINEAR_LOOP_MAPPING( springLength ) ); + +class OscillatingSpringNode extends ParametricSpringNode { -/** - * @param {Spring} spring - * @param {ModelViewTransform2} modelViewTransform2 - * @param {Tandem} tandem - * @param {Object} [options] - * @constructor - */ -function OscillatingSpringNode( spring, modelViewTransform2, tandem, options ) { - const self = this; + /** + * @param {Spring} spring + * @param {ModelViewTransform2} modelViewTransform2 + * @param {Tandem} tandem + * @param {Object} [options] + */ + constructor( spring, modelViewTransform2, tandem, options ) { - options = merge( { - deltaPhase: 3 * Math.PI / 2, - loops: MAP_NUMBER_OF_LOOPS( spring.lengthProperty.get() ), // {number} number of loops in the coil - pointsPerLoop: 28, // {number} number of points per loop - radius: 6.5, // {number} radius of a loop with aspect ratio of 1:1 - aspectRatio: 4, // {number} y:x aspect ratio of the loop radius - unitDisplacementLength: modelViewTransform2.viewToModelDeltaY( 1 ), // {number} view length of 1 meter of displacement - minLineWidth: 1, // {number} lineWidth used to stroke the spring for minimum spring constant - deltaLineWidth: 1.5, // increase in line width per 1 unit of spring constant increase - leftEndLength: -15, // {number} length of the horizontal line added to the left end of the coil - rightEndLength: -15, // {number} length of the horizontal line added to the right end of the coil - rotation: Math.PI / 2, // {number} angle in radians of rotation of spring, - pathBoundsMethod: 'safePadding', - tandem: tandem - }, options ); + options = merge( { + deltaPhase: 3 * Math.PI / 2, + loops: MAP_NUMBER_OF_LOOPS( spring.lengthProperty.get() ), // {number} number of loops in the coil + pointsPerLoop: 28, // {number} number of points per loop + radius: 6.5, // {number} radius of a loop with aspect ratio of 1:1 + aspectRatio: 4, // {number} y:x aspect ratio of the loop radius + unitDisplacementLength: modelViewTransform2.viewToModelDeltaY( 1 ), // {number} view length of 1 meter of displacement + minLineWidth: 1, // {number} lineWidth used to stroke the spring for minimum spring constant + deltaLineWidth: 1.5, // increase in line width per 1 unit of spring constant increase + leftEndLength: -15, // {number} length of the horizontal line added to the left end of the coil + rightEndLength: -15, // {number} length of the horizontal line added to the right end of the coil + rotation: Math.PI / 2, // {number} angle in radians of rotation of spring, + pathBoundsMethod: 'safePadding', + tandem: tandem + }, options ); - ParametricSpringNode.call( this, options ); + super( options ); - // @public {Spring} (read-only) - this.spring = spring; + // @public {Spring} (read-only) + this.spring = spring; - this.translation = modelViewTransform2.modelToViewPosition( - new Vector2( spring.positionProperty.get().x, - spring.positionProperty.get().y - length ) ); + this.translation = modelViewTransform2.modelToViewPosition( + new Vector2( spring.positionProperty.get().x, spring.positionProperty.get().y - length ) ); - function updateViewLength() { + const updateViewLength = () => { - // ParametricSpringNode calculations - // Value of coilStretch is in view coordinates and doesn't have model units. - const coilStretch = ( - modelViewTransform2.modelToViewDeltaY( spring.lengthProperty.get() ) - - ( options.leftEndLength + options.rightEndLength ) ); - const xScale = coilStretch / ( self.loopsProperty.get() * self.radiusProperty.get() ); + // ParametricSpringNode calculations + // Value of coilStretch is in view coordinates and doesn't have model units. + const coilStretch = ( + modelViewTransform2.modelToViewDeltaY( spring.lengthProperty.get() ) + - ( options.leftEndLength + options.rightEndLength ) ); + const xScale = coilStretch / ( this.loopsProperty.get() * this.radiusProperty.get() ); - // The wrong side of the PSN is static, so we have to put the spring in reverse and update the length AND position. - // Spring is rotated to be rotated so XScale relates to Y-direction in view - self.xScaleProperty.set( xScale ); - self.y = modelViewTransform2.modelToViewY( spring.positionProperty.get().y - spring.lengthProperty.get() ); - } + // The wrong side of the PSN is static, so we have to put the spring in reverse and update the length AND position. + // Spring is rotated to be rotated so XScale relates to Y-direction in view + this.xScaleProperty.set( xScale ); + this.y = modelViewTransform2.modelToViewY( spring.positionProperty.get().y - spring.lengthProperty.get() ); + } - // Link exists for sim duration. No need to unlink. - spring.naturalRestingLengthProperty.link( function( springLength ) { - self.loopsProperty.set( MAP_NUMBER_OF_LOOPS( springLength ) ); - updateViewLength(); - } ); + // Link exists for sim duration. No need to unlink. + spring.naturalRestingLengthProperty.link( springLength => { + this.loopsProperty.set( MAP_NUMBER_OF_LOOPS( springLength ) ); + updateViewLength(); + } ); - // Link exists for sim duration. No need to unlink. - spring.lengthProperty.link( function() { - updateViewLength(); - } ); + // Link exists for sim duration. No need to unlink. + spring.lengthProperty.link( () => updateViewLength() ); - // ParametricSpringNode width update. SpringConstant determines lineWidth - // Link exists for sim duration. No need to unlink. - spring.thicknessProperty.link( function( thickness ) { - self.lineWidthProperty.set( thickness ); - } ); -} + // ParametricSpringNode width update. SpringConstant determines lineWidth + // Link exists for sim duration. No need to unlink. + spring.thicknessProperty.link( thickness => this.lineWidthProperty.set( thickness ) ); + } -massesAndSprings.register( 'OscillatingSpringNode', OscillatingSpringNode ); - -inherit( ParametricSpringNode, OscillatingSpringNode, { /** * @public */ - reset: function() { - ParametricSpringNode.prototype.reset.call( this ); + reset() { + super.reset(); this.spring.reset(); } -}, { - MAP_NUMBER_OF_LOOPS: MAP_NUMBER_OF_LOOPS -} ); +} +OscillatingSpringNode.MAP_NUMBER_OF_LOOPS = MAP_NUMBER_OF_LOOPS; + +massesAndSprings.register( 'OscillatingSpringNode', OscillatingSpringNode ); export default OscillatingSpringNode; \ No newline at end of file ```

All of this looked straightforward, and ParametricSpringNode works great in hookes-law. But when I run masses-and-springs, it fails at startup in MutableOptionsNode

MutableOptionsNode.js:65 Uncaught TypeError: Class constructor OscillatingSpringNode cannot be invoked without 'new'
    at new MutableOptionsNodeConstructor (MutableOptionsNode.js:65)
    at MutableOptionsNode.replaceCopy (MutableOptionsNode.js:87)
    at new Multilink (Multilink.js:61)
    at Function.multilink (Property.js:522)
    at new MutableOptionsNode (MutableOptionsNode.js:72)
    at SpringScreenView.js:85
    at Array.map (<anonymous>)
    at IntroScreenView.SpringScreenView [as constructor] (SpringScreenView.js:84)
    at IntroScreenView.TwoSpringScreenView [as constructor] (TwoSpringScreenView.js:33)
    at new IntroScreenView (IntroScreenView.js:50)

It looks to me like MutableOptionsNode might not be compatible with class. If that's the case, then this bit of masses-and-springs needs to be rewritten, and MutableOptionsNode should be deprecated since all new PhET code must use class.

@Denz1994 can you take the lead on this?

Also assigning @jonathanolson to comment on the compatibility of MutableOptionsNode with class.

jonathanolson commented 4 years ago

I'd like to try getting MutableOptionsNode working with class, it looks like we now have the browser support requirements to do so.

samreid commented 4 years ago

It's likely that the commit in https://github.com/phetsims/sun/issues/627 will help here. Is the next step for this issue for @pixelzoom to reapply the patches and see if the fix helped?

pixelzoom commented 4 years ago

Sorry, I can't be involved in this issue. I just ran into it while working on phetsims/tasks#1044. Up to @Denz1994 and @jonathanolson to decide how to proceed.