search

phetsims / forces-and-motion-basics

"Forces and Motion: Basics" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/forces-and-motion-basics
GNU General Public License v3.0
7 stars 10 forks source link

Prepare for Legends of Learning #235

Closed samreid closed 6 years ago

samreid commented 7 years ago

@kathy-phet said

Some good ones for legends of learning priorities would be force and motion: basics and energy skate park : basics ... assuming they had these on their list too. Force and motion: basics is somewhere awaiting the republication of the updates on screen 1. It would be great to get this version out. I'm unsure of its status. I think maybe Jesse wanted someone to check over the iO code?

@amanda-phet we will also need icons for this.

samreid commented 7 years ago

Discussion in #232 says there was an RC that was close to publication and we were deliberating whether to use RC SHAS or not. @jessegreenberg can you comment on whether the RC included LOL testing? If not, can a quick test or two reveal whether this sim (in the RC SHAS) exhibits touch problems like the ones we saw for build an atom? If the RC SHAS are problematic, is master problematic?

jessegreenberg commented 7 years ago

FAMB 2.3.0-rc.2 passed RC testing here: https://github.com/phetsims/QA/issues/11

FAMB 2.3.0-phetiorc.2 has not been completed: https://github.com/phetsims/QA/issues/12

2.3.0-rc.2 was deployed at the same time as 2.3.0-phetiorc.2 as part of the release strategy outlined in https://github.com/phetsims/chipper/issues/587. RC SHAs are not problematic, but to keep both sets of SHAs aligned both versions need to be deployed at the same time, and fixes required for the 2.3-phetio branch will need to be applied to the 2.3 branch.

2.3.0-rc.2 was built Tue Jun 27 2017 and did not include testing for LOL. The fix for the issues in build an atom were committed afterward in https://github.com/phetsims/scenery/commit/e75c1c55981e54cb87e77a430a9fcee66362ddb6 on Jun 29 2017.

jessegreenberg commented 7 years ago

To verify, I tested 2.3.0-rc.2 in the LoL test harness, and was able to reproduce the same problems with dragging that we were seeing in build-an-atom.

I am unable to reproduce the problem in master. So I think to support LoL we should rebuild with master and do a new RC test including LoL testing.

@samreid @zepumph do you think that we should coordinate the 2.3 and 2.3-phetio releases again? This would be in line with our decision in phetsims/chipper#587, but it could take quite a bit longer to deploy a LoL version.

If 2.3-phetio doesn't need to pick up scenery changes, we could start fresh with a 2.4 branch and let 2.3-phetio proceed as it is (not coordinating the two).

zepumph commented 7 years ago

I think the simplest solution to get both released as quickly and effectively as possible is to go ahead with 2.3-phetio, and take master for 2.4, The solution for the phet-io bug https://github.com/phetsims/forces-and-motion-basics/issues/232 can be cherry-picked into that branch. There is an argument for releasing both as 2.3/ 2.3-phetio from current master, because of less branching confusion.

I don't have a preference. Whatever is easiest for QA.

jessegreenberg commented 7 years ago

Thanks @zepumph, I agree. Though I might recommend replacing 2.3 with a 2.4 branch so it is clear that it has separate SHAs from 2.3-phetio.

zepumph commented 7 years ago

Edited above, I agree. That was a typo!

jessegreenberg commented 7 years ago

Sounds good! @samreid if you agree, should I proceed with an RC for FAMB now?

samreid commented 7 years ago

Cherry picking https://github.com/phetsims/chipper/issues/587 into 2.3-phetio seems reasonable, but it looks like https://github.com/phetsims/QA/issues/12 still has a lot of testing to do for the phet-io version.

I can see that the non-phet-io version passed RC testing and that's great, but it may be inefficient to fully test the phet-io version now then again in a week or two for the LOL release. On the other hand, getting phet-io through testing now probably means less bugs or easier testing for 2.4 LOL time.

I don't think I can make the call, it seems like it should be up to @kathy-phet or @ariel-phet.

jessegreenberg commented 7 years ago

Sounds good @samreid, Ill bring it up for discussion at tomorrow's status meeting.

samreid commented 7 years ago

I emailed @ariel-phet (because I think he may be out tomorrow)

Hi Ariel,

Jesse and I had a question about scheduling testing for Forces and Motion: Basics. Kathy wanted to schedule Legends of Learning for the near future (which requires master), but the previous version which has passed QA for non-phet-io is buggy for Legends of Learning and still needs to go through phet-io QA. We weren't sure whether to test the buggy LOL one for phet-io and then test another version for LOL later, or jump to master and only do one round of phet-io testing.

Please comment here or in https://github.com/phetsims/forces-and-motion-basics/issues/235

Best Regards, Sam

ariel-phet commented 7 years ago

@samreid I say bite the bullet and go to master.

samreid commented 7 years ago

OK thanks @ariel-phet. I'll leave it up to you @jessegreenberg

jessegreenberg commented 7 years ago

Thanks @samreid, but just to clarify, the decision is to build 2.3 and 2.3-phetio off of master and deploy both at the same time, synchronizing their release and SHAs?

samreid commented 7 years ago

I think that's been our consensus for deploying sims that have phet-io (though I have to admit there has been a lot of churn in those ideas and I'm not 100% sure of our final decision). It seems the discussion was mostly here? https://github.com/phetsims/chipper/issues/587

jessegreenberg commented 7 years ago

Alright, thanks. I will build new RCs for 2.3 and 2.3-phetio off of master once these issues are completed:

zepumph commented 7 years ago

232 is all set!

samreid commented 7 years ago

A reminder to test for LOL features in the next RC, unassigning self.

amanda-phet commented 7 years ago

Icons are located here: https://drive.google.com/drive/folders/0B3v_ZP9Kq36CQnVYRnZYaDRhVkk

jessegreenberg commented 7 years ago

I created an RC for this issue here: https://github.com/phetsims/QA/issues/42

This RC is for the phet-io version first, when that RC passes, we will start a new one off of the same SHAs.

jessegreenberg commented 6 years ago

2.3.0-rc.4 passed testing, we are ready for a 2.3.0 RC.

EDIT: this will be 2.3.0-rc.3

samreid commented 6 years ago

We will need the fix in https://github.com/phetsims/joist/issues/447

samreid commented 6 years ago

@jessegreenberg as identified in https://github.com/phetsims/joist/issues/450 we will need a joist change before this can be published for LOL. Can you include https://github.com/phetsims/joist/commit/63046a699e8e3994f9c40f8d93482a497f0316b3 and create a new RC?

samreid commented 6 years ago

Please see also https://github.com/phetsims/joist/issues/449#issuecomment-340263119, we are considering picking up some other commits for this maintenance release.

jessegreenberg commented 6 years ago

Sure, sounds good. I read through phetsims/joist#449 and it sounds like there is still some work to do there so I will wait to cherry pick commits related to that issue and create a new RC until that is settled.

jessegreenberg commented 6 years ago

Actually, the version of joist and LegendsOfLearning.js did not have sim.boundRunAnimationLoop();, it was added after the rc for FAMB was created. I will try to verify that phetsims/joist#447 is not a problem for FAMB.

jessegreenberg commented 6 years ago

From https://github.com/phetsims/joist/issues/447:

CCKDC stayed at 60 fps through multiple pauses and resumes and didn't climb to 60000 fps.

I pressed pause many times and pressed pause then resume many times and did not see fps ever go above 60, and since sim.boundRunAnimationLoop() isn't being used on the 'pause' event we shouldn't need any patching for 2.3.

EDIT (for phetsims/joist#447, will still need changes for phetsims/joist#449)

samreid commented 6 years ago

May also need https://github.com/phetsims/joist/issues/458 ?? It's a question.

jessegreenberg commented 6 years ago

Thanks! Understood.

jessegreenberg commented 6 years ago

It looks like phetsims/joist#447 and phetsims/joist#449 and phetsims/joist#458 are all closed, Ill review and see if I can determine what needs to be picked up.

jessegreenberg commented 6 years ago
jessegreenberg commented 6 years ago

Grabbed all updates, verified they are in both 2.3 and 2.3-phetio branches of this sim. Once https://github.com/phetsims/QA/issues/57 is complete, we can spot check and then redeploy.

phet-steele commented 6 years ago

Testing is good to go for this.

kathy-phet commented 6 years ago

yay!

jessegreenberg commented 6 years ago

FAMB 2.3.0 was deployed with testing and support for LoL. Closing.