Documentation is backwards for Electron.switchCurves

[samreid]

During code review #103, we observed this code in Electron.ts

  private switchCurves( coilSegmentPosition: number, recursionDepth = 0 ): void {
    assert && assert( recursionDepth < this.coilSegments.length, `infinite loop? recursionDepth=${recursionDepth}` );

    const oldPathSpeedScale = this.getSpeedScale();

    if ( coilSegmentPosition <= 0 ) {

      // We've passed the end point, so move to the next curve. Wrap around if necessary.
      const coilSegmentIndex = this.coilSegmentIndex + 1;
      if ( coilSegmentIndex > this.coilSegments.length - 1 ) {
        this.coilSegmentIndex = 0;
      }
      else {
        this.coilSegmentIndex = coilSegmentIndex;
      }

      // Set the position on the curve.
      const overshoot = Math.abs( coilSegmentPosition * this.getSpeedScale() / oldPathSpeedScale );
      coilSegmentPosition = 1.0 - overshoot;

      // Did we overshoot the curve? If so, call this method recursively.
      if ( coilSegmentPosition < 0.0 ) {
        this.switchCurves( coilSegmentPosition, ++recursionDepth );
      }
      else {
        this.coilSegmentPosition = coilSegmentPosition;
      }
    }
    else if ( coilSegmentPosition >= 1.0 ) {

      // We've passed the start point, so move to the previous curve. Wrap around if necessary.
      const coilSegmentIndex = this.coilSegmentIndex - 1;
      if ( coilSegmentIndex < 0 ) {
        this.coilSegmentIndex = this.coilSegments.length - 1;
      }
      else {
        this.coilSegmentIndex = coilSegmentIndex;
      }

We feel the documentation may be backwards. Going <=0 says "passed the end point, so move to the next curve" but should be changed to say "passed the start point, so move to the prior curve". Likewise >=1.0 says "passed the start point, so move to the previous curve" but should be changed to say "passed the end point, so move to the next curve".

[pixelzoom]

Similar to #138... The documentation here is not incorrect. The semantics of coilSegmentPosition are documented in a few places, including PickupCoil.ts.

  // Electron's position [0,1] along the coil segment that it occupies: 0=endPoint, 1=startPoint
  private coilSegmentPosition: number;

See #138 (comment) for why I do not intended to change this.

For the switchCurves method that you referred to above, I've added this reminder:

  private switchCurves( coilSegmentPosition: number, recursionDepth = 0 ): void {
    assert && assert( recursionDepth < this.coilSegments.length, `infinite loop? recursionDepth=${recursionDepth}` );

    const oldPathSpeedScale = this.getSpeedScale();

+   // Reminder: For coilSegmentPosition, 0=endPoint, 1=startPoint.
    if ( coilSegmentPosition <= 0 ) {

      // We've passed the end point, so move to the next curve. Wrap around if necessary.
      const coilSegmentIndex = this.coilSegmentIndex + 1;

Back to @samreid, close if OK.

[samreid]

Thanks, the additional documentation helps. Closing.