search

MixedRealityToolkit / MixedRealityToolkit-Unity

This repository holds the third generation of the Mixed Reality Toolkit for Unity. The latest version of the MRTK can be found here.
BSD 3-Clause "New" or "Revised" License
413 stars 111 forks source link

[BUG] `UGUIInputAdapter` of disabled interactable masking other `UGUIInputAdapters` in hierarchy #561

Open ahmed-shariff opened 1 year ago

ahmed-shariff commented 1 year ago

Describe the bug

UGUIInputAdapter of disabled interactable masking other UGUIInputAdapters in hierarchy

To reproduce

  1. Have two nested interactables with accompanying UGUIInputAdapter
  2. Disable one of the interactables
  3. In-editor try clicking on the interactable not disabled
  4. Expected behaviour OnClicked not being triggered.

Expected behavior

Your setup (please complete the following information)

Target platform (please complete the following information)

Additional context

A potential solution could be to have UGUIInputAdapter add a callback to the StatefulInteractable.OnEnabled/OnDisabled to enable and disable itself when UGUIInputAdapter.thisInteractable is set?

AMollis commented 1 year ago

@marlenaklein-msft can you check if this repros on HL2

AMollis commented 11 months ago

Hi @ahmed-shariff ,

Thank you for reporting this issue or requesting this feature. We appreciate your feedback and interest in MRTK3.

We have reviewed your issue or feature request and confirmed that it is valid. We have added it to our backlog and will prioritize it accordingly.

However, if you would like to speed up the process and contribute to the MRTK3 project, you are welcome to submit a Pull Request with your proposed fix or implementation. We would be happy to review your code and merge it into the main branch if it meets our quality standards.

Please let us know if you have any questions or need any assistance. Thank you for your support and collaboration.

Best regards, The MRTK3 team

ahmed-shariff commented 11 months ago

I started looking into a potential fix for this, well, wasn't as simple as I had initially hoped for. I'll document my thoughts/findings here.

The simple solution would have been to use the StatefulInteractable.OnEnabled/OnDisabled events. But UGUIInputAdapter.thisInteractable is an IXRInteractable. IXRInteractable doesn't have any hooks to indicate when it gets enabled/disabled. Based on going through the source of ugui, as far as I can understand, the only way for an event handler (like the UGUIInputAdapter) to indicate it shouldn't be processed is by disabling itself (see implementation of ExectueEvents .ShouldSendToComponent). Hence, I can't check if the ThisInteractable is enabled and tell the input module to process the next event.