XRTK / com.xrtk.core

The Official Mixed Reality Framework for Unity
https://xrtk.io
MIT License
308 stars 34 forks source link

New Locomotion System #876

Closed FejZa closed 3 years ago

FejZa commented 3 years ago

XRTK - Mixed Reality Toolkit Pull Request

Overview

This PR introduces the new locomotion system which replaces the old teleport system. This new system is set up in a way that it is extremely easy for users to implement new types of locomotion if needed but also comes with common default implementations for smooth locomotion and teleportation.

Key Features:

https://user-images.githubusercontent.com/9565734/129541455-ae127fe7-d0a1-4590-b6ce-bbf1f6910485.mp4

Changes

Breaking Changes

Related Submodule Changes

StephenHodgson commented 3 years ago

Please double check that all the submodules are pointing to the right commits

SimonDarksideJ commented 3 years ago

Curious, the UltraLeap changes in that Branch? should that be in this PR?

FejZa commented 3 years ago

Nope. I gotta do some clean up here, not sure what happened. I'll let you know.

SimonDarksideJ commented 3 years ago

Bit curious as to the alignment of implementations in base and what is in the SDK.

I was under the impression we were just going to have a single (default) Locomotion provider in the core and all the others would be in the SDK, am I incorrect?

If so, all inspectors and relevant providers in the SDK should also have their inspectors / profile definitions and so on in the SDK.

FejZa commented 3 years ago

We talked about this a while ago. I don't like having to move anything to SDK. All the default implementations are in Core. Especially base classes like BaseLocomotionProvider etc. Because if they weren't, then I couldn't make use of them in Core implementations without creating a dependency on SDK. Which does not make sense.

In general, back then when we talked about it, we agreed to move things to Core so they can work on their own without the SDK.

The only thing living in SDK are the default profiles basically.

FejZa commented 3 years ago

Eversince I first saw MRTK and still to this day, the existence of the SDK module was confusing to me. I just don't get what its purpose is supposed to be and why we have it to this day.

FejZa commented 3 years ago

Oh yeah sorry, Pointers and HotSpot are in SDK as well (which I would love to move to Core tbh) :D

StephenHodgson commented 3 years ago

Pointers and HotSpot

Makes sense to me.

Yeah the general consensus was to try to keep monobehaviours out of core as much as possible for future engine porting. But at this point I don't think it really matters. I think we should just stick to Unity.

FejZa commented 3 years ago

I see. Well I think we can leave it like this for now. And yes I think we should just stick to Unity.

I am really torn in between making locomotion its own package and not.

StephenHodgson commented 3 years ago

I am really torn in between making locomotion its own package and not.

I think it is ubiquitous enough to stay in core

FejZa commented 3 years ago

Renamed hotspots to TeleportAnchor

StephenHodgson commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
FejZa commented 3 years ago

/azp run

azure-pipelines[bot] commented 3 years ago
Azure Pipelines successfully started running 1 pipeline(s).
StephenHodgson commented 3 years ago

Needs to be fixed. It won't pass until the packages are updated I think

FejZa commented 3 years ago

Any idea how I can reset submodules? This PR shows I made changes in Ultraleap and glTF, which does not make any sense. Not sure what happened here.

StephenHodgson commented 3 years ago

I can take a quick look

FejZa commented 3 years ago

Thanks. I'll revert some of the changes now.

FejZa commented 3 years ago

Feedback addressed @StephenHodgson.

StephenHodgson commented 3 years ago

LGTM, hopefully the builds start passing again then we'll merge it in

StephenHodgson commented 3 years ago

@FejZa I think I figured it out. I reverted the gltf submodule changes. It seems to hang the editor when importing models for some reason. I'll look into it.

FejZa commented 3 years ago

Great, thanks. Once the package is published I need to bump SDK dependencies so the build can pass.

StephenHodgson commented 3 years ago

It's already published