phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

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

Open KatieWoe opened 3 years ago

KatieWoe commented 3 years ago

Found during https://github.com/phetsims/QA/issues/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 commented 3 years ago

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

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

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

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

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

pixelzoom commented 3 years ago

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

KatieWoe commented 3 years ago

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

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

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

KatieWoe commented 3 years ago

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

KatieWoe commented 3 years ago

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

KatieWoe commented 3 years ago

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

Over in https://github.com/phetsims/vegas/issues/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 commented 3 years ago

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

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

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

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

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

pixelzoom commented 2 years ago

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

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.

KatieWoe commented 2 years ago

This looks fixed on Expression Exchange in master on iPadOS 15

jbphet commented 2 years ago

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.