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

User reported issue: radians incorrect at 9pi, -9pi and after #97

Open
oliver-phet opened this issue Sep 19, 2024 · 9 comments
Open

User reported issue: radians incorrect at 9pi, -9pi and after #97

oliver-phet opened this issue Sep 19, 2024 · 9 comments
Assignees
Labels

Comments

@oliver-phet
Copy link

There’s a bug with the angles on Trig Tour. With Special Angles marked and angle in radians, once you rotate clockwise past -8 pi – 5 pi/6, the angle resets to -8 pi instead of becoming -9 pi. The angles after that are incorrect by – pi.

@amanda-phet when I went to verify, I noticed 9pi has the same issue. But the sim self-corrects at 10pi. At 17pi and 18pi, the radians are off by 1pi, but the sim self-corrects at 19pi. Then at 21, 23, 27, 31, 34, 35, 36, 39. Same in the negative direction: -9, -17, -18, -21...

@amanda-phet
Copy link
Contributor

Yes this seems to be a bug at 9π.

@amanda-phet amanda-phet removed their assignment Nov 21, 2024
@jessegreenberg jessegreenberg self-assigned this Nov 21, 2024
@jessegreenberg
Copy link
Contributor

I was able to observer this, Ill take a look. The numerical value is correct, so it is just about the fraction readout.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Dec 5, 2024

We are going to change this representation in #80, so any changes here may overlap with that one.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Dec 6, 2024

The halfTurnCount in the model is one less than it should be. It is calculated here:

// set turn counts and angles
this._fullTurnCount = Utils.roundSymmetric( ( targetAngle - remainderAngle ) / ( 2 * Math.PI ) );
remainderAngle = targetAngle % ( Math.PI );
this._halfTurnCount = Utils.roundSymmetric( ( targetAngle - remainderAngle ) / ( Math.PI ) );
this.fullAngleProperty.value = this.constrainFullAngle( targetAngle ); // now can trigger angle update
this.previousAngle = smallAngle;

When it works well, the remainderAngle is 0. But in all the cases where this is broken, the remainderAngle is exactly Math.PI.

@jessegreenberg
Copy link
Contributor

OK, I believe this is fixed. @amanda-phet can you please verify? @oliver-phet thanks for the testing and list in #97 (comment) that helped identify the problem, and I made sure each value you found is working.

@jessegreenberg
Copy link
Contributor

Well, I guess this behavior is going to change with #80 so I maybe this should be on hold.

@jessegreenberg
Copy link
Contributor

#80 is ready for review, is this working as expected after the change in #80?

@jessegreenberg jessegreenberg removed their assignment Dec 9, 2024
@amanda-phet
Copy link
Contributor

This is working as expected. Thank you!

@oliver-phet do you want to reply to that user and let them know a fix will be published, and the representation is also changing?

@oliver-phet
Copy link
Author

I think it's best to communicate to the user once the fix is published (unless that is a long ways out). Can this issue be kept open and assigned to me once it is published?

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

No branches or pull requests

3 participants