Skip to content

feature: Add protected FindParent method #692

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 12, 2024
Merged

feature: Add protected FindParent method #692

merged 1 commit into from
Jul 12, 2024

Conversation

gtg092x
Copy link
Contributor

@gtg092x gtg092x commented Jul 2, 2024 •
edited
Loading

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 Vercel
Copy link

vercel bot commented Jul 2, 2024 •
edited
Loading

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
Copy link
Owner

hadashiA commented Jul 4, 2024

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
Copy link
Contributor Author

gtg092x commented Jul 4, 2024

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

@hadashiA
Copy link
Owner

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 gtg092x force-pushed the master branch 3 times, most recently from 140ed4e to 5970e32 Compare July 11, 2024 17:38
@gtg092x
Copy link
Contributor Author

gtg092x commented Jul 11, 2024

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

@hadashiA
Copy link
Owner

👍 Thanks for the fixing.

@hadashiA hadashiA merged commit e743c10 into hadashiA:master Jul 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gtg092x @hadashiA