-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
move scene logic to components (fixes #239, #759, #886) #776
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
Conversation
38c05f2 to
d87c45f
Compare
src/core/a-scene.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fullscreen events should be ignored when vr-mode is forced to VR? Stereo rendering resets to mono when exiting fullscreen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed my updated changes to this PR. Moved fullscreen stuff to https://github.com/aframevr/aframe/pull/776/files#diff-344cbe83c2f901ba50d1f8824d89ccfeR18
So you want the use case to always force stereo and completely ignore fullscreen events?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, debatable I suppose. my thought was that if you are going to manage your own viewport rendering, then you would want to also manually handle fullscreen events as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused. Are you suggesting that if vr-mode is forced to VR, then ignore fullscreen events? And how should we allow vr-mode being forced to VR?
667c838 to
cf91fb8
Compare
cf91fb8 to
8ee9942
Compare
|
Okay after what must be a good week of working this, this is ready. |
src/core/a-entity.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set those to empty string so they're not defined for every entity?
8ee9942 to
7d1ab4c
Compare
|
Updated. Moved fullscreen/metatags/wakelock to modules. Added keyboard-shortcuts scene component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these changes related to the intent of the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah wanted to expose the scene components in the DOM
Scene components:
Scene modules:
Additional changes:
HTMLElement.setAttributeto DOM for default components for scene.render-target-loadedevent on scene for scene to set up renderer.Tested: