Legends of Learning message log changes sim state

[KatieWoe]

Devices
Dell and HP laptop
OS
Win 10 and Win 7
Browser
Chrome and Firefox
Problem Description
For phetsims/qa/issues/180
First documented in phetsims/masses-and-springs/issues/324
Pausing the sim with LoL and using the "message log" changes the state of the sim.
Steps to Reproduce

1. Enter the masses and springs sim (https://bayes.colorado.edu/dev/html/masses-and-springs/1.0.0-rc.4/phet/masses-and-springs_all_phet.html) into the Legends of Learning harness
2. Go to any of the screens
3. If damping is non-zero on the screen, turn it down to zero so that this effect can be better seen.
4. Start the harness.
5. Attach a mass and pull it all the way down so you have a large amplitude of motion
6. Press pause on the harness (not the sim). Attempt to do this when it is at the top of the screen.
7. Open the message log.

Screenshots

[KatieWoe]

Another example from phetsims/qa/issues/181

[Denz1994]

While doing some debugging with @jonathanolson we noticed that LegendsOfLearningSupport.js wrapper bugs at line 27. It seems to be sending a "pause" message when opening message logs. Should we be sending “pause” message when we didn’t actually pause in the wrapper? We opened the log so perhaps a "view log" message should be triggered as a separate event?

[jessegreenberg]

@ariel-phet is this a blocking issue?

[jessegreenberg]

LegendsOfLearningSupport.js is receiving the messages from the test harness. If the harness is paused (not the sim), the harness mutes and then sends another "pause" message to the sim when the "View Message Logs" button is pressed .

This doesn't show up under "Game Messages" in the UI. And no messages are sent when the harness is playing. Is this a LoL bug?

The sim changes because we step one frame when responding to the pause message, I think this was to handle another input bug but I can't quite remember.

[jessegreenberg]

Adding blocking label to flag and as a reminder to bring up during meeting, I am not sure if this needs to be fixed before the next RC for phetsims/qa#181.

[ariel-phet]

Seems blocking'ish to me (but might be something to report to LoL and not necessarily fix on our side)

[jessegreenberg]

We discussed this at dev meeting on 9/20/18 - We think this is a problem with the LoL test harness. @arouinfar it sounded like you were the point person for LoL, could you please reach out to them and let them know that there may be a bug in the test harness where opening the message log sends a second "pause" message when the app is already paused? Sorry if you aren't the right person for this, if that is the case please reassign to me and we can talk with @kathy-phet about next steps.

This issue doesn't need to block publication.

[arouinfar]

@jessegreenberg my only contact at LoL is a content manager, not a dev. I can certainly send him a link to this issue, and ask him to forward it to the appropriate dev. Before I do that, it would good to hear from @kathy-phet or @samreid, in case they know of an appropriate dev who I can contact directly.

[arouinfar]

I've drafted an email to LoL. I'll check with @kathy-phet this afternoon to see who she recommends I send it to.

[arouinfar]

@kathy-phet said she would reach out to LoL about this issue, so unassigning myself and @samreid.

[jessegreenberg]

This was recently reported again in #675. It still seems like a bug in the LoL test harness. But in #675 I said

Also, I can't remember why we needed to call sim.stepOneFrame here, but maybe we could only call it if sim.activeProperty.value is true?

That may stop the move forward on our end if this is important.

[samreid]

The additional frame was introduced in phetsims/pendulum-lab#187. But maybe we can work around the problem in this issue by skipping the frame when "pause" is called on an already "paused" sim. Like so:

Index: js/thirdPartySupport/LegendsOfLearningSupport.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/thirdPartySupport/LegendsOfLearningSupport.js	(revision 39712126e23f03fa6500ee3b85525f71026b4cec)
+++ js/thirdPartySupport/LegendsOfLearningSupport.js	(date 1604339397761)
@@ -24,8 +24,10 @@
     // Respond to pause/resume commands from the Legends of Learning platform
     window.addEventListener( 'message', function( message ) {
       if ( message.data.messageName === 'pause' ) {
-        sim.stepOneFrame();
-        sim.activeProperty.value = false;
+        if ( sim.activeProperty.value ) {
+          sim.stepOneFrame();
+          sim.activeProperty.value = false;
+        }
       }
       else if ( message.data.messageName === 'resume' ) {
         sim.activeProperty.value = true;

@jessegreenberg what do you think?

[arouinfar]

This was also reported in phetsims/qa#562 and phetsims/qa#565.