Mixedrealitytoolkit-unity: Dependency Injection vs Singletons

[StephenHodgson]

Let's start a conversation about using Dependency Injection in place of our current Singleton Pattern.

See https://github.com/Microsoft/MixedRealityToolkit-Unity/pull/1603#issuecomment-358954220

@yacuzo:

Ideally, for looser coupling and to make this friendly for dependency injection (Zenject for example), all of the InputManager's logic should be in a plain C# class, together with a MonoBehaviour(Singleton) which only links the C#-class (new it up in Awake) to the unity lifecycle. Those who are fine with singletons can still use the singleton, while those who have other architectures can use the plain C#-class directly. The same concept could be applied to all managers/singletons. If the managers need some other C#-class or some singleton, then the dependency should be injected in the managers constructor by the MonoBehaviour singleton.

@maxouellet:

About DI, I wrote some kind of DependencyResolver a while back that I've been using in my projects instead of singletons. It's not a dependency injector with a pull model though: it's essentially a static class that wraps a Dictionary. It verifies that anything registered to it is an interface, and allows consumers to retrieve an interface from it. Doing that, you can change all the singletons so that they implement some interface and register themselves with the DependencyResolver on awake, and they anything that depends on the singletons just queries the interface they want from it.

I never gave a serious thought about submitting it to HoloToolkit because it makes things a little harder to understand, which would impact things like the HoloAcademy for example (for the same reasons that @StephenHodgson mentioned around the input system being complex).

|Pros|Cons|
|---|---|
|Removes the Singleton pattern's object life cycle issues|Adds some complexity and makes the Input System and other MRTK systems using them more difficult to understand|
|Decouples dependencies|No GameObject presence in the scene.|
|Adds ability to mock/stub systems for other platforms (i.e. Spatial Mapping and anchors)| |
|Makes it easier to swap a system with an alternate implementation that has the same interface| |

[maxouellet]

I've attached a version of what I've been using recently. It has been working pretty well for us.

Basically, you have every singleton implement an interface instead of the SingletonBehaviour base class. Alternatively. you can wrap the singleton with another class that has an interface if you don't want to make a breaking change. Then, you simply have your MonoBehaviour register itself with the DependencyResolver as the implementation for its interface. Other modules can query the DependencyResolver to get the interfaces they need.

There's definitely other possible solution to the problem, but I thought this one was relatively simple. Add it allows for unit testing, mocking, and all can of other neat things.

DependencyResolver.cs.txt

[yacuzo]

It is not necessary to remove the singletons completely. I think we can keep the best of both worlds if all logic is in plain C# with singleton-MonoBehaviours as wrappers. That way it will be dependency injection friendly for those that want a custom setup, but still be drag and drop for newbies/prototypes.

[Railboy]

@maxouellet @yacuzo Personally I see the value in handling managers in this way, and I find dependency injection to be useful in general.

But its place in the Unity ecosystem is murky. I feel that if we break with Unity convention it should be in ways that make the MRTK simpler and easier to understand. And I suspect that the value of this approach to an average developer would likely be outweighed by the complexity and confusion it introduces.

[keluecke]

Dependency injection seems more complicated then singletons to me, involving more work to use and generally harder to understand. What benefit does it provide over singletons?

[maxouellet]

