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

Make Listening for Events section inclusive of custom controllers #4900

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

evansjohnson
Copy link

Description:
As I understand, this section is really about listening for controller events, which won't necessarily be for buttons and axes. On an initial read through the docs, I had just read about creating custom controllers, so felt it may be more clear to make this section inclusive of the custom controllers just covered. Open to any suggested edits, I believe my statements are backed up from what I've seen so far but am generally unfamiliar with the library so need to be checked!

At a minimum, I'd like to fix up the "For each button, every time a button is pressed down, released, or for some cases, even touched." sentence fragment!

Changes proposed:

  • Make section about listening to controller events, not specific to A-Frame controllers
  • Link tracked-controls-component instead of relisting events

…om controllers

As I understand, this section is really about listening for controller events, which won't necessarily be for buttons and axes. On an initial read through the docs, I had just read about creating custom controllers, so felt it may be more clear to make this section inclusive of the custom controllers just covered. Open to any suggested edits, I believe my statements are backed up from what I've seen so far but am generally unfamiliar with the library so need to be checked!

At a minimum, I'd like to fix up the "For each button, every time a button is pressed down, released, or for some cases, even touched." sentence fragment!
event handlers how we want:
## Listening for Controller Events

Controllers may have many buttons, many axes, and may emit many events. The
Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Some alt rewording?

The events emitted by the vendor specific controllers (e.g Quest, Index...) map the lower events emitted by tracked-controls component to the specific buttons of each physical controller. To handle input events, we look for the event name...

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the suggestion! I think it really helped to break away from the original wording there. I did one more round, happy to keep iterating if something feels off.

Notes on changes to suggestion:

  • this is for the A-Frame and vendor specific controllers
  • when I see "map" I'm thinking about mapping over an array and it's not necessarily a 1:1 transformation so felt "build on the base level events" captured this a bit better
  • "specific buttons" -> "specific events" - not limited to buttons
  • "each physical controller" -> "each controller" - I believe this refers to the "controllers" earlier in the sentence which are the software a-frame controllers and may be levels of abstraction away from a physical controller.
  • To handle input events -> To handle events - I would imagine controllers don't have to exclusively emit input events. Ex a controller "random-interval" that emits a signal at a configurable random interval to power some dynamic content.

Copy link
Member

Choose a reason for hiding this comment

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

Not 100% if talking about tracked controls helps the avg user. It should be an implementation detail. Probably more clear to keep it high level. As you mentioned the section create custom controllers exposes lower level detail. Maybe including a link to that section would be enough for those that want to dig deeper?

Copy link
Author

Choose a reason for hiding this comment

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

good point, I updated to make it a footnote for the section. will let you decide if it's worth the backlinks, they do link to anchor tags on the same page so perhaps this is trying to do too much and is noisy but I found myself looking for this info when I read this section

@evansjohnson
Copy link
Author

thank you for your work with this library @dmarcos! it was really cool reading about how it builds on HTML and the event system already built into browsers

this is ready for another look if you get a chance and want to get it merged

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