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

incorporate support for Legends of Learning #210

Closed
jbphet opened this issue Dec 1, 2017 · 15 comments
Closed

incorporate support for Legends of Learning #210

jbphet opened this issue Dec 1, 2017 · 15 comments

Comments

@jbphet
Copy link
Contributor

jbphet commented Dec 1, 2017

I reported in a recent status meeting that I'm spending some time working on #208. @kathy-phet has requested that as I work on this I should also add support for LoL. Here is the email dialog:

(@kathy-phet): As you work to fix States of Matter, please also add legends of learning capabilities to this sim. The various versions will fit nicely into their categories and help us start to earn revenue from them, and if we are republishing/testing we should go ahead and add this capability.

(@jbphet): Sounds good, will do. Is @samreid the person to talk to in order to make sure I'm clear as to what exactly this entails?

(@ariel-phet): Yes, @samreid would be good to talk to, but I believe as long as the sim is built off master these days, it comes with LoL features (@phet-steele might also know). So it might be as simple as bringing the sim up to current SHAs, but don't quote me on that. LoL testing comes standard as part of QA for new sims.

(@jbphet): I'm okay with building off of master, but it would mean a more intensive round of QA is required. Are we cool with that?

(@ariel-phet): Yes, I am cool with that. Although that might mean that actually deploying the sim happens after we have chipper worked out.

@jbphet
Copy link
Contributor Author

jbphet commented Dec 1, 2017

@samreid - can you confirm that building off of master will incorporate LoL support?

@samreid
Copy link
Member

samreid commented Dec 1, 2017

Joist/master of LegendsOfLearningSupport.js and its usages are intended to support Legends of Learning and hopefully sufficient. However, the simulation developer also needs to make sure that nothing in the simulation happens outside of the step(dt) function. For example, some of our older sims use window.setTimeout or window.setInterval--those will not be able to pause correctly. Also there is a caveat that our Legends of Learning support is in its infancy, so we require a testing phase for it during QA time and we need to be prepared for new problems that come up.

@samreid samreid assigned jbphet and unassigned samreid Dec 1, 2017
@jbphet
Copy link
Contributor Author

jbphet commented Dec 1, 2017

Thanks @samreid. Is there a good way for me as a developer to check that the functionality works once I've taken the steps you outlined, or is it best to build a dev version and have QA check it?

@samreid
Copy link
Member

samreid commented Dec 1, 2017

From @jessegreenberg note on slack:

It does, to see features you can add query param ?legendsOfLearning, and put sim url in "Game URL" field in http://harness.legendsoflearning.com/

Basically test that pause works, pause in 5 seconds works, try dragging things when you pause in 5 seconds, etc. Try dragging things outside the iframe, etc.

@jbphet
Copy link
Contributor Author

jbphet commented Jul 27, 2018

I tested today using the test harness URL provided above and used localhost links from master for the sims. I tested all 3 sim variations, and all loaded, and the "Pause" function worked. I didn't see any control that said "Pause in 5 Seconds", so I don't know if this has gone away or if it's a bit of a hidden option and I don't know how to find it. At any rate, since the pause functionality worked, I'm going to mark this as fixed and will get QA to test it as part of the deployment process.

For the record, I tested this using the following URLs for the "Enter Game Source URL" field in the Legends of Learning test harness:

@KatieWoe
Copy link
Contributor

KatieWoe commented Sep 18, 2018

@jbphet do you want this issue closed?
verified on 1.1.0-rc.1

@jbphet
Copy link
Contributor Author

jbphet commented Sep 18, 2018

@KatieWoe - please note if it's been verified to work and, if so, on what release, and then clear the "fixed-pending-testing" label but leave it open. I'll make a pass through after deployment and close everything that we leave in the "fixed-pending-deployment" state.

@jbphet jbphet assigned KatieWoe and unassigned jbphet Sep 18, 2018
@KatieWoe KatieWoe removed their assignment Oct 23, 2018
@jbphet
Copy link
Contributor Author

jbphet commented Oct 29, 2018

Published in v1.1.0. @samreid - does anyone need to be notified of this? If not, feel free to close.

@samreid
Copy link
Member

samreid commented Oct 29, 2018

@arouinfar may be a good contact for LOL, reassigning.

@samreid samreid assigned arouinfar and unassigned samreid Oct 29, 2018
@arouinfar
Copy link
Contributor

Since we decided to deliver only SOMB (see #211), I'll mark this as on-hold until SOMB is redeployed.

@arouinfar
Copy link
Contributor

@jbphet just wanted to check in on this issue. SOM added LoL support in the 1.1 branch, but the latest SOMB is still on 1.0, so it doesn't have LoL support yet, correct?

@jbphet
Copy link
Contributor Author

jbphet commented Mar 8, 2019

Yes, that's correct. A while back it was decided that redeploying SOMB wasn't a priority at the time, and we haven't revisited it since then. If it's important to get out it, please let @kathy-phet and @ariel-phet know, and we'll get 'er done.

@jbphet jbphet removed their assignment Mar 8, 2019
@arouinfar
Copy link
Contributor

Thanks @jbphet. I was just doing a bit of GitHub hygiene and noticed the latest version of SOMB was dated a week or so after my last comment, so I wanted to be sure that that wasn't the version that was supposed to be delivered.

@jbphet
Copy link
Contributor Author

jbphet commented Mar 11, 2019

@arouinfar - no, the version that is currently live, which is v1.0.7, built on Nov 6 2018, does not include LoL support. The date reflects a maintenance release that was done to fix issues with full screen behavior on iOS devices.

@jbphet
Copy link
Contributor Author

jbphet commented Mar 14, 2019

A separate issue has now been created for publishing States of Matter Basics with LoL support, please see phetsims/states-of-matter-basics#19. Closing this one.

@jbphet jbphet closed this as completed Mar 14, 2019
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

4 participants