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

double check descriptions before RC #173

Closed
zepumph opened this issue Sep 13, 2019 · 8 comments
Closed

double check descriptions before RC #173

zepumph opened this issue Sep 13, 2019 · 8 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Sep 13, 2019

@terracoda sorry this issue is coming so late. I think we are now ready for an RC, and I wanted to have you double check the descriptions before going there. This is mainly just to double check that other work in ISLC hasn't changed anything. There has been PhET-iO and some common code work done in this suite of sims in the past month. I don't think much has changed, but I think it would be good for the lead designer to give a brief once over before continuing. Thanks!

@terracoda
Copy link

@zepumph, I'll try to get to this over the weekend.

@zepumph
Copy link
Member Author

zepumph commented Sep 13, 2019

Thanks!

@zepumph
Copy link
Member Author

zepumph commented Sep 17, 2019

I discussed this with @terracoda, and we think that any changes that are found here can easily be propagated to the RC, and that this shouldn't hold up the creation of the RC task.

@terracoda
Copy link

terracoda commented Sep 23, 2019

@zepumph, I have found one issue with the descriptions which will need to also perhaps reference phetsims/qa#429

  1. when masses are the smallests (or set to fixed radius) and closest together, the description is "extremely close to mass 1 (or mass 2)", but I think it its supposed to say "closest to mass 1 (or mass 2)". NOTE that there is a "farthest from mass 1 (or mass 2)".

See design doc: https://docs.google.com/document/d/1HdDG9ds2MdbCb21l9qk3cI8yBQxK6-wBWNXC4Tloji8/edit#heading=h.g72a78v7mdil
Screen Shot 2019-09-23 at 6 23 54 PM

I'm still checking and will post anything else I find.

@zepumph
Copy link
Member Author

zepumph commented Sep 23, 2019

When I set both masses to be 1 billion KG, I do hear the "closet to mass to" clause on the alert. When I have the constant size checked, I don't hear that because the centers aren't 1.3 km away, it is bigger. I can add that in as a case if you would like, but I didn't know that was expected (since they technically can be closer.

What would you like me to do there?

@terracoda
Copy link

Oh, apologies. I thought I tried both cases.
In GFL regular the he masses can get to 1.2 meters, but here in GFL:Basics the closest seems to be 1.3 km. Is it just a matter of that?

Again, I'll check again asap.

@zepumph
Copy link
Member Author

zepumph commented Sep 23, 2019

In GFL regular the he masses can get to 1.2 meters,

In GFL they can get all the way down to .6 meters by the looks of it. The min radius is much smaller in that sim. Likely we will need to reimplement those regions once we get back to GFL.

Anyways, again I am happy to implement whatever behavior you would like, just let me know what's best.

@terracoda
Copy link

@zepumph, you are indeed correct.
The spheres can only reach their "closest" described position (1.3 km apart) with Constant Size unchecked.

With Constant Size checked the spheres can only get to "1.4 km" apart.

My main concern was that the "closest" description gives a nice indication that they can't move any farther (i.e., closer together in this case). That said, the learner's main indication of not being able to go any farther is the alert that says "last stop right (or left)", so the role of the "closer" value is not the only indication of that.

It is only on a deep exploration of mass and distance that a learner will get to the "closest" value. I think that's fine. The role of the described position "closest" is to indicate they have reached the closest possible in the sim, and not that they have reached the max position in the current exploration.

I'll discuss with @arouinfar, to be sure, AND @arouinfar and I can have that discussion in the bigger context of GFL regular and other ISLC sims.

We do not need to make any changes at this point to GFL:Basics.

I did find one missing period in an alert. To not confuse the decision in this issue, I'll close and open a new issue about the missing period.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants