hadashiA / VContainer

The extra fast, minimum code size, GC-free DI (Dependency Injection) library running on Unity Game Engine.
https://vcontainer.hadashikick.jp
MIT License
1.9k stars 164 forks source link

Execution order for entry points #271

Open pnarimani opened 3 years ago

pnarimani commented 3 years ago

Is it possible to specify execution order for non-MonoBehaviour classes that are created through RegisterEntryPoint?

hadashiA commented 3 years ago

Currently, this feature is not available.

I feel that controlling the order at the class level is a bit complicated.

There are several timings available, so you can separate the groups. https://vcontainer.hadashikick.jp/integrations/entrypoint#available-interfaces

4nonym0us commented 3 years ago

The feature is important for Unity-oriented DI containers because there are cases where having only couple timings is not enough to cover the requirement:

In some basic cases having just XEvent and PostXEvent is sufficient, but it's not unusual that a service\dependency must be IInitialized earlier/later then others. For example,

4nonym0us commented 3 years ago

I've created a PR with a basic implementation.

Public API for controlling execution order is here. Corresponding unit test is located here. Execution order number works just like Unity's or Zenject's ExecutionOrder: the lower the number, the earlier player loop timing is executed. Disposal of objects which implement IDisposable is performed in reversed order (similar to Zenject).

Let me know what you think about the feature and the implementation in particular. I can add new API to docs if you think that PR is worth merging.

hadashiA commented 3 years ago

Some services/dependencies need to be initialized before others (i.e. game saving/loading system needs to get initialized early to restore the game state). Having just 2 timings might not be enough for sophisticated use-cases with complex initialization or where it's necessary to control the execution order of scripts within the same timing.

4nonym0us commented 3 years ago

Yeah, the scenario above (as well as any scene/asset/resource loading) is asynchronousand It absolutely makes sense to use message brokers (like UniRx or MessagePipe) to coordinate the services, which might not depend on each other explicitly.

On the other hand, there is no doubt that some services/tasks might require synchronous sequential execution withing the same timing – some Unity-focused DI frameworks offer same/similar feature and I'm just curious whether this feature is in the scope of VContainer at all.

Artein commented 1 year ago

Isn't it sufficient to expect execution order based on registration order? In this case the earliest "system" must be registered on top of Configure() method. It also makes sense in terms of architecture layers, eg. core system will be registered earlier than gameplay systems.

@4nonym0us , do you have any case (cases?) where you need to hook that up and manually overwrite invocation order?

4nonym0us commented 1 year ago

@Artein Sometimes it is sufficient indeed. Nevertheless, there is a reason for API to control execution order to be common in Unity world:

  1. Unity (as DefaultExecutionOrderAttribute and Script Execution Order in project settings, which can be managed via MonoImporter.SetExecutionOrder(...)) which means that there are edge cases where such API could be handy.
  2. Other unity-targeted DI containers like Zenject's BindExecutionOrder.

Real-life sample usage of execution order control API can be found in Unity's NavMeshComponents repository and even VContainer. Team that developer NavMeshComponents has following reasoning to use DefaultExecutionOrder attribute :

Q: What's the deal with the 'DefaultExecutionOrder' attribute? A: It gives a way of controlling the order of execution of scripts - specifically it allows us to build a NavMesh before the (native) NavMeshAgent component is enabled.

Zenject devs provided following reasoning to implement this feature in their DI framework (they also have a sample game that uses execution order control API):

In many cases, especially for small projects, the order that classes update or initialize in does not matter. However, in larger projects update or initialization order can become an issue. This can especially be an issue in Unity, since it is often difficult to predict in what order the Start(), Awake(), or Update() methods will be called in. Unfortunately, Unity does not have an easy way to control this (besides in Edit -> Project Settings -> Script Execution Order, though that can be awkward to use)


In this case the earliest "system" must be registered on top of Configure() method. It also makes sense in terms of architecture layers, eg. core system will be registered earlier than gameplay systems.

I agree, It absolutely makes sense, but such approach works well only when the container is small and simple. I'd say that it's not uncommon for a Unity games (especially for the large ones) to require components to have specific execution priority even if all your in-game systems are not directly dependent on each other because all of them are running in the same lifecycle and the more control we have over it, the better it is for developers. Then, having an explicit way to define execution order on registration will allow a developer (who is even not familiar with VContainer) to have a better understanding of game's life cycle and internals. Consider also a scenario when you have a container with many registration and you would like to extract similar registrations or group them up next to each other for better readability. In example, currently you must write following code if you want S1Foo1 and S2Bar1 to be invoked early:

protected override void Configure(IContainerBuilder builder)
{
    builder.Register<S1Foo1>();
    builder.Register<S2Bar1>();

    builder.Register<S1Foo2>();
    builder.Register<S1Foo3>();

    builder.Register<S2Bar2>();
    builder.Register<S2Bar3>();
}

instead of

protected override void Configure(IContainerBuilder builder)
{
    builder.Register<S1Foo1>().WithExecutionOrder(-10);
    builder.Register<S1Foo2>();
    builder.Register<S1Foo3>();

    builder.Register<S2Bar1>().WithExecutionOrder(-5);
    builder.Register<S2Bar2>();
    builder.Register<S2Bar3>();
}
// or using extension methods
protected override void Configure(IContainerBuilder builder)
{
    builder.ConfigureS1();
    builder.ConfigureS2();
}

So in regards to the original question (about the necessity of "hooking up manually to overwrite invocation order"). First of all, the execution order has not even been documented yet and doesn't seem to be covered in unit-tests. If something is neither documented nor covered in unit-tests, then even if registration order is preserved in EntryPointDispatcher during resolution of IReadOnlyList<T>, the lack of documentation/tests leads to having no guarantee that the future updates will not affect current behavior. Even then, having such a restriction (to respect registration order during resolution of IReadOnlyList<T>) might not be good for a DI framework (some IoC containers do not provide such guarantee because of performance cost in comparison to returning unordered collection).

After all, I don't like feel like calling the feature like "overwriting" because something must we written in first place in order to make it be able to be overwritten. Having an explicit way to define behavior is always nice to have, especially when we are talking about DI framework in Unity where everything is based around loops/lifecycles.

I used to think that it was worth discussing this feature. Then, depending on the feedback, we could either add a way to explicitly define execution order instead of relying on undocumented behavior (and provide a better control over execution order of VContainer's Timings) or consider improving documentation and/or adding unit-tests if they are missing.

Any thoughts are welcome.