phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Second sim not present in multi wrapper #266

Closed liammulh closed 3 years ago

liammulh commented 3 years ago

Test Device

iPad Pro / MacBook Air

Operating System

iPadOS 14.4.2 / macOS 11.2.3

Browser

Safari 14.0.3 / Chrome 89.0.4389.114

Problem Description

For https://github.com/phetsims/QA/issues/636. The second sim in the multi wrapper doesn't appear. See screenshot.

Visuals

issue

phet-steele commented 3 years ago

The errors I get on Win 10 Chrome:

image

liammulh commented 3 years ago

I'm seeing the same errors on macOS Chrome.

pixelzoom commented 3 years ago

This looks like a job for the PhET-iO team! 🦸🏻‍♂️

pixelzoom commented 3 years ago

On Slack, I asked @kathy-phet about which issues are blocking. Related to this issue, she replied:

https://github.com/phetsims/natural-selection/issues/266 - I will ask phet-io to look at this

zepumph commented 3 years ago

In https://github.com/phetsims/phet-io/issues/1733 we updated the way in which we compare apis, and added it as part of the built phet-io library/preload for our wrappers.

@samreid, In the above commit I made it so that it could gracefully support being overwritten, like in the multiwrapper when we import the library file two times. There is already a precedent for this in files like WrapperTypes, and PhetioIDUtils.

I also nested the global under phetio, because that is where globals go these days (same precedent in all our files). Especially look at PhetioIDUtils, as it is used in wrappers and as a preload in our sims.

@samreid please review.

samreid commented 3 years ago

I reviewed the change sets and tested in a built version of natural selection, and it all looks correct, thanks! Since NS was a dev version, I don't think we need cherry-picking. So perhaps this issue can be closed.

pixelzoom commented 3 years ago

Since this was not verified by QA before closing, I'm going to keep this open for verification by QA in RC testing.

pixelzoom commented 3 years ago

To verify this issue:

KatieWoe commented 3 years ago

Looks good in rc.1