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

Feature/scale tool #1122

Closed
wants to merge 3 commits into from
Closed

Feature/scale tool #1122

wants to merge 3 commits into from

Conversation

gfodor
Copy link
Contributor

@gfodor gfodor commented Mar 28, 2019

Adds a scale tool to the menu and also renames the rotate components to xform since it is now used for scaling.

} else {
v.set(0, 1, 0);
const scaleFactor =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main new control flow, everything else here is just renames

@gfodor gfodor mentioned this pull request Mar 28, 2019
Copy link
Contributor

@johnshaughnessy johnshaughnessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -16,12 +21,12 @@ const v2 = new THREE.Vector3();
const q = new THREE.Quaternion();
const q2 = new THREE.Quaternion();

AFRAME.registerComponent("rotate-button", {
AFRAME.registerComponent("xform-button", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer transform to xform. We would have called xforms in the userinput system transforms or transformFunctions but we didn't want to risk confusion with the "position/rotation/scale" interpretation of "transform". Here that IS what we mean, so I think it fits better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah good point. i was trying to save characters since this term is used in many places, but i agree the conflict with the user input concept makes it worth changing.

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

Successfully merging this pull request may close these issues.

2 participants