VContainer icon indicating copy to clipboard operation
VContainer copied to clipboard

Add open generic registration api and resolve feature

Open DenisPimenov opened this issue 2 years ago • 2 comments

Hi! I try to implement open generic feature #301. If you have time to look and tell me what can be changed or improved I can do it

DenisPimenov avatar Apr 17 '22 07:04 DenisPimenov

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hadashia/vcontainer/42FG2b3uqBQcKJysb5xMwiRxGK2h
✅ Preview: https://vcontainer-git-fork-denispimenov-feature-open-generic-hadashia.vercel.app

vercel[bot] avatar Apr 17 '22 07:04 vercel[bot]

Thanks for working on it! Sorry I haven't had time to look at the slightly bigger changes. I'll look at it sometime this month or next!

hadashiA avatar May 11 '22 14:05 hadashiA

Hey! Can you tell me if there is any progress on this issue?

DenisPimenov avatar Oct 17 '22 03:10 DenisPimenov

@DenisPimenov Sorry for the delay in replying.

First of all, I would like to thank you. I think this implementation is not bad.

However, I would like to confirm two points.

https://github.com/hadashiA/VContainer/pull/200#issuecomment-1221527508

Since this approach is only making instances of the closed generic types using reflection it means it won't work with IL2CPP, right? It will only work if code generation is used since it will then generate the code that actually uses those types so they won't be missing in runtime.

As pointed out here, in an IL2CPP environment, classes that are not statically referenced will probably be omitted. Therefore, in reality, I think it may be less problematic to list the closed generic type registrations instead of using the open generic api.

If this problem cannot be solved, there is probably a problem.

Second, in some use cases like MessagePipe, we want all ISubscriber in the project to be registered, even if they are not referenced anywhere. For example, as far as I know, autofac has the API like this: https://autofac.readthedocs.io/en/latest/register/scanning.html

So, my I understand is that, there are two ways to support generic types.

  1. Dynamic generic type making at resolve time
  2. Assembly scanning at container initialization time

If either or both of these are to be implemented, API conflicts need to be avoided.

( However, both 1 and 2 probably have IL2CPP issues...

hadashiA avatar Oct 30 '22 02:10 hadashiA

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 Dec 18, 2023 0:01am

vercel[bot] avatar Oct 31 '22 05:10 vercel[bot]

@hadashiA I will try different builds with il2cpp and report the results in a few days.

DenisPimenov avatar Oct 31 '22 08:10 DenisPimenov

Is this still happening?

slimshader avatar Jun 01 '23 09:06 slimshader

Perhaps, the full generic sharing in Unity 2022.1 allows the following to work:

        var t1 = typeof(GenericClass<>).MakeGenericType(typeof(int));
        var i1 = (GenericClass<int>)Activator.CreateInstance(t1);

        var t2 = typeof(GenericClass<>).MakeGenericType(typeof(ClassA));
        var i2 = (GenericClass<ClassA>)Activator.CreateInstance(t2);

        var t3 = typeof(GenericClass<>).MakeGenericType(typeof(StructA));
        var i3 = (GenericClass<StructA>)Activator.CreateInstance(t3);

        var t4 = typeof(GenericStruct<>).MakeGenericType(typeof(int));
        var i4 = (GenericStruct<int>)Activator.CreateInstance(t4);

        var t5 = typeof(GenericStruct<>).MakeGenericType(typeof(ClassA));
        var i5 = (GenericStruct<ClassA>)Activator.CreateInstance(t5);

        var t6 = typeof(GenericStruct<>).MakeGenericType(typeof(StructA));
        var i6 = (GenericStruct<StructA>)Activator.CreateInstance(t6);

So this PR should probably work fine, as long as it's Unity 2022.1 or later.

I'd like to refine the API a bit more and merge this functionality.

hadashiA avatar Dec 09 '23 08:12 hadashiA

Hi. It has been a long time since opening PR. I have synchronized with master, but the tests are not passing. I will fix them.

DenisPimenov avatar Dec 12 '23 08:12 DenisPimenov

Hi! I fixed all the tests and fixed singleton resolution when the service is configured with multiple implementation types.

DenisPimenov avatar Dec 14 '23 19:12 DenisPimenov

@DenisPimenov Oh. Sorry. I was going to merge, can you just fix the conflicts?

hadashiA avatar Dec 18 '23 10:12 hadashiA

@hadashiA I have synchronized with master, but it has compilation errors

DenisPimenov avatar Dec 18 '23 10:12 DenisPimenov

Can you fix them?

DenisPimenov avatar Dec 18 '23 10:12 DenisPimenov

I have synchronized with master, but it has compilation errors

Oh . . I see. This is another issue, so I'll check over here after the merge.

hadashiA avatar Dec 19 '23 11:12 hadashiA