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

Mirror-inputs wrapper quickly is out of sync after keyboard nav & Reset All #194

Open
phet-steele opened this issue Feb 13, 2017 · 13 comments

Comments

@phet-steele
Copy link
Contributor

It is more obvious when interrupting towards the end of the discharge (when there are about to be no electrons left):

The sims will end up with differing amounts of electrons, leading to different discharge occurrences (about halfway in the video the bottom one discharges while the top does not):

jtio02

Seen on macOS 10.12.3 Chrome. For phetsims/tasks/issues/780.

@EthanWJohnson
Copy link

Possibly a macOS specific issue; I'm unable to get the same to occur in win10.

@samreid
Copy link
Member

samreid commented Feb 14, 2017

I'm not seeing any rand calls or Date.now that might lead to a divergence of playbacks.

@samreid
Copy link
Member

samreid commented Feb 14, 2017

Likewise there are no lodash random calls.

@samreid
Copy link
Member

samreid commented Feb 16, 2017

I tested for a minute and could not create any discrepancies in the mirror wrapper.

@phet-steele
Copy link
Contributor Author

@samreid @jessegreenberg I know how to make this happen consistently (although, I don't know why). It is very much reliant on keyboard nav. Here is a controlled setup to demonstrate:

  1. Tab to the leg in the top sim
  2. Move the leg with the arrow keys in slow increments until you grab one electron. The bottom will not grab an electron (this is expected).
  3. Every step after this is with a mouse. Click Reset All.
  4. Grab the leg at the top and move it very slowly until the first instance where you pick up electron(s). The two sims should then grab differing numbers of electrons.

I do not believe you need to do this procedure slowly to see the issue, but doing it slowly is more demonstrable. Subsequent Reset Alls will not re-sync the sims, you must refresh the browser.

jtio04

@phet-steele
Copy link
Contributor Author

@EthanWJohnson was successful in reproducing on Win 10.

@phet-steele phet-steele changed the title Mirror-inputs wrapper quickly is out of sync after interrupting a discharge Mirror-inputs wrapper quickly is out of sync after keyboard nav & Reset All Feb 16, 2017
@jessegreenberg
Copy link
Contributor

This issue seems related to keyboard navigation or the main model, assigning myself to take a look.

@jessegreenberg jessegreenberg self-assigned this Feb 17, 2017
@samreid samreid removed their assignment Feb 20, 2017
@jessegreenberg
Copy link
Contributor

@phet-steele did the example in #194 (comment) also use keyboard navigation first?

@phet-steele
Copy link
Contributor Author

@phet-steele did the example in #194 (comment) also use keyboard navigation first?

More than likely. At the time of recording, it was certainly not apparent that keyboard nav was a necessary part of the procedure. #194 (comment) is the most informed report.

@jessegreenberg
Copy link
Contributor

Ok, great, thanks!

@jessegreenberg
Copy link
Contributor

There are two parts of this issue. First, the number of electrons in the body can become out of sync as shown in #194 (comment). Second, positions of electrons in the body can become out of sync, as shown in #194 (comment).

The above commit fixes the first part of the issue. The second part of the issue is because JohnTravoltageModel.addElectron calls phet.joist.random.nextDouble() to place the electron in a random spot in the foot. In the input frame, we go through the random number sequence as electrons are added with the keyboard. So the position in the random number sequence is getting behind in the mirror input frame when we use the keyboard. From then on, all electron positions in the body are out of sync.

The best fix for this is probably to get phet-io to capture all keyboard events.

@jessegreenberg
Copy link
Contributor

This issue illustrates an problem in the overlap between phet-io and accessibility, not sure if it should remain at high priority.

@ariel-phet should this be a high priority issue?

@ariel-phet
Copy link

@jessegreenberg I do not think this issue should remain at high priority. iO and a11y will need to "play nice" in the long run, so it is worth some investigation, but since iO strategy is still in flux, I don't think we need to have a perfect fix/overlap just yet. Good to understand the "why" but not necessarily make things perfect yet.

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

6 participants