search

phetsims / geometric-optics

Geometric Optics is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 5 forks source link

Migrate overrides.js into code. #459

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

Depending on the outcome of prerequisite https://github.com/phetsims/phet-io/issues/1873 ...

Migrate geometric-optics-overrides.js and geometric-optics-basics-overrides.js into code.

pixelzoom commented 1 year ago

Do https://github.com/phetsims/geometric-optics/issues/460 before doing this issue, so that it's possible to confirm that the API does not change as the result of migrating overrides.js to code.

pixelzoom commented 1 year ago

460 is done. Migration can be verified by running grunt generate-phet-io-api and verifying that there are no changes to the API files.

pixelzoom commented 1 year ago

Migration has been completed, with the exception of a small number of entries in geometric-optics-basics-overrides.js -- see https://github.com/phetsims/geometric-optics/issues/463.

Migration took ~3 hours. It was complicated by the fact that it involved 2 overrides.js files: geometric-optics-overrides.js and geometric-optics-basics-overrides.js. There were some differences between the 2 sims that were a bit tricky/messy, and I ended up referring to GOSimOptions.isBasicsVersion quite a bit to decide whether to feature something. This definitely added some significant complication to the code. I guess it's a trade-off, but time will tell.

Keeping this issue self-assigned until https://github.com/phetsims/geometric-optics/issues/463 is resolved. Then I'll assign to @arouinfar to check my work.

pixelzoom commented 1 year ago

Addressing https://github.com/phetsims/geometric-optics/issues/463 resulted in the remainder of geometric-optics-basics-overrides.js being deleted. So migration is done, and both overrides.js files are now empty.

Migration was verified by running grunt generate-phet-io-api to ensure that there were no API changes. So I'm confident that no regressions were introduced. But I'll assign this to @arouinfar in case she'd like to do a quick manual spot check. Feel free to close when you're done.

pixelzoom commented 1 year ago

Discusssed with @arouinfar ... This will not block the QA dev version for https://github.com/phetsims/geometric-optics/issues/278, and I'll proceed with publishing that. @arouinfar can spot-check at her convenience, either in the dev version or master.

pixelzoom commented 1 year ago

@arouinfar Reminder that while this does not block dev testing, it will block creation of the release branch and RC testing.

arouinfar commented 1 year ago

I reviewed the featured tree and didn't notice anything amiss. Thanks @pixelzoom.