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

Split angularSensibiliy into X and Y for additional functionality #683

Merged
merged 3 commits into from Sep 8, 2015

Conversation

Projects
None yet
2 participants
@Remwrath
Contributor

Remwrath commented Sep 6, 2015

redid change in type script to allow flipping of rotation for individual or both axis and ability to have different rotation speeds for the different axis.

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Sep 6, 2015

Contributor

Before validating this PR, we need to keep angularSensibility as well. It could just be a set/get that updates your values. We cannot remove it because of backward compatibility

Contributor

deltakosh commented Sep 6, 2015

Before validating this PR, we need to keep angularSensibility as well. It could just be a set/get that updates your values. We cannot remove it because of backward compatibility

@Remwrath

This comment has been minimized.

Show comment
Hide comment
@Remwrath

Remwrath Sep 6, 2015

Contributor

so angularSensibility should be private with get and set so on set it will use setter to set angularSensibilityX and angularSensibilityY but on get im not sure what should happen for example if you get angularSensibility when angularSensibilityX and angularSensibilityY are different values then which do you return or should you set them the same as angularSensibility and then return that, which will work but may seem confusing to a deveoper?

Contributor

Remwrath commented Sep 6, 2015

so angularSensibility should be private with get and set so on set it will use setter to set angularSensibilityX and angularSensibilityY but on get im not sure what should happen for example if you get angularSensibility when angularSensibilityX and angularSensibilityY are different values then which do you return or should you set them the same as angularSensibility and then return that, which will work but may seem confusing to a deveoper?

@Remwrath

This comment has been minimized.

Show comment
Hide comment
@Remwrath

Remwrath Sep 6, 2015

Contributor

Also is there a way to mark angularSensibility as depreciated to it can be removed at a later stage?

Contributor

Remwrath commented Sep 6, 2015

Also is there a way to mark angularSensibility as depreciated to it can be removed at a later stage?

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Sep 7, 2015

Contributor

get (as property get in typescript) should return the biggest for instance and should log an warning as deprecated.
set should affect both to same value

Contributor

deltakosh commented Sep 7, 2015

get (as property get in typescript) should return the biggest for instance and should log an warning as deprecated.
set should affect both to same value

@Remwrath

This comment has been minimized.

Show comment
Hide comment
@Remwrath

Remwrath Sep 7, 2015

Contributor

sorry for all the back and forth, hope ive covered your requests now

Contributor

Remwrath commented Sep 7, 2015

sorry for all the back and forth, hope ive covered your requests now

@deltakosh

This comment has been minimized.

Show comment
Hide comment
@deltakosh

deltakosh Sep 8, 2015

Contributor

Good to go :)

Contributor

deltakosh commented Sep 8, 2015

Good to go :)

deltakosh added a commit that referenced this pull request Sep 8, 2015

Merge pull request #683 from Remwrath/develop
Split angularSensibiliy into X and Y for additional functionality

@deltakosh deltakosh merged commit dc93c91 into BabylonJS:master Sep 8, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Remwrath Remwrath deleted the Remwrath:develop branch Sep 8, 2015

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