Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Steam does not animate when advanced from pause #190

Closed
KatieWoe opened this issue Jan 22, 2019 · 14 comments
Closed

Steam does not animate when advanced from pause #190

KatieWoe opened this issue Jan 22, 2019 · 14 comments

Comments

@KatieWoe
Copy link
Contributor

Test device:
Dell
Operating System:
Win 10
Browser:
chrome
Problem description:
For phetsims/qa#263. Graphical oddity, likely very minor.
When the sim is playing, steam animates upwards. However, if pause and moving forward frame by frame, the steam stays completely in place. Other objects such as energy chunks and falling objects move, but not the steam. The steam stay even if the beaker cools during this advancement.
Steps to reproduce:

  1. Heat a beaker up until steam appears
  2. Pause sim while steam is present
  3. Hold down the advance button to move the sim forward frame by frame
  4. Having an object fall during this may indicate that the advance button is otherwise working.

Screenshots:
stepforwardsteam

Troubleshooting information (do not edit):

Name: ‪Energy Forms And Changes‬
URL: https://phet-dev.colorado.edu/html/energy-forms-and-changes/1.0.0-dev.16/phet/energy-forms-and-changes_en_phet.html
Version: 1.0.0-dev.16 2019-01-18 17:19:03 UTC
Features missing: touch
Flags: pixelRatioScaling
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/71.0.3578.98 Safari/537.36
Language: en-US
Window: 1536x732
Pixel Ratio: 2.5/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: {}

@chrisklus
Copy link
Contributor

Ooh nice, that's a problem! I'll investigate.

@KatieWoe
Copy link
Contributor Author

Also seems to happen on second screen when heating the water

@KatieWoe
Copy link
Contributor Author

Much less noticeable, but the water from the faucet, and more noticeable the teapot cloud.

@chrisklus
Copy link
Contributor

I think these were all animations that originally didn't stop animating when the sim was paused, so I didn't remember to add a way to step them once the sim was in a paused state.

@ariel-phet
Copy link

Certainly would be nice to fix, but also not a showstopper for 1.0 since I doubt it would cause major pedagogical issues.

@ariel-phet ariel-phet removed their assignment Jan 22, 2019
chrisklus added a commit that referenced this issue Jan 22, 2019
@chrisklus
Copy link
Contributor

chrisklus commented Jan 22, 2019

I would argue that this was pretty problematic, at least for the water faucet and the tea kettle, since you can turn those elements off while the sim is paused and the energy/energy chunk emission will update accordingly while manually stepping, but the corresponding steam and falling water would remain unchanged.

Regardless, all of the cases that @KatieWoe mentioned have been fixed. I split up the view step method in the same way that they are arranged in the model, and then added an emitter in the model's manualStep to call the view's new manualStep.

@KatieWoe please review on master.

@KatieWoe
Copy link
Contributor Author

Looks good on master @chrisklus

@chrisklus
Copy link
Contributor

Thanks @KatieWoe. I got a code sign-off from @jbphet after I made a recommended update to my changes in the commit above. Closing.

@KatieWoe
Copy link
Contributor Author

For phetsims/qa#268. Minor update. I saw that the smoke animates as you start dragging the beaker around. This is a very minor thing.
https://drive.google.com/file/d/1kgYv_obHYBZT8ImxC4JrFpPYeLHSq71Q/view?usp=sharing

chrisklus added a commit that referenced this issue Jan 25, 2019
@chrisklus
Copy link
Contributor

chrisklus commented Jan 25, 2019

@KatieWoe that is wild!

/**
 * step this view element, called by the framework
 * @param dt - time step, in seconds
 * @public
 */
step: function( dt ) {
  if ( this.model.isPlayingProperty.get() ) {
    this.stepView( dt );
    console.log( 'sim is playing');
  }
},

This is how we're stepping view-only things like the steam. The isPlaying guard is working correctly, so when the sim is paused, the console stops logging 'sim is playing', regardless of whether the beaker is being dragged or not. I tried this log statement at every step along the way until the steam itself (stepView in EFACIntroScreenView, step in BeakerView, step in PerspectiveWaterNode, step in SteamCanvasNode) and all of them are not running when this sim is paused and the beaker is being dragged. So, that means that something else (not EFAC code) is calling paintCanvas in SteamCanvasNode.

I'm hoping @jonathanolson has some insight as to why this could happen.

@chrisklus
Copy link
Contributor

Update:

I committed a quick hack that prevents the steam from animating when the sim is paused and the beakers are being dragged, but paintCanvas is still being called from an unknown source.

You can't pass in args to invalidatePaint, but we still wanted to use dt to animate the steam, so when dt is passed into the step function, we're saving it as a property of SteamCanvasNode so that the method renderSteam has access to it. I realized that the only way the steam was still animating correctly when paintCanvas is called from a mystery location was because it still had the most recent this.dt to give it the illusion of passing time. So, I set this.dt = 0 at the end of renderSteam so that the mystery source has no time to work with when it calls paintCanvas, and the animation remains frozen.

@chrisklus
Copy link
Contributor

@jbphet thought it was normal behavior for paintCanvas to be called while dragging, so I'm assigning him to review my fix, and then we can close.

@jbphet
Copy link
Contributor

jbphet commented Feb 21, 2019

The current behavior looks good, but the setting of dt to zero is no longer needed because the sim un-pauses when a drag starts. Back to @chrisklus to remove and update.

@jbphet jbphet removed their assignment Feb 21, 2019
chrisklus added a commit that referenced this issue Feb 25, 2019
@chrisklus
Copy link
Contributor

Thanks @jbphet. Updated, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants