search

provencher / MRTK-Quest

MRTK-Quest: Mixed Reality Toolkit (MRTK) extension bridge for Oculus Quest + Rift / S
Other
389 stars 67 forks source link

Use standard teleport #59

Closed machenmusik closed 4 years ago

machenmusik commented 4 years ago

This PR modifies MRTK-Quest teleport support to leverage the existing teleport pointer and system.

provencher commented 4 years ago

I’m going to take some time to look at this more closely soon. However, please revert deleting the custom teleport handler.

I think it serves as a useful educational tool, and this PR doesn’t need to completely wipe it away to work.

provencher commented 4 years ago

Im hesitant to take the default teleport pointer wholesale for a few reasons.

Beyond re routing input, I also added ray stabilization, improved the feel when pointing downward, and made it possible to teleport outside of hotspots. In addition to that I visually polished the teleport pointer and added audio, which I think add a lot to the experience.

With all that said, I think there’s stil value in having a custom teleport pointer that is more modernized visually with additional accommodation for hands. Yet having custom controller definitions for these controllers is a more correct approach. I’m open to seeing how to port these changes to the official pointer using the correct systems, I think that will take time however.

I’d appreciate if it were possible to configure MRTK Quest to choose between the behavior currently in master, and the built in teleport pointer. That could be a good middle ground until we sort this out more properly.

Thanks for your contribution!

machenmusik commented 4 years ago

I'm happy to keep the custom teleport in, but I am not sure whether the custom teleport can be invoked the same way to allow the choice (i.e. whether it will function properly if we had a CustomParabolicPointer for articulated hands that used custom teleport). Let me see what I can do.

machenmusik commented 4 years ago

OK, the custom teleport is back in but dormant; I'm not sure what you had in mind w.r.t. how to choose between, so I'll leave this PR here.

provencher commented 4 years ago

Cheers! When I get the chance I’ll try and make the custom teleport pointer work through the proper system.

Going to leave this PR open for now. Appreciate you taking the time to work on it.

In the mean time I hope conversations continue with MRTK to get this to work officially for all articulated hands.

machenmusik commented 4 years ago

Great.

The one thing lacking was a way to turn off custom teleport without turning off the entire teleport system, since you can't unassign the custom pointer from articulated hands as is doe for the stanard teleport; you may want to consider having a setting allowing for standard, custom (which maybe you should call improved and see whether they want to adopt upstream in MRTK), or off.

provencher commented 4 years ago

@machenmusik Im going to create an enum with the teleport mode choices. I'll merge this into a branch and work from there.

provencher commented 4 years ago

@machenmusik here I created a new PR on this topic https://github.com/provencher/MRTK-Quest/pull/60