Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
pixelzoom opened this issue Sep 10, 2020 · 3 comments
Open
Assignees

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 10, 2020

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

ParametricSpringNode
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
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
Copy link
Contributor

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

@samreid
Copy link
Member

samreid commented Sep 20, 2020

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

@pixelzoom
Copy link
Contributor Author

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.

@pixelzoom pixelzoom removed their assignment Sep 20, 2020
@jonathanolson jonathanolson assigned chrisklus and unassigned Denz1994 Jan 28, 2021
@chrisklus chrisklus removed their assignment May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants