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

Add offset, polar, and interpolate to Point class #764

Merged
merged 4 commits into from Dec 21, 2017

Conversation

Projects
None yet
2 participants
@jschatz1
Contributor

jschatz1 commented Apr 22, 2016

This adds three point methods. I did not see any Point specs so I did not add tests for them. I did run them against the AS3 Point library and got the same results.

Jacob Schatz and others added some commits Apr 22, 2016

Jacob Schatz Jacob Schatz
Add 3 methods to point class with documented comments.
* `offset`: which offsets the Point object by the specified amount.
* `polar` : which converts a pair of polar coordinates to a Cartesian point coordinate.
* `interpolate` : which determines a point between two specified points.
Show outdated Hide outdated src/easeljs/geom/Point.js Outdated
Show outdated Hide outdated src/easeljs/geom/Point.js Outdated
@lannymcnie

This comment has been minimized.

Show comment
Hide comment
@lannymcnie

lannymcnie Apr 25, 2016

Member

Added a few comments on the documentation.

Member

lannymcnie commented Apr 25, 2016

Added a few comments on the documentation.

@jschatz1

This comment has been minimized.

Show comment
Hide comment
@jschatz1

jschatz1 Apr 25, 2016

Contributor

@lannymcnie Thanks I'll fix that up.

Contributor

jschatz1 commented Apr 25, 2016

@lannymcnie Thanks I'll fix that up.

Fix correct method names.
Add parameter names.
@jschatz1

This comment has been minimized.

Show comment
Hide comment
@jschatz1

jschatz1 Apr 25, 2016

Contributor

@lannymcnie Fixed those things up. For optional params I followed the styles you have in Matrix2D.js.

Contributor

jschatz1 commented Apr 25, 2016

@lannymcnie Fixed those things up. For optional params I followed the styles you have in Matrix2D.js.

@lannymcnie

This comment has been minimized.

Show comment
Hide comment
@lannymcnie

lannymcnie May 16, 2016

Member

Thanks! Will chat internally about the additions, and merge them.

Member

lannymcnie commented May 16, 2016

Thanks! Will chat internally about the additions, and merge them.

@jschatz1

This comment has been minimized.

Show comment
Hide comment
@jschatz1

jschatz1 Jul 14, 2016

Contributor

@lannymcnie I guess you guys aren't doing this as this has been stale for a while? I can close if it isn't happening. NBD, I am using a forked version of createjs to do those extra things that are missing. Would be willing to commit those extras too but obviously I don't want to put in the effort if you are not interested.

Contributor

jschatz1 commented Jul 14, 2016

@lannymcnie I guess you guys aren't doing this as this has been stale for a while? I can close if it isn't happening. NBD, I am using a forked version of createjs to do those extra things that are missing. Would be willing to commit those extras too but obviously I don't want to put in the effort if you are not interested.

@lannymcnie

This comment has been minimized.

Show comment
Hide comment
@lannymcnie

lannymcnie Jul 14, 2016

Member

Hey, sorry for the radio silence.

Definitely still open to this PR. Unfortunately we have been pretty busy with production work, and haven't had much time to review stuff like this for CreateJS. When it comes to adding helper functions, we take a pretty conservative approach, but I think this sort of thing is valuable, so it is still on the table :)

Hopefully pushing out a pretty big update in the near future!

Member

lannymcnie commented Jul 14, 2016

Hey, sorry for the radio silence.

Definitely still open to this PR. Unfortunately we have been pretty busy with production work, and haven't had much time to review stuff like this for CreateJS. When it comes to adding helper functions, we take a pretty conservative approach, but I think this sort of thing is valuable, so it is still on the table :)

Hopefully pushing out a pretty big update in the near future!

@jschatz1

This comment has been minimized.

Show comment
Hide comment
@jschatz1

jschatz1 Jul 14, 2016

Contributor

Thanks @lannymcnie. I totally understand! 😄

Contributor

jschatz1 commented Jul 14, 2016

Thanks @lannymcnie. I totally understand! 😄

@lannymcnie lannymcnie merged commit 4582b0e into CreateJS:master Dec 21, 2017

@jschatz1

This comment has been minimized.

Show comment
Hide comment
@jschatz1

jschatz1 Dec 21, 2017

Contributor

Thank you so much @lannymcnie!

Contributor

jschatz1 commented Dec 21, 2017

Thank you so much @lannymcnie!

@jschatz1 jschatz1 deleted the jschatz1:js-offset-polar-interpolate branch Dec 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment