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
2.01k stars 176 forks source link

Add non-generic RegisterInstance overload #695

Open Valax321 opened 4 months ago

Valax321 commented 4 months ago

This pull request simply adds a new overload for RegisterInstance that allows the caller to specify the type of the instance as a parameter without generics. This matches the behaviour of other registration overloads that allow passing the type at runtime.

This behaviour is useful when registering multiple items from a list when we can't know at compile-time what types may appear in the list. For instance, in my game each game state can register arbitrary ScriptableObjects that are assigned in the inspector:

foreach (var data in m_GlobalData)
 {
    builder.RegisterInstance(data, data.GetType());
 }

The implementation is nearly identical to the generic version but uses the method parameter implementationType instead of a generic type parameter in the As() call.

vercel[bot] commented 4 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vcontainer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 22, 2024 3:23am
hadashiA commented 4 months ago

Thank you for thePR.

I think this is fine on its own, but overall, it seems to deviate from the API consistency point of view, as it is aligned with .As(Type) when passing the type.

I don't think .As() is the best way to go at the moment, but I'm trying to be consistent, so I think it would be better to be able to pass Type as an argument for all registers if I were to make such a change. But I think it's a bit late now. If you have a strong reason why it would be better to give special treatment only to RegisterInstance, please let me know.

Valax321 commented 3 months ago

Sorry it took me a while to get back to this, I've been moving and haven't had much time recently.

The issue I had with using .As() is that the generic version of RegisterInstance() assumes you to want register the object as the type you pass in TInterface at minimum in addition to any As() calls, which was behaviour that didn't work in my case.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
        public static RegistrationBuilder RegisterInstance<TInterface>(
            this IContainerBuilder builder,
            TInterface instance)
            => builder.Register(new InstanceRegistrationBuilder(instance)).As(typeof(TInterface)); // No good for me!

In my case I am registering objects from a list where I only know the base class (which is ScriptableObject in the case of my original PR message, from an inspector-editable list), but want to register them only as their actual type, not ScriptableObject as well.

My project's use case is maybe fairly unusual (I'm registering lots of things assigned by inspector rather than through code and already had to work around a couple of API limitations already) but I couldn't find a way to go about this through the public API, so if you think this behaviour should be exposed or named differently I am happy to rework it.