-
Notifications
You must be signed in to change notification settings - Fork 11
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
More wires? #432
Comments
@arouinfar thoughts here? |
Found the commit that first limited wires. Doesn't have a linked issue, but at least gives us a date to look for a discussion: April 27, 2016. 62aa788 |
Issue that requested to bump the number up to 20: #110. Could be that the limit was requested by Stanford? @arouinfar maybe it's in an old email message? |
I think it would be pedagogically useful to increase the number of wires @ariel-phet. The circuits in the teacher's screenshot are quite reasonable, and are likely to be recreated by other teachers. I believe the initial limit may have been performance/Stanford related -- thanks for digging up the issue @phet-steele! I requested 20 wires in #110 because it was double the previous limit, but 20 is not a particularly special number. Perhaps 25 would be a better limit? Talking to @phet-steele, longer wires = more electrons = more expensive (generally speaking). If a student has lots of long wires with electrons turned on, then there would likely be some performance hit on iPad2. That said, I think the added flexibility of increasing number of wires would be be an acceptable trade-off. |
How does that sound @ariel-phet? |
@arouinfar sounds good to me, and my guess is an easy change. |
@samreid can you increase the number of wires to 25? |
Sure, do you want this published right away as a maintenance release? What level of testing do you want, if any? I think doing a textual diff to make sure 20=>25 is the only change, and a spot check could be sufficient. |
@samreid I think it would be nice to publish as a maintenance release. A spot check seems appropriate. |
@mattpen am I cleared to do chipper 1.0 maintenance releases? I know the build server has been in flux lately. Please reassign to me after responding. |
@samreid, I am doing all of my testing for the build-server on ox-dev. A maintenance release should be fine. |
@ariel-phet I'm operating under the assumption that we'll make maintenance releases for both DC and Virtual Lab. Please correct me right away if that's wrong. |
@arouinfar and I discussed a little and perhaps we could try this as a query parameter such as Another option would be to put this option in an options menu. @samreid how difficult would adding such a query parameter be? |
Adding a query parameter to set the max number of wires would be a very easy change. (Easier than adding an options menu). |
Perhaps it would be better to just increase the number of wires permanently, for all users. Now that we no longer support iPad 2, performance may not be an issue. Can we test/investigate that approach before adding query parameters? What is our new slowest supported platform? How many wires do we want? |
@samreid according to phetsims/energy-forms-and-changes#191 (comment), our slowest iPad above the iPad2 is Jemison (iPad 4, iOS 10). I tested the published version of CCK: DC on Jemison, and it ran pretty smoothly. When maximizing the number of wires in the play area, the frame rate dipped to 11FPS, but the sim was still pretty responsive. I didn't feel like dragging components was particularly laggy. Tentatively, it seems like we could increase the number of wires for all users, assuming iPad2 is no longer supported. I still think we'd want QA to test the sim a bit with a larger wire count to be sure. As for the number of wires, I think that it will always be a bit arbitrary. Maybe we could simply double the number to 50? Thoughts @ariel-phet? |
For now, we would like to add a query parameter in master. Then we'll assign to @arouinfar for testing. May do an RC (from master) in March. Call it something like |
Added query string in the preceding commits. @arouinfar can you please test and check performance? |
I tested |
I tested @samreid I think we can safely move forward with |
Based on #432 (comment), I'll unassign for now and put a note in my calendar to re-evaluate mid-March. |
I confirmed I was able to create 50 wires by using the published version https://phet.colorado.edu/sims/html/circuit-construction-kit-dc/latest/circuit-construction-kit-dc_en.html?moreWires Closing. |
I forget how we arrived at the current wire limit (was it a performance concern on iPad 2)?
This user makes a case for circuit comparison:
https://twitter.com/MO_Keeffe/status/971114960699969536
The text was updated successfully, but these errors were encountered: