Closed saitonakamura closed 1 year 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 |
@CodyJasonBennett I'd really appreciate any feedback on this
I'm happy with the premise. Will give this a deeper review later today.
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-R103And
Controllers
now directly renderXRControllerModel
So the flow now is
XR
creates a pair ofXRController
and subscribes toconnected
/disconnected
events from three.jsXRController
callxr.getControler
and others in constructor therefore creating aWebXRController
instances in three.js. BothWebXRController
and it'sXRController
counterparts will now be present for the lifetime of an appWebXRManager
react toinputsourceschange
and populates it's state and dispatchesconnected
eventconnected
event is handled byXR
and it populatesstate.controller
with correspondingXRController
handleControllerModel
react toref
and callsmodelFactory.initializeControllerModel
which does all the 3d/profiles heavy lifting as previousPreviously, 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