Mixedrealitytoolkit-unity: Non-core services have their Enable() function called twice

Created on 11 Jun 2019  路  7Comments  路  Source: microsoft/MixedRealityToolkit-Unity

Describe the bug

Non-core services (such as the WindowsMixedRealityDeviceManager) have their Enable() functions invoked twice.

From what I can see, this was a regression from this PR, which started invoking the registrar:

https://github.com/microsoft/MixedRealityToolkit-Unity/pull/3738

And the registrar would internally also end up calling Enable() on the service as well as part of registration.

To reproduce

Steps to reproduce the behavior:

1) Open the hand interaction example scene.
2) Set a breakpoint on WindowsMixedRealityDeviceManager::Enable()
3) See how it hits twice.

We shouldn't be calling enable twice.

0 - Backlog Bug Services

All 7 comments

@davidkline-ms If we remove this Enable call in RegisterServiceInternal, Enable is only called once (in MRTK's OnEnable).

private bool RegisterServiceInternal(Type interfaceType, IMixedRealityService serviceInstance)
{
    ...

    if (!isInitializing)
    {
        serviceInstance.Initialize();
=====>  serviceInstance.Enable(); 
    }

    return true;
}

It looks like these aren't meant to be called while the MRTK is initializing. But the instance's OnEnable is called after the instance has been initialized. Is this step included to cover situations where you register a service in the middle of a session, well after the instance has been initialized?

@Railboy, that is exactly where i was going to focus my investigation :)

Is this step included to cover situations where you register a service in the middle of a session, well after the instance has been initialized?

I believe this was a side-effect of my attempting to reuse code with the least amount of modifications,

@davidkline-ms I ask because I'm cleaning up instance registration in #4838 and IF there's a straightforward fix, this could easily be folded into that PR as a relevant issue.

@Railboy, feel free to make the change and add me to the PR. I can help validate.

Committed.

fixed by @Railboy

Was this page helpful?
0 / 5 - 0 ratings