realitycollective / com.realitycollective.service-framework

The Service Framework package components for the Reality Collective. This package an extensible service framework to build highly performant components for your Unity projects.
https://service-framework.realitycollective.net
MIT License
9 stars 5 forks source link

Inability to Retrieve Service Profiles in SceneServiceProvider #112

Open lynconEBB opened 1 day ago

lynconEBB commented 1 day ago

When setting up a service using SceneServiceProvider, it seems the service profile configurations are not accessible from the main ServiceProvider. As a result, the TryGetServiceProfile function cannot retrieve the profile for the service, which impacts functionality.

The current implementation of TryGetServiceProfile appears to restrict configuration lookup to only Providers of type ServiceProviderProfile in the rootProfile parameter.

  public bool TryGetServiceProfile<TService, TProfile>(out TProfile profile,
  ServiceProvidersProfile rootProfile = null) // rootProfile should be of type BaseServiceProfile<IService>
            where TService : IService
            where TProfile : BaseProfile
        {
            if (rootProfile.IsNull())
            {
                rootProfile = ActiveProfile;
            }

            if (!rootProfile.IsNull())
            {
                foreach (var configuration in rootProfile.ServiceConfigurations)
                {
                    if (typeof(TService).IsAssignableFrom(configuration.InstancedType))
                    {
                        profile = (TProfile)configuration.Profile;
                        return profile != null;
                    }
                }
            }

            profile = null;
            return false;
        }

Changing the rootProfile parameter to BaseServiceProfile<IService> would enable SceneServiceProviderProfile to be passed in. This change should make the configurations accessible and allow TryGetServiceProfile to locate the correct profile, as SceneServiceProviderProfile would contain the necessary configurations.

SimonDarksideJ commented 19 hours ago

Thanks for the report @lynconEBB , are you planning a PR, if not I will see about updating the implementation.

Also, could you describe a use case in which this would be used, so I can ensure we have test coverage for this pattern?

lynconEBB commented 7 hours ago

Hi @SimonDarksideJ, thank you for the quick response!

I’ve created a pull request to address this issue, which updates the rootProfile parameter type as previously discussed. This adjustment is aimed at providing greater flexibility in accessing service profiles.

For testing, I believe we can cover the essential use cases by calling TryGetServiceProfile with rootProfile instances of both SceneServiceProvidersProfile and ServiceProvidersProfile. Each test should validate that both types can access configurations and retrieve the appropriate profiles for the specified service.

You can review the pull request here: #113.

Thanks for your feedback on this!