Error: Assertion failed: Impossible set state (electric potential line)

[phet-steele]

1. State wrapper, http://www.colorado.edu/physics/phet/dev/html/charges-and-fields/1.1.0-phetiodev.1/wrappers/state/state.html?sim=charges-and-fields&relativeSimPath
2. Drag out a charge.
3. Drag out the electric potential sensor.
4. Draw an electric potential line.
5. Wait...
6. Receive an error:

SimIFrameAPI Invocation Error: Error: Assertion failed: Impossible set state:
chargesAndFields.chargesAndFieldsScreen.model.electricPotentialLines_0
    at window.assertions.assertFunction (assert.js:22)
    at iterate (phetio.js?bust=1503431706507:210)
    at Object.setState (phetio.js?bust=1503431706507:227)
    at TPhETIO.implementation [as setState] (TPhETIO.js?bust=1503431706507:176)
    at Object.invoke (SimIFrameAPI.js?bust=1503431706507:258)
    at Object.handleSingleInvoke (SimIFrameAPI.js?bust=1503431706507:119)
    at Object.handleRequest (SimIFrameAPI.js?bust=1503431706507:161)
    at SimIFrameAPI.js?bust=1503431706507:278

Kills the wrapper. Seen on macOS 10.12.6 Chrome. For phetsims/qa/issues/37.

[samreid]

Proposed fix in preceding commits, @phet-steele can you verify on phettest? Would you like a new dev version?

[phet-steele]

@samreid with this one, I'd like a new dev version and will probably update the QA task with the new version.

[samreid]

New dev version is here: http://www.colorado.edu/physics/phet/dev/html/charges-and-fields/1.1.0-phetiodev.3/wrappers/index/

[phet-steele]

Looks fixed in the above dev version.

[jbphet]

@chrisklus and I stumbled across some code that looks potentially (pun semi-intended) problematic that was committed to resolve this issue. ChargesAndFieldsModel.addElectricPotentialLine now has two optional arguments - a Vector2 and a tandem, and there doesn't appear to be any code that is testing the types of the supplied arguments. Re-opening for @samreid to have a look.

[samreid]

In the commit, I rewrote the function using ES6 default parameters and I added checks for the parameter types. This pattern works in this case because the tandem is only supplied if the position is also supplied (options are not fully orthogonal). But let me know if this is too obscure and I can rewrite it using our idiomatic options pattern.

Also note: before I made any changes, I observed #161, which makes it difficult to test this code fully.

[jbphet]

It definitely does look a little odd to me, especially since there are function invocations for setting the default parameters, and I've never done this before. However, this apparently is valid in JavaScript, and the sim runs and creates electric potential lines just fine. Also, the asserts would catch any inappropriate parameters, so it seems okay to me.

The only additional item I would suggest is to explicitly document that the tandem is optional but is never included if the vector position vector is not (and fix the comment to adhere to the line length convention). Otherwise, I think others will find this confusing the way that @chrisklus and I did.

[jbphet]

@samreid - feel free to close once suggested comment is added, no need to assign back to us.

[samreid]

Thanks, I updated the comments accordingly, closing.