The main benefits of using some kind of DI (or dependency pulling, or anything that isn't a static Instance) that I can think of right now are:

1. Makes it easier to swap a system with an alternate implementation that has the same interface. This will potentially become a bigger scenario as people start building apps that support both VR and HoloLens. For example, you might want a VR implementation of the SpatialMapping singleton that does some spatial mapping simulation. Or a different kind of input manager for your app. Another good example is that this allowed us to write a simulate SpatialAnchorManager for code running in the Unity editor, without having to change all of the code that depends on spatial anchors.

2. Allows mocking/stubbing systems, thus allowing unit testing of components.

3. Reduces coupling between classes

This is probably not an exhaustive list. 1 and 2 have been especially helpful for the projects I've been on recently, as it allowed our engineering team to work on different modules that had to interact with each other, just by providing quick mock implementations while the real implementation is being completed. It also allowed us to build unit tests and integration tests.

I understand that this is not necessarily a use case for hobbyists or people looking to quickly build something. However, using Singletons actively prevents scenarios 1 and 2, which can be important for larger projects. Using some kind of very simple DI-like system enables these scenarios while still keeping the overall code pretty simple.

Also keep in mind that DI doesn't necessarily mean "super complex framework". For example, the Dependency resolver I attached is only about 100 LOCs, which is about the same length as the Singleton.cs file we have in MRTK.

[StephenHodgson]

Yeah I agree with Max here, the concepts are move complex, but in reality will simplify the code and decouple a lot of dependencies.

[StephenHodgson]

Guys, check this out. We can do this for all singleton classes that don't need to to inherit from Monobehavior but they'll need to be static.

[RuntimeInitializeOnLoadMethod(loadType: RuntimeInitializeLoadType.BeforeSceneLoad)]
private static void RunInitializeInPlayer()
{
    // We're using this method just to make sure the class constructor is called
    // so we don't need any code in here. When the engine calls this method, the
    // class constructor will be run if it hasn't been run already.
}

This guarantees we have an instance before any other Unity API is called. (Awake, OnEnable, Start, etc)

[Railboy]

Something I haven't seen mentioned yet is specifying script execution order. It's not something I use often, but forcing manager singletons to execute prior to all other scripts would solve the Unity life-cycle problem, correct?

[StephenHodgson]

Yes, but I don't think there's a way to set this up on a per project basis.

[SimonDarksideJ]

That sounds like a Plan @StephenHodgson , will just need some testing to see if it works as described.
If we can pair that with some ScriptableObject use, to separate the "settings" of components in to reusable assets, we may have a plan.

[Railboy]

I believe script execution order is set in the script's meta file.

[StephenHodgson]

I believe script execution order is set in the script's meta file.

That is incorrect, or else this would be easily reconcilable.

[Edit]
@Railboy You were correct. My apologies.

This data used to be stored in a project settings asset file back in the day. Not sure when this changed.

[keluecke]

This doesn't help the situation where singletons depend on each other.

[Railboy]

@keluecke With execution order mangers would be initialized in a deterministic sequence, so dependency can be planned more carefully. And I'd be surprised if we couldn't manage the same with DI somehow.

@StephenHodgson @DDReaper @maxouellet I won't dispute the value of a DI solution in an enterprise context. It's awesome. But given our goal of appealing to the often-neglected prototypers / hobbyists, and given that a simpler, more standard-practice option is viable, are we now willing to entertain that approach?

I'm especially concerned about losing the simplicity of changing singleton settings by selecting gameobjects in the editor.

It's no fun being a wet blanket because the excitement in this thread is palpable. I just want to make sure we're not mistaking our appreciation of a technique's elegance for value to the users we want to appeal to.

[keluecke]

@Railboy I second your concern that DI may end up being to complex.

I find anything that requires knowledge of which object needs to be initialized before which other ones just makes development exponentially harder as you add more. However, the solution I have in this PR auto sorts dependencies, so nobody has to know what needs to go first, or move meta files with c# scripts.

[StephenHodgson]

I find anything that requires knowledge of which object needs to be initialized before which other ones just makes development exponentially harder.

Agreed, but I'm pretty sure DI solves this problem as well.

But given our goal of appealing to [less experienced developers]

Is that our goal? I definitely utilize the repo for enterprise applications, and would like to keep it geared for production environments.

That being said, the usefulness and utility of the repo should always be maximized and it shouldn't matter what kind of user ends up with this in their project. A good pattern should always be championed and taught to others if it's a good approach. (And sometimes there's more than one, like this for example). We should be leading by example and setting the pace for people who are learning or are just doing this for fun.

I'm especially concerned about losing the simplicity of changing singleton settings by selecting gameobjects in the editor.

I don't think we would lose this option to customize settings and could find appropriate solutions for different managers. Most likely there would still be some sort of option on a GameObject somewhere in the scene.

On that note, it might be best to limit the number of classes that inherit from Monobehaviour except when required. This would reduce the complexity and facilitate better coding standards and practices that align more to traditional C# programming practices. It'll also help facilitate more complex design goals we might have in the future like multithreading or higher .net language support.

[Railboy]

But given our goal of appealing to [less experienced developers]

Is that our goal?

I realize your question is rhetorical but I'll answer it anyway: Yes, our goal is to appeal to a broad range of developers and not just enterprise / production. That's been made explicit. It even says as much on the front page readme.

Your personal experiences with the toolkit are obviously invaluable, but as a sample size of one within a relatively narrow context it can't always inform our choices regarding complexity. And as you know, the most consistent feedback we get from our toolkit users - over & over - is that the pendulum has swung too far in the direction of complexity.

[maxouellet]

There's no good or bad answer. I think the needs of professional enteprise-scale developers sometimes directly conflict with the needs of hobbyists / prototypers. That being said, sometimes this toolkit has taken the approach of providing a building block that can only realistically be used as-is by the latter group, and completely ignores the needs of the first group. This makes the toolkit impossible to consume as-is, thus making it very hard for professionals to contribute back to it.

A different approach would to build common building blocks that both groups need, and share the front-end for these common building blocks as optional components (potentially as part of examples). So for example, you could end up with a SpatialMapper class, and then you could expose it as the SpatialMapping Singleton in the examples, but developers could choose to consume the spatial mapper class itself and wrap it differently (through DI, for example). In that sense, I agree with @StephenHodgson last paragraph saying that limiting the amount of MonoBehaviour would facilitate these goals.

All that being said, if we were to move to some kind of DI like the one I proposed, users would NOT lose the ability to edit "singleton" values in the editor, and it wouldn't change anything to the way that objects spawn (so accessing DI-registered components still wouldn't be safe to do in Awake, which is already the case for our Singletons). All it would change is the way you retrieve them (.Instance vs DependencyResolver.Get).

Also singletons in Unity is not something that plays well with the editor at all. They essentially break the live recompilation of code (AKA changing your code while your app is running in editor, and seeing the change happen live), because all static variables are re-initialized to their default value upon recompilation. Their usage is pretty controversial in Unity itself (see http://wiki.unity3d.com/index.php/Singleton for example), so it's not like this is the first time this discussion as come up in the history of Unity development :)

[SimonDarksideJ]

But given our goal of appealing to [less experienced developers]

Is that our goal?

I'd say that the goal is to appeal to two very different audiences.

• The Indies / prototypers, who just want to pick up and go with as much drag and drop as possible
• The Pros - Who want to bend and twist the project to meet their needs.

So long as we provide an "out of the box" option for the Indies, I don't see any reason why DI would complicate anything. This would be one more thing hidden away so they don't see it. It would be handled by the Toolkit.

For Pro's, it gives the additional flexibility to swap out components for their own.

We're not yet in a place for "the indies" but I think it's a worthy road to aim for.

[jbienzms]

I've been following along in the background and doing my own research, but I think now is a good time for me to chime in.

Good or bad, I've spent a LOT of time with DI. Before I joined Microsoft I worked on several enterprise projects along side Microsoft Consulting Services. On those contracts we often used Unity (Unity the Microsoft IoC framework, not Unity the 3D engine. Confusing I know! 😁)

Since joining Microsoft I've continued using DI. Well, more accurately I would say I've continued using IoC containers. As we've built _countless_ Xaml projects with customers, MvvmLite became a tremendously popular framework. And their SimpleIoC container has been leveraged in many of our projects.

Dependency Injection is no doubt an advanced technique. Especially when you start doing things like constructor injection and chained factory injection, these concepts require quite a bit of relearning to wrap your head around them. I'm not at all opposed to injection, it just requires thinking in a very different way. And while most of the complexity can be hidden from developers _consuming_ the toolkit, those that are _contributing_ to the toolkit will need to understand these things if we start making liberal use of them.

There is a component of DI, however, that is quite easy to understand and take advantage of: The Container. Personally, I feel that well-designed use of a container would go a long way to solving most (if not all) of the issues we currently face with Singleton< T >.

Containers are very straightforward. At some point (usually at the start of the application) things get added to the container. Later, when anyone needs something, they get it from the container. Good containers can also enforce rules that help keep applications from breaking. For example, when you add something to a container you can say "And there should only be one of these". Then, if someone tries to add more than one an error is thrown. Another great rule is "I need one of these and null is not an option". Then, if someone asks for the thing and there isn't one in the container, an error is thrown.

Hopefully you'll agree that container rules could solve many of our issues. Singleton< T > could simply be changed to say "Add me to the container, there can only be one". And all of the behaviors that need a manager can say "Get me this manager and null is not an option." At that point, the problem is basically solved. And because these rules are enforced as exceptions, Unit Tests would fail the moment something breaks.

Now my question is, if we decide to take a dependency on a container (see what I did there?), which container should we use?

I've been doing some research and so far I'm quite fond of Zenject. It's open source (MIT), it's free, it's in the Unity Asset Store, it has a 5 star rating with 130 reviews, and it's designed _specifically_ for Unity. Their injectors can do things like instantiate prefabs and components, handle load order, and even handle instances across scenes. That's really cool!

In my examples above regarding container rules, here's how you'd write them with Zenject:

"Add me to the container, there can only be one"
Container.Bind<InputManager>().FromInstance(this).AsSingle();

and

"Get me this manager and null is not an option."
InputManager manager = Container.Resolve<InputManager>();

"null is not an option" is actually the default. If you wanted to support a case where InputManager was optional you could do:
InputManager manager = Container.TryResolve<InputManager>();

In summary, I'm not opposed to full on DI. I think we can use it in the cases where we _really_ need it. But I'm a huge fan of containers, and from what I've seen Zenject looks like one heck of an option.

[jbienzms]

Well, turns out I may have really oversimplified things. I just created a sample project and imported Zenject. Their documentation on the home page it seems is a bit misleading. In fact the code I wrote above is misleading because Container is not a singleton.

Zenject looks very powerful, but it also again requires a different approach. They have things like Factories and Installers that are supposed to be used to setup the bindings in the container. They have things like SceneContexts that you "decorate your scene" with to specify your installers. It's all fascinating stuff, but it's also a tremendously different way of doing things. One I'm not entirely sold on. Especially if this tax gets passed onto the end user.

Something else to ponder... My use of the container as shown above is essentially using the container as a Service Locator. Some consider that an anti-pattern. I understand the concern, especially when classes can appear to have an empty constructor but in fact depend internally on other classes. Having said that, this is not at all uncommon in Unity where classes get linked together dynamically through Inspector fields and GetComponent methods.

Right now I'm back to the drawing board on which container I would use and whether that's even the right approach. Zenject looks awesome, but it's also quite large and requires (IMO) too much setup to use for our purposes.

I welcome your thoughts.

[maxouellet]

@jbienzms did you look at the class I shared in the second post of this thread? It's the simplest form I could create of a container. It seems very similar to what you are talking about, unless I misunderstood what you meant.

[SimonDarksideJ]

Well @jbienzms / @maxouellet
Great minds think alike. I proposed something very similar through other channels in the MRTK and have been planning a similar pattern for starters that can be extended later.

The idea was originally implemented in XNA (and now MonoGame) for containerised style "services / Managers" that are centrally controlled.

As a kick off for further discussion, I've created an experimental branch to demonstrate this in code.
https://github.com/ddreaper/MixedRealityToolkit-Unity/tree/Experimental_MRTKManager

The First demonstration starts small to create an example using the Boundary Manager, (a singleton that really doesn't need to be, like so many)

The first pass Demonstrates several core new features for discussion:

• New MRTK Manager - meant to be a complete replacement for existing Singletons (also intended to be the new Manager for a Multi-VR solution)
• New IManager interface, replaces existing Singleton implementation for MRTK (except manager :D)
• New Scriptable Object MRTK configuration system. Allows different behaviour configurations to be applied to the manager for the project

Test implementation

• Updated Boundary Manager to inherit from IManager. Updated internals to register with MRTKManager for events and get the FloorQuad from current MRTK behaviour config
• Updated Behaviour Example scene, to grab the BoundaryManager reference from the MRTK Manager.
• Removed Boundary GO from scene as it is no longer needed.

This is a first pass example for moving away from the ScriptableObject dependency and introduce the possibility of a ScriptableObject configuration system.

Aims to improve:

• Remove GO / singleton dependency for "Managers"
• enable easier configuration of MRTK through central config asset (possible discussion to support multiple or sub assets for different elements.
• New centralised manager that will also eventually manage the Multi-VR solution

Following discussion, I'll expand this further.
For reference, I've also done a survey of the existing singleton use in MRTK (shocking results), not one implementation currently requires a full singleton pattern imho - for discussion.
https://github.com/ddreaper/MixedRealityToolkit-Unity/tree/Experimental_MRTKManager

[keluecke]

I am extremely resistant to complicating core components of MRTK. I have worked with many people who already find MRTK to large to want to bring all of it in and to inter-connected to pull pieces out.

Toolkits often go through cycles of growth, followed by the creation of a 'lightweight' version that eventually become the main kit as nobody wants the heavy version.

With this change I suggest we keep in mind the person who just wants one component from the toolkit. How many files to they need to hunt down form the enlistment in order to get just a little bit of functionality? Our goal isn't to tell people how to design Unity apps and be the standard for best practices in unity. It is instead, to give people extra tools to prevent them from needing to solve the same problems common to mixed reality over and over.

As the toolkit grown we need to be extra carful that it is still separable as possible. If we are talking about swapping the inherited "singleton" file with _one_ other file I can get behind that, but I would advise against it if switching to DI just makes it more likely to expand to a few files in the future.

[SimonDarksideJ]

I hear your comments @keluecke , the plans are greatly simplify the MRTK, not over complicate it.
At the moment there are far too many dependencies, especially when (as you say) most only want a few components. The aim is to reduce complexity for starters and then enable advanced options later for more advanced use cases (without sacrificing the simple case)
Unity simply isn't built for singletons, so these improvements will strip away overhead and at most, you'll only need one or two components going forward and you will be able to choose which are enabled.

The updates to the Boundary manager (shown above) demonstrate this, as to enable / disable the boundary now, simply means clicking a checkbox without any other side effects. Other components should follow suit. (especially with a simplified / replaceable input system)

Also using Unity's advice to switch to ScriptableObjects for configuration, means you will be able to create your own profile for MRTK and then apply it.
For example, check out Unity's latest video for creating skinning profiles for the Unity UI system on their latest Live training session.
https://unity3d.com/learn/tutorials/topics/user-interface-ui/introduction
And also check out their recent posts around architecture and using scriptable components
https://blogs.unity3d.com/2017/11/20/making-cool-stuff-with-scriptableobjects/

If you also have some example use cases for small, single component projects. Please feel free to jot them here or on Slack.

[maxouellet]

@DDReaper Looking at your change, I'm not sure I understand how you'll avoid bloating the MRTManager class with a ton of references to Behaviours. I like the idea of using ScriptableObjects for config (that's what we've been switching a lot of our objects to recently), but the way your branch has it setup, it seems like you still require a central class that knows about all ScriptableObjects.

I wonder one way to do it would be for each system to be a ScriptableObject, and the manager just spawns and tracks them as needed. That still wouldn't address the dependencies between systems though, I'm not sure how to handle that one cleanly. And there are systems that need to be called from Update/Awake/Start, although that could be handled by the MRTKManager calling into them during Update...

[SimonDarksideJ]

In writing my response, I did come to the conclusion it may not be as clear as I'd hoped. I plan to write a small doc and add it to the repo (with screenshots)
It's still early days, so feedback like this is crucial to help shape the work.

[SimonDarksideJ]

OK, updated experimental branch to also include the MotionControllerVisualisation and added some high level docs (plus images) to hopefully convey the discussion:

https://github.com/DDReaper/MixedRealityToolkit-Unity/blob/Experimental_MRTKManager/External/ReadMeImages/Experimental/ScriptableMRTKManager.md

*Note, I've been made aware that a supplementary requirement for the toolkit, is to allow consumers to "pick and choose" which bits to have in their project and deleting / removing other parts. This isn't covered in this branch yet and will shape the plans going forward.