search

phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Legends of Learning message log changes sim state #515

Open KatieWoe opened 6 years ago

KatieWoe commented 6 years ago

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 jumpmenu

KatieWoe commented 6 years ago

Another example from phetsims/QA/issues/181 jumpmenu2

Denz1994 commented 6 years ago

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 commented 5 years ago

@ariel-phet is this a blocking issue?

jessegreenberg commented 5 years ago

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 . image

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 commented 5 years ago

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 https://github.com/phetsims/QA/issues/181.

ariel-phet commented 5 years ago

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

jessegreenberg commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

jessegreenberg commented 3 years ago

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 commented 3 years ago

The additional frame was introduced in https://github.com/phetsims/pendulum-lab/issues/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 commented 3 years ago

This was also reported in https://github.com/phetsims/QA/issues/562 and https://github.com/phetsims/QA/issues/565.