xr.pointerSelection.getMeshUnderPointer silently fails when uniqueId isn't provided

Bugs
, , ,
1

Hi,

I just spent a majority of a day figuring out why my VR controller pointers always returned null despite setting all meshes to pickable and clearly seeing the selection marker when I realized that I had written

const nullableMesh = xr.pointerSelection.getMeshUnderPointer(inputSource);

instead of

const nullableMesh = xr.pointerSelection.getMeshUnderPointer(inputSource.uniqueId);

Maybe we should add an error for failing to provide the uniqueId? Would save some future headache unless it was intended this way.

2

We can add a dynamic type check: [XR] Add dynamic type check for getMeshUnderPointer argument by carolhmj ¡ Pull Request #13979 ¡ BabylonJS/Babylon.js (github.com)

2 Likes
3

Here is my two cents - you passed the wrong parameter. This is the same as passing anything else other than a string to the name of a mesh.

Using typescript will help here - a string must be passed. if anything else is passed (or a wrong string) it will fail silently. If you ask why it fails silently (and doesn’t throw an exception) - in XR it’s better to continue rendering.

1 Like
4

Yeah I was using a quick .js file for mocking up a scene so it wasn’t .ts. it was also in an observable before render so it might be unwise to add a typecheck that runs every frame.

Maybe we just add lots of big red warnings in the documentation. “IT’LL SILENTLY FAIL LIKE THIS IF YOU DON’T PROVIDE THE RIGHT INPUT” :upside_down_face:

1 Like
5

We ended up talking about the best way to make this user friendly and we added WebXRInputSource as a possible argument type so this:

const nullableMesh = xr.pointerSelection.getMeshUnderPointer(inputSource);

will actually work now :slight_smile:

1 Like
6

still unsure if that’s the ideal solution since it’ll run every frame for people doing something every frame like a hover check over a mesh.

I think one better solution would be to have a method like getMeshUnderPointerRight and getMeshUnderPointerLeft if we know which ones are left hand or right hand. or use numerical index to get the inputSource instead of id? something so that the user doesn’t need to pass in a param would be nice

7

I think ideally there should be a warning given to the user when returning null with the wrong param. this at least shouldn’t affect performance if the user is providing the correct param and the controller is found without issue.

I also added a way to get the controller via index instead. This might be more useful for debugging if you somehow don’t have the id and/or need to just see if the picking is working at all for any controller by checking controller index 0

8

WebXR abstracts controllers deliberately so that it will support any scenario, including a scenario where you have more than 2 controllers. It doesn’t really exist so far, but who knows? :slight_smile:

typeof is fast enough to run it on each frame without losing performance. I am very neutral about the need of that to be honest. but I don’t think it will hurt perf

9

This is similar to the first suggestion by @carolhmj , which I think it a slippery slope :slight_smile:

We have different functions that do that. getMechById in scene, for example. you can pass an object, you can pass a boolean. a null will be returned. do you want to add it there as well?

The fact that js is not checking for types is an issue with the language itself (or not, one might argue). We can’t protect the dev from making those mistakes. TypeScript can.

1 Like
10

I guess it’s the same even if we put that check in the fail block, huh. the problem is it lacks precedent so yeah, if we do it here we should do it everywhere else.

For now, I have the issue solved by just using uniqueId properly so I personally don’t really need the method to start accepting InputSource as params instead. I only suggest adding a by numerical index method so it might be easier to debug in the future then