vovgou / loxodon-framework

An MVVM & Databinding framework that can use C# and Lua to develop games
MIT License
1.86k stars 363 forks source link

Service container doesn't register generics properly #30

Closed kondratov-saritasa closed 1 year ago

kondratov-saritasa commented 1 year ago

Steps to reproduce:

  1. Create a generic service Service<T>
  2. Register Service<Type1>
  3. Register Service<Type2>

Result: DuplicateRegisterServiceException is thrown.

Expected: All works fine.

Here is a source of the problem: https://github.com/vovgou/loxodon-framework/blob/master/Loxodon.Framework/Assets/LoxodonFramework/Runtime/Framework/Services/ServiceContainer.cs#L72

namespace Loxodon.Framework.Services
{
    public class ServiceContainer : IServiceContainer, IDisposable
    {
        private Dictionary<string, IFactory> services = new Dictionary<string, IFactory>();

        public virtual object Resolve(Type type)
        {
            return this.Resolve<object>(type.Name);
        }

        public virtual T Resolve<T>()
        {
            return this.Resolve<T>(typeof(T).Name);
        }

...

        public virtual void Register<T>(T target)
        {
            this.Register<T>(typeof(T).Name, target);
        }

typeof(T).Name for Service<Type1> Service<Type2> will return same string: Service'1.

My suggestion is to use typeof(T).ToString() then we get something like: Service'1[Type1] and Service'1[Type2]. If you ok with that - I can prepare a PR.

vovgou commented 1 year ago

I need to think about your suggestion,because changing it may break existing code.

The features of my service container are simple, if you need a powerful container, you can integrate the third-party Ioc/DI container.In the question you mentioned, you can inherit the IServiceContainer interface and customize your own ServiceContainer,or you can integrate Zenject into your project.

There is another way, that is, you can customize the name for your service, use the following method:

ServiceContainer.Register<T>(string name, T target);
container.Register("serviceName1",service1);
container.Register("serviceName2",service2);
kondratov-saritasa commented 1 year ago

@vovgou That is what I did already. But I decided to report in hope it might be resolved sometime.

Regarding breaking something - I think this unlikely will happen unless you save registered services somewhere (like serialize to file) or may be lookup for services manually.

Also as an option we could have different strategy for generic classes and keep current code for usual classes.

vovgou commented 1 year ago

I will modify it in the next version. image