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

Fix #1880: setOrientation description #1885

Merged
merged 1 commit into from May 16, 2019

Conversation

rtoy
Copy link
Member

@rtoy rtoy commented May 15, 2019

  • Apply simple fix to setOrientation and setPosition paragraphs
    • Add some very simple descriptive text for the parameters of the methods
    • Add links to the method parameters in the method descriptions.

Preview | Diff

  * Apply simple fix to setOrientation and setPosition paragraphs
  * Add some very simple descriptive text for the parameters of the methods
  * Add links to the method parameters in the method descriptions.
@rtoy rtoy added this to In PR Review in V1 via automation May 15, 2019
@rtoy rtoy requested a review from hoch May 15, 2019 17:18
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

LGTM with few questions

<b>front</b> vector represents which direction the person's
nose is pointing. The <b>up</b> vector represents the direction
{{AudioListener/setOrientation()}} describes which direction the listener is pointing in the 3D
cartesian coordinate space. Both a [=front=] vector and an
Copy link
Member

Choose a reason for hiding this comment

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

I guess [=front=] is defined above?

{{AudioListener/setOrientation()}} describes which direction the listener is pointing in the 3D
cartesian coordinate space. Both a [=front=] vector and an
[=up=] vector are provided. In simple human terms, the
<dfn dfn>front</dfn> vector represents which direction the person's
Copy link
Member

Choose a reason for hiding this comment

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

Or is this the definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is the definition. Kind of bad to have a forward definition, but I didn't want to restructure everything, and just made explicit links between uses and the definition.

@rtoy rtoy merged commit 99fe322 into WebAudio:master May 16, 2019
V1 automation moved this from In PR Review to Done May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
V1
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants