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

iPadOS 14 Safari does not show falling images when a game is won. #81

Open
KatieWoe opened this issue Sep 15, 2020 · 22 comments
Open

iPadOS 14 Safari does not show falling images when a game is won. #81

KatieWoe opened this issue Sep 15, 2020 · 22 comments

Comments

@KatieWoe
Copy link

Found during phetsims/qa#549.
If you win a game in which images are supposed to fall, on iPadOS 14 Safari they do not do so. The sound plays and the dialog saying you won comes up, but the "confetti" does not. Seen in Build a Fraction as an example. Does occur in Win 10 Chrome, so not an issue with the sim itself. Not a major issue itself. Not sure if such graphical issues could be more impactful elsewhere. Haven't seen anything yet, but will keep an eye out.
iPadOS 14 is currently in Beta, but will likely roll out fairly soon.

@samreid
Copy link
Member

samreid commented Sep 15, 2020

Have you checked whether this problem occurs on other iOS versions? When tethering to iOS 14, is there any error or message in the console?

@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 15, 2020

@KatieWoe does the vegas demo work on iOS 14? Go to the "Reward" screen and you should immediately see stars and smiley faces falling, like this:

screenshot_570

@KatieWoe
Copy link
Author

Where can I find the vegas demo? And I don't have an iPadOS 13 at the moment because I put it on Beta for this. @jbphet do you mind seeing if this happens with your device?

@pixelzoom
Copy link
Contributor

You can find the vegas demo on phettest, just like running any sim. I also just published 1.0.0-dev.4 if that helps.

@KatieWoe
Copy link
Author

I am seeing the falling objects in 1.0.0-dev.4

@pixelzoom
Copy link
Contributor

pixelzoom commented Sep 15, 2020

vegas 1.0.0-dev.4 works fine for me on my iPad6 running iPadOS 13.6.1.

@KatieWoe
Copy link
Author

Is Build a Fraction working on that device? If not it may be more sim specific than I thought, and not due to the new OS

@pixelzoom
Copy link
Contributor

Which version of Build a Fraction are you testing? phetsims/qa#549 doesn't say. And does it have any query parameters that show the reward regardless of whether you win? (It should, so I don't have to waste my time playing and winning a game to test the reward.)

@KatieWoe
Copy link
Author

I'm going through published sims. No query parameters.

@KatieWoe
Copy link
Author

KatieWoe commented Sep 15, 2020

I believe I am seeing the same behavior on Expression Exchange
Clarification: Expression Exchange is showing the bug.

@KatieWoe
Copy link
Author

Fraction Matcher is working as expected. Not sure what the issue stems from in that case. Sorry.

@KatieWoe
Copy link
Author

KatieWoe commented Sep 16, 2020

Summary:
Sims that I saw with the bug: Expression Exchange, Build a Fraction
Did not show it: Fraction Matcher, Vegas demo
These are the sims with reward node that I checked.

I think this may also be happening with iPadOS 13.7, so it looks like it isn't an iPadOS 14 issue.

@pixelzoom
Copy link
Contributor

Over in #82, I noticed that 3 sims have their own subclass of RewardNode: Build an Atom, Expression Exchange, and Make A Ten. One of those (Expression Exchange) is implicated here. @KatieWoe was the reward working for the other 2?

@KatieWoe
Copy link
Author

Both Build an Atom and Make a Ten seem to behave normally with the falling objects. Only Expression Exchange, of those three options, seems to show the bug.

@jbphet
Copy link
Contributor

jbphet commented Oct 1, 2020

We discussed this in the 10/1/2020 developer meeting, and I should follow up by double checking that the reward node works in both Build an Atom and Expression Exchange, and should remove the code in BAARewardNode that instruments nodes that don't need it.

@jbphet
Copy link
Contributor

jbphet commented Oct 23, 2020

I did some work on BAARewardNode for #82. I just tested it on iOS 14 using the live version and the master version, and the reward node worked fine in both cases, so I think we're good to go on that one (though I don't know what resolved the problem).

I then tested expression-exchange on iOS 14 and used the showRewardNodeEveryLevel and, unfortunately, I'm not seeing the reward node. I tested the version that is live on the PhET site and the one on master, and both have this problem. It works fine in Chrome. I think I will do the ES6 conversion for this sim and then come back and do more investigation on this.

@ariel-phet
Copy link

@jbphet although it is unfortunate that the reward node is not showing in a case like expression exchange, the sim still works, no crashing occurs, and it is certain not going to impact the pedagogical value of the sim.

Perhaps this would be a good bug for someone like @SaurabhTotey to track down and investigate?

@jbphet
Copy link
Contributor

jbphet commented Oct 16, 2021

I did some investigation on this since the issue has been lying around for a long time. The reason that the reward node doesn't appear is because there is explicit code that prevents it from appearing. It looks like this was done to reduce the memory footprint on an iPad, see phetsims/expression-exchange#91. The issue explicitly mentions iPad 2, which is no longer supported, so maybe this isn't an issue anymore.

I went ahead and removed the code, effectively turning the reward node back on for iOS devices, and tested it on the iPad 4 that I have (I used the showRewardNodeEveryLevel to save time), and it worked great.

@KatieWoe - Please have QA test this change for expression-exchange on multiple iPads and, if it seems to work okay, expression-exchange should be good to go.

Once the testing is complete, please assign the issue to @jonathanolson. I'm seeing similar code on line 305 of BuildingGameScreenView, which is probably why the reward node isn't showing for the fractions sims, and @jonathanolson should decide if he wants to change this, assuming that the QA testing goes well.

@jbphet jbphet assigned KatieWoe and unassigned jbphet Oct 16, 2021
@pixelzoom
Copy link
Contributor

pixelzoom commented Oct 16, 2021

@jbphet said:

I went ahead and removed the code, effectively turning the reward node back on for iOS devices

Where did you do this? I see no commit linked here or in phetsims/expression-exchange#91. I also don't see any recent commits in master for expression-exchange or vegas that look like they are related to this. Did you "remove the code" only in some branch?

I'm trying to determine if this is relevant only to expression-exchange and fractions-common, or whether it's relevant to all sims with a reward (like the just-deployed Fourier 1.0).

jbphet added a commit to phetsims/expression-exchange that referenced this issue Oct 18, 2021
@jbphet
Copy link
Contributor

jbphet commented Oct 18, 2021

Sorry @pixelzoom, I thought I'd pushed it but apparently hadn't. It's there now, and it is specific to Expression Exchange code so it shouldn't impact Fourier.

@jbphet jbphet assigned pixelzoom and unassigned jbphet Oct 18, 2021
@pixelzoom pixelzoom removed their assignment Oct 18, 2021
@KatieWoe
Copy link
Author

This looks fixed on Expression Exchange in master on iPadOS 15

@jbphet
Copy link
Contributor

jbphet commented Oct 19, 2021

Assigning to @jonathanolson to potentially address in the fractions screen. @jonathanolson - You'll want to read through this comment from above, most of the rest of the information in the issue can be ignored.

@jbphet jbphet assigned jonathanolson and unassigned jbphet and KatieWoe Oct 19, 2021
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