search

pmndrs / react-xr

šŸ¤³ VR/AR with react-three-fiber
https://npmjs.com/@react-three/xr
MIT License
1.98k stars 137 forks source link

refactor(controllers): Refactored how Controllers interact w/ three #294

Closed saitonakamura closed 11 months ago

saitonakamura commented 11 months ago

This a big refactor of controller and how they interact with three counterparts. The flow of events was very confusing, to say the least. This PR makes this flow much direct and does some other notable changes.

ControllerModel abstraction is removed

It's responsibilities are moved to Controllers, mainly to https://github.com/pmndrs/react-xr/pull/294/files#diff-f17a5c6c1d801f88dd02bbbcaacc32ee99c5630535d1db5f3d5527ddd84b89eaR85-R103

And Controllers now directly render XRControllerModel

So the flow now is

  1. XR creates a pair of XRController and subscribes to connected/disconnected events from three.js
  2. XRController call xr.getControler and others in constructor therefore creating a WebXRController instances in three.js. Both WebXRController and it's XRController counterparts will now be present for the lifetime of an app
  3. three.js WebXRManager react to inputsourceschange and populates it's state and dispatches connected event
  4. connected event is handled by XR and it populates state.controller with corresponding XRController
  5. handleControllerModel react to ref and calls modelFactory.initializeControllerModel which does all the 3d/profiles heavy lifting as previous

Previously, the 5th step was actually another multi-step flow which involved sending fake events and such

XRControllerModel moved to a separate file from XRControllersModelFactory

No notable changes inside it

InputSource on XRController can now be null

Now inputSource will become null if controller is disconnected, no real reason to hold on to outdated inputSource. This resulted in a bunch of null checks here and there, but nothing substantial

Whole bunch of unit tests added

We're still far from good coverage, but this is a big step forward

codesandbox-ci[bot] commented 11 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4103966bd74d59357606f167d9e74e125d316b95:

Sandbox Source
examples Configuration
saitonakamura commented 11 months ago

@CodyJasonBennett I'd really appreciate any feedback on this

CodyJasonBennett commented 11 months ago

I'm happy with the premise. Will give this a deeper review later today.