Mixedrealitytoolkit-unity: Global event listeners and speech input handlers

[Bazilikum]

Does this affect the legacy HoloToolkit (master) or the Mixed Reality Toolkit (mrtk_release)?
HoloToolkit 2017.4.1.0

Describe the bug
I'm not sure this is a real bug, but I would like to discuss this issue.
A speech global listener that is also a button will be a global listener also for all click events.

To Reproduce
Create an object that has a component which implements IInputClickHandler.
Attach a SpeechHandler to it and set it to be a global listener.
The click handler is also global and responds to any click in the scene.

Expected behavior
Be a global speech handler but not a global click handler.

Actual behavior
Object responds to all events globally.

Unity Editor Version
2018.2.2f1

Mixed Reality Toolkit Release Version
HoloToolkit 2017.4.1.0

Additional context
When a button is required to also have a voice command to trigger it, and the button might be instanced during the runtime of the scene.
We solved the problem by separating the global speech handler, making it a child of the button and connecting the speech events to the triggering of the button.
This solution is a bit messy.
It also makes me wonder about how events are handled in relation to focused objects and if it is the responsibility of the event system or the event receiver to check if the object is being focused.
If I'm not mistaken usually in unity if you want to check for a mouse click or keyboard status its usually the object's responsibility to check "focus".
I would love to hear your opinion and why you chose the way you did.

[PJBowron]

Unfortunately, by adding itself as a global listener all a component does is simply say: "give me first crack at ALL events from now on!"
A lot of things believe they should be global listeners when actually they'd operate more considerately as fallback handlers :)

[StephenHodgson]

Yeah, and unfortunately there's not a way to specify a _specific_ component as the event is routed via GameObject reference.

[Bazilikum]

Thank you guys for the information and help!
I'm wondering what is the benefit of a system that is designed this way?
Is there a way for us to change it or propose something to mitigate this?
Thanks again!

[Bazilikum]

Based on this issue, I would be interested to hear your opinions about an update to the event system where global listeners are seperated by the handler type, so an object can be a global listener on one handler and not on another (for example voice commands vs. click).
As far as I understand how the system is designed we could update the global listeners list to be a dict where the key is the type.
Is that something you people believe makes sense?
Thanks!

[StephenHodgson]

It's not our event system, it's Unity's. It's the event system that they use to drive the UI.

imo, the best practice here is to add a new GameObject as a child or sibling and add the components to each.

I am researching ways to be able to target specific components but it would still be expensive as it'd incur a GetComponent<T>() call.

[PJBowron]

By 'event system' I think the OP is probably talking about the toolkit's handling of the raised events, but yes that is essentially a light wrapper around Unity's event system.

I think this comes back to the earlier point that components registering themselves as Listeners/Handlers do so by GameObject (which they must do under the Unity system), and whether it might be beneficial to change to component registration in vNext. And that's not to say abandon the current implementation entirely: rather just provide a bespoke alternative to ExecuteEvents.Execute & .ExecuteHierarchy, though I can't imagine it would be a simple as that :)

Returning to the OP's original example, I was recently reminded that each event's eventData carries with it the source that raised the event, which the handling components could examine to determine whether or not they should activate.

[Bazilikum]

Correct me if Im wrong, but registering for global events is happening in the InputManager class to a local list of GameObjects (globalListeners List).
This list is then iterated locally in the HandleEvent template.
Why cant we index the global listeners in the local list by handler type, and in the HandleEvent template function index the global listeners based on event type.

From the InputManager toolkit script:

// Send the event to global listeners
for (int i = 0; i < globalListeners.Count; i++)
{
    // Global listeners should only get events on themselves, as opposed to their hierarchy.
    ExecuteEvents.Execute(globalListeners[i], eventData, eventHandler);
}

[StephenHodgson]

I'll see what I can do.

Maybe register them my UnityEngine.Component instead of GameObject?

I think it'll work as long as we can easily get the game object reference from that base component type.

[StephenHodgson]

Returning to the OP's original example, I was recently reminded that each event's eventData carries with it the source that raised the event, which the handling components could examine to determine whether or not they should activate.

Yup and in vNEXT we added an additional check to make sure the input action is the same as well.

[Bazilikum]

Thanks @StephenHodgson! Please let me know if there is anything I can help with.

[StephenHodgson]

Okay the first problem I ran into was we might get double events called on Global Handlers that are registered twice. If we register as a Component, it's possible that there could be more than one on a single game object.

[PJBowron]

Sorry, meant to reply earlier, and now my thoughts have blurred a little!

The problems you mention sound... okay to me, actually. Generally we don't put more than one of the same component on an object. Similarly GlobalListeners should be self contained systems and perhaps shouldn't exists in multiples?

[Bazilikum]

Sorry for my late reply. SOmehow missed this notification about the subject...

Okay the first problem I ran into was we might get double events called on Global Handlers that are registered twice. If we register as a Component, it's possible that there could be more than one on a single game object.

Im not sure I understand. If I have a component, and I add "this" component instance to the list, why do I care about other similar or not components?
If I have a compoenent that listens to speech input globally, and is registered as a global listener, why would other components care? Same as when you register a game object, other game objects are not affected.

[StephenHodgson]

Okay I think I should make a few things clear here.

Global Listeners -> Input where focus/gaze is not required

You can have multiple input handler components on a single GameObject.

If you register a single GameObject to receive events even tho it's not focused. A single component could have lots of handler interfaces or just one or two. (Ideally you'd likely want to implement them in a way that makes sense for related functionality). That event gets fired on the single GameObject and the event will go to any handler that has that interface implemented.

If we were to register by component, then we would fire the same event on the same GameObject more than once, which is not desired. It matters because the Global Listeners don't check if the event has been consumed or not. As a Global Listener it is expected that you'll get an event even if it's been consumed.

Now if you'd like to send an event to a GameObject that doesn't have focus you can use the modal input stack, which lets you push a GameObject (that may or may not have focus). This modal input will also check to make sure that the event was not consumed.

Overall I think the way it's currently setup works great. To kinda come full circle on the topic of this thread, you'll want to create a separate GameObject with a "focused" handler and a different GameObject with a "unfocused" handler that is registered as a global listener. You can still wire these up with a common parent GameObject, but likely you'll need to keep them on separate GameObjects so the global "unfocused" event isn't fired to the "focused" handler.

[PJBowron]

This is some great detail on how the events are processed at the moment. What's being proposed now though is to change input handling to not route events to a GameObject and thence to whatever components are found on that object; rather, to iterate through registered components and route the event only to those components.
As I mentioned in my second comment above, we would not be able to use ExecuteEvents.Execute & .ExecuteHierarchy any more; and would need bespoke event routing.

[StephenHodgson]

Just going to reiterate that itsy not possible to route events to specific components. Only specific GameObjects. Also this is Unity's event system, so that would be something we would have to ask unity to change.

[keveleigh]

If we were to register by component, then we would fire the same event on the same GameObject more than once, which is not desired.

I'm not sure I understand this statement. What specific steps would fire the same event twice? Global listeners only get events once regardless.

[StephenHodgson]

I did a quick test to see how feasible it would be to register the global listeners by component instead of GameObject.

When multiple components with the same GameObject were registered they would fire the event twice on the same GameObject because Unity's Event Messaging system sends the events via GameObject.

ExecuteEvents.Execute<ICustomMessageTarget>(targetGameObject, null, (x,y)=>x.Message1());

[Bazilikum]

@StephenHodgson thanks for the clarification, I am aware of how the event system works, and that's why its causing my problem. We also solved the problem exactly how you suggested when we 1st encountered it. Yet I still believe you are misunderstanding what I wish to do.
@PJBowron I believe you misunderstood what I meant as well.
What I wish to change is how we register and invoke global listeners. I do not wish to change the whole event system.
I wish to change the way global listeners are stored. From a list of game objects, to a dictionary of .
this way a global listener will only invoke the game object if it matches the event type that it registered as a global listener for.

[Bazilikum]

I personally find it hard to believe that global listeners should be "only handler component" on game object by design. Why is that intended behavior? why should I split my hierarchy to 2 objects when I wish to have them trigger the same thing on the same object?

[PJBowron]

I think there may be some crossed wires somewhere 😄 Let me try and get us all on the same page (updated to included Bazilikum's latest):

The current implementation:

• is using Unity's event system, which we can't expect them to change for us;
• uses Unity's ''ExecuteEvents.Execute'', etc, to route events to GameObjects;
• cannot route an event to a component, even if we register listeners by component, due to the syntax requirements of the ExecuteEvents methods;

What I am suggesting is a slightly modified implementation for vNext, which:

• retains the majority of the current event implementation in terms of how events get raised and sent to HandleEvent for processing;
• does not use ExecuteEvents.Execute, etc, to route events;
• stores listeners/handlers as components rather than GameObjects;
• for a given event, iterates the component list and obtains the interfaces they implement, routing the event directly to those components in an appropriate way;

I think that covers everything. If there's something I've misrepresented or a reason that we couldn't swap the use of the ExecuteEvents methods for something bespoke, please help me out! 😄

[StephenHodgson]

does not use ExecuteEvents.Execute, etc, to route events;

I'm fairly certain that will break the uGUI support.

Also I'm not sure we can get rid of that. It's essentially how the whole thing works so nicely without allocations or the use of GetComponent<T>

[StephenHodgson]

I personally find it hard to believe that global listeners should be "only handler component" on game object by design. Why is that intended behavior? why should I split my hierarchy to 2 objects when I wish to have them trigger the same thing on the same object?

I'm only saying that it's a limitation of Unity's event system. And that's not _always_ the case.

You'll only want to use two game objects if you're mixing and matching focused vs unfocused event handlers (I.e. registering as a global listener or not).

Only put handlers that don't require focus on their own GameObject and handlers that require focus on a different one.

You can put as many handlers of the same focus requirement on the same GameObject.

[Bazilikum]

Hey guys,
I am sorry for this confusion, but it seems I am doing a terrible job of explaining my proposal, as I have the feeling that either i am missing something extremely obvious, or I have failed explaining myself.
I'm gonna start with writing a list of asumptions on my side, please let me know if any of them are wrong:

1. InputManager.cs is part of the toolkit and maintained by it.
2. Global listeners are stored in a list in the InputManager.
3. Global listeners are called using the template function HandleEvent which is declared in the inputManager.
4. ExecuteEvents.Execute(...) is called inside the InputManager HandleEvent function on the whole list of GlobalListeners for any event.
5. Its up the the implementation on the GlobalListener anyway to decide if it marks the event used or not (based on the check in the HandleEvent function after calling the event on all the global listeners to check for used)

Please let me know if any of those assumptions are wrong.

[PJBowron]

Okay so far...

[Bazilikum]

Awesome, thanks! :)
So if we look at the 2 parts that are related to global listeners that are part of the toolkit:

1. Registering and Removing

public void AddGlobalListener(GameObject listener)
{
        globalListeners.Add(listener);
}

public void RemoveGlobalListener(GameObject listener)
{
        globalListeners.Remove(listener);
}

1. Calling

for (int i = 0; i < globalListeners.Count; i++)
{
    // Global listeners should only get events on themselves, as opposed to their hierarchy.
    ExecuteEvents.Execute(globalListeners[i], eventData, eventHandler);
}

Can we make the GlobalListeners list to a Dictionary of and then register the global listeners (part 1) as an object list based on event type?
Then when we call them (part 2), we only call the relevant gameobject based on the event type.
It will require a bit of maintenance on the global listeners dictionary but might perform better as we wont iterate on all global listeners at every event.

[StephenHodgson]

1. is still a bit unclear for me.

[StephenHodgson]

Do you have a code snippet for us to see of the possible change?

[PJBowron]

does not use ExecuteEvents.Execute, etc, to route events;

I'm fairly certain that will break the uGUI support.

@StephenHodgson That's Unity's UI? Is that not handled differently? If I hover over or click a unity button I don't hit a breakpoint set in HandleEvent. Should that happen?

[StephenHodgson]

With the mouse or with motion controllers?

[PJBowron]

Can we make the GlobalListeners list to a Dictionary of and then register the global listeners (part 1) as an object list based on event type?
Then when we call them (part 2), we only call the relevant gameobject based on the event type.

@Bazilikum Yeah you lose me a little here too 😄

Short answer is yes, they could be stored/registered like that, but you're saying you would still trigger the (admittedly shorter) list of matched gameobjects using ExecuteEvents.Execute on each? What would this gain?

Even implemented like this, if you had two click handlers on an object, one focusable, one global, and you called Execute on the gameobject because it was registered in the global list for that event type, Execute would still send the event to both click handlers.

[StephenHodgson]

Execute would still send the event to both click handlers.

Yep, that's what I found out when I tried to prototype it out real quick.

[Yoyozilla]

Bulk closing older HTK bug. Please reopen if you can repro this issue in MRTK v2.
Thanks!
Yoyo