tizzle / aframe-orbit-controls-component

An Orbit Controls Component for A-Frame VR
https://tizzle.github.io/aframe-orbit-controls-component/
MIT License
74 stars 25 forks source link

option to automatically switch to look-controls in VR mode #13

Closed morandd closed 7 years ago

morandd commented 7 years ago

Added an option (default:true) to automatically switch to look-controls when the user enters VR mode with a HMD.

IMHO this is an intuitive and default way to handle VR mode. Folks can set autoVRLookCam:false if they prefer to handle the camera in VR differently. But as it stands, orbit-cam doesn't work in VR.

I also made a camera juggler to handle going in/out of VR mode on desktop and mobile https://github.com/morandd/dans-aframe-camera-juggler

but with this change, this juggler is not necessary.

tizzle commented 7 years ago

Hey @morandd,

thanks for the submission. I looked at the changes and think it's a very good idea.

I have one question though, that stems mostly from not having built many VR applications, yet. Is the look-controls the only choice people go to when doing VR experiences? I looked into the Aframe docs and there are a few more options, like hand-controls, oculus-touch-controls, vive-controls...

I fear if we hardwire the look-controls we exclude other controls for VR. Maybe we should think about something a bit more modular, like injecting the desired controls as a string, e.g. "look-controls" or "hand-controls" to use them when switching to VR.

When thinking about auto-creating controls that are not already present on the entity i also feel we might run into an issue in the future. Hardwiring the creation in the orbit-controls-component rids us of the possibility to add individual properties (like reverseMouseDrag on look-controls) to the controls.

Maybe we can go for something like this and simply output a warning when the controls referenced in autoVRControls are not present on the entity:

<a-entity
  id="camera"
  look-controls="
    enabled:false;
    reverseMouseDrag: true;
    "
  orbit-controls="
    enabled: true;
    target: #target;
    enableDamping: true;
    dampingFactor: 0.125;
    autoVRControls:"look-controls";
    "
/>

What do you think?

Cheers

Till

morandd commented 7 years ago

Is the look-controls the only choice people go to when doing VR experiences? AFAIK, yes: look-controls seems to be the only camera controller that listens to HMD orientation inputs. The hand controller is for the handheld input devices.

Hardwiring the creation in the orbit-controls-component rids us of the possibility to add individual properties

I'd code it so that if a look-controls already exists, we use those settings. I think I had written this in the README and maybe also in the example. But adding a console.warn is also a nice idea.

You're right that auto-creating a controller is a bit dangerous territory. And I agree this may have to change in the future. That's also why I added this entire behaviour controlled by a boolean flag that defaults to false. However I think this look-controls is obvious and desirably in the majority (90+%?) of cases. In those cases when it's not desirable, folks can just leave the boolean flag at it's default false value and write their own camera juggler (as I did with github.com/morandd/dans-aframe-camera-juggler)

Thanks, dan

morandd commented 7 years ago

Ah, sorry - I made the option default=true. TBH I think this is a quite reasonable default, so that orbit-controls works out of the box. However we could change the default to false to build in some future-proofing.

tizzle commented 7 years ago

Hey @morandd,

i had a look at all the options in aframe again and it seems the look-controls are the one and only goto place for VR. I accepted your PR as is and will look into the rotation/look direction on switch issue that can cause the POI to be in your back when switching to VR.

I will also add a new example showcasing this feature and then bump npm to the new version.

If any alternatives to look-controls arise one day, i have also built a more dynamic version of this, that is of no use atm but might be in the future. I will stash this for now.

Thanks for your input!

Cheers

Till