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

feature: Add protected FindParent method #692

Closed gtg092x closed 4 months ago

gtg092x commented 5 months ago

This will allow implementors the opportunity to override runtime Find behavior. This is particularly useful for projects that implement multiple potential parent scopes and need a way to disambiguate them.

For example, some child scopes might want to specifically find ancestors up their transform tree:

public class ParentedLifetimeScope : LifetimeScope {
    protected override LifetimeScope FindParent() {
        LifetimeScope[] allScopes = FindObjectsByType(type, FindObjectsSortMode.None).Cast<LifetimeScope>().ToArray();
        return allScopes.First(x => IsAncestor(x.gameObject));
    }

    public bool IsAncestor(GameObject target) {
            var parent = this.transform.parent;
            while (parent != null)
            {
                if (parent == target.transform)
                {
                    return true;
                }

                parent = parent.parent;
            }

            return false;
        } 
}

All unit tests pass with this change and because the implementation is a virtual method that defaults to the previous functionality, current behavior is unchanged.

vercel[bot] commented 5 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 Jul 11, 2024 5:52pm
hadashiA commented 5 months ago

It is a good idea.

However, with this implementation, it seems that the following conditions have to be met.

if (parentReference.Type != null && parentReference.Type != GetType())

Shouldn't it be possible to override with or without parentReference.Type?

gtg092x commented 5 months ago

That's true. I'll make a small design change so that the override can be either.

hadashiA commented 4 months ago

Thank you very much. I think it's good.

The only point, is that it doesn't seem very clear from the point of view of the user considering override how FindParent() and FindParent(Type) should be used.

Since FindParent() probably also serves as a FindParnet(Type), why not just have the former be virtual?

gtg092x commented 4 months ago

Thank @hadashiA! I had to fight my commit history a little bit to get a clean PR, but this should simplify the interface.

hadashiA commented 4 months ago

👍 Thanks for the fixing.