Toggling all coefficients with moving/merging expressions sets them free

[EthanWJohnson]

This bug is evil, because reproducing it is unreliable at best. FuzzMouse could do it because it could manipulate expressions the frame after it toggled coefficients. @phet-steele and I tried to get this figured out yesterday, and today I've found a way to reproduce this regularly on Win 10 chrome. The steps below outline the process;

1.) Assemble two expressions of coins on the explore screen. The first should be about 9 long and can be any coin, but I would recommend the second be composed of six coins using the largest two coins available--they get thrown out of alignment by All Coefficients more easily 2.) Place the second expression on top of the first one so that they will merge. 3.) Just before they merge (as close as possible) toggle All Coefficients. They should not merge at this point and all coefficients should be toggled on. 4.) Place the now-misaligned second expression back on top of the first one. 5.) Just before they merge (once again as close as possible) toggle All Coefficients again. If timed right, one of two things should happen; a.) The second expression realigns itself as per all coefficients and then merges anyway. If this happens, the bug has been triggered. Split the resulting expression, reassemble the original expressions (order doesn't matter but the composition might) and then do steps two and three again. One of the terms should be outside the box (even if slightly) and can be grabbed and added to expressions as if it were singular. b.)The second expression realigns itself properly as if it had merged but doesn't. The bug has been triggered in this case as well. Do steps two and three again to get a coin sticking outside of the box, and it will be able to be grabbed. c.) Neither of the above happens, which means your timing was off. Repeat four and five until one of them does.

The trick here is in timing; toggling coefficients has to be triggered almost precisely before the expressions are due to merge with each other once one of them has already been misaligned by the all coefficients toggle.

Once I have access to my laptop again (Microsoft decided they HAVE to install updates RIGHT NOW) I'll throw up a gif showing the process.

Related to https://github.com/phetsims/tasks/issues/860

Debug info; Name: ‪Expression Exchange‬ URL: http://www.colorado.edu/physics/phet/dev/html/expression-exchange/1.1.0-dev.3/expression-exchange_en.html Version: 1.1.0-dev.3 2017-06-13 18:51:38 UTC Features missing: touch User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Language: en-US Window: 1920x960 Pixel Ratio: 1/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"5c3f9204","branch":"master"},"axon":{"sha":"045bd1b0","branch":"master"},"babel":{"sha":"6d9fa297","branch":"master"},"brand":{"sha":"c904b097","branch":"master"},"chipper":{"sha":"80ae1f14","branch":"master"},"dot":{"sha":"6b4ec39c","branch":"master"},"expression-exchange":{"sha":"5d9ee7b4","branch":"master"},"joist":{"sha":"b35547e9","branch":"master"},"kite":{"sha":"7d2df183","branch":"master"},"phet-core":{"sha":"76ec0204","branch":"master"},"phetcommon":{"sha":"ca2f62cd","branch":"master"},"query-string-machine":{"sha":"c74e454e","branch":"master"},"scenery":{"sha":"c2248424","branch":"master"},"scenery-phet":{"sha":"01067d19","branch":"master"},"sherpa":{"sha":"812fdf9e","branch":"master"},"sun":{"sha":"32cc2197","branch":"master"},"tandem":{"sha":"7b8b8188","branch":"master"},"vegas":{"sha":"45acc1de","branch":"master"},"vibe":{"sha":"5e8151e1","branch":"master"}}

[jbphet]

I've added code that prevents animation of the coin term position updates when an expression is in motion. I've tested a bit after adding this, and was unable to duplicate the problem. I hesitate to say it is addressed, though, since it's so hard to duplicate in the first place. Assigning to @phet-steele and @EthanWJohnson to test on master. We are trying to get this sim out soon, so this testing is a high priority.

[phet-steele]

@jbphet this is still possible, but is a little fishy... I didn't have a term visibly outside of the expression bounds, but after hitting 5a. and splitting the expression, there was ONE term that could not be remade into an expression. It is almost like it failed to be removed from the expression when the expression was split, but was still visually removed.

Here's what it looked like. Trying to add it back to an expression threw an error and left the joining hint:

Error: Assertion failed: coin term is already present in expression
    (anonymous function) (assert.js:21)
    addCoinTerm (Expression.js:418)
    addCoinTermAfterAnimation (ExpressionManipulationModel.js:1046)
    emit (Emitter.js:114)
    step (Expression.js:302)
    (anonymous function) (ExpressionManipulationModel.js:347)
    forEach
    forEach (ObservableArray.js:306)
    step (ExpressionManipulationModel.js:346)
    stepSimulation (Sim.js:830)
    runAnimationLoop (Sim.js:770)
    runAnimationLoop

[EthanWJohnson]

I'm having a lot of difficulty reproducing this in its original form, particularly due to increased merge speed.

[phet-steele]

Agreed, this is tough to execute let alone reproduce. One of my "trials" in 1.1.0-dev.4 looked like a decent execution, but I saw no bug afterward. This seems like a one in a million (if it even can happen anymore) so, closing as fixed.