Skip to content
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.

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

Prevent Node from providing its own dependency? #10

Closed
ZumiKua opened this issue Jan 28, 2024 · 5 comments
Closed

Prevent Node from providing its own dependency? #10

ZumiKua opened this issue Jan 28, 2024 · 5 comments

Comments

@ZumiKua
Copy link

ZumiKua commented Jan 28, 2024

As the title says, should a node be able to provide its own dependency? Or at least, should we provide a attribute parameter to control this behavior?

Currently I'm using Ninject with AutoInject, Ninject's kernel will be used to inject Godot Nodes. And some Node will create a ChildKernel from its parent's Kernel for its descendants to inject itself.

But with the following code, my Dependent's OnResolved will never be called, because its IKernel is provided by itself, not its parent, and Dependent's Provide() will only be called in OnResolved, which is a deadlock.


[SuperNode(typeof(Dependent), typeof(Provider))]
public partial class SceneA : Sprite2D, IProvide<IKernel>
{

	[Dependency]
	public IKernel Kernel => DependOn(() => RootProvider.FindRoot(this));

	private IKernel _childKernel;


	[Inject]
	public void Initialize()
	{
		GD.Print("Initialize");
	}
	
	public override partial void _Notification(int what);

	public void OnResolved()
	{
		GD.Print("Resolved");
		_childKernel = Kernel.Get<SceneAKernel>();
		_childKernel.Inject(this);
		Provide();
	}

	IKernel IProvide<IKernel>.Value()
	{
		return _childKernel;
	}
}
@ZumiKua ZumiKua changed the title Prevent Node from Providing its own Dependency? Prevent Node from providing its own dependency? Jan 28, 2024
@jolexxa
Copy link
Member

jolexxa commented Jan 28, 2024

I hadn't anticipated using AutoInject with other DI systems -- it's designed to simply be something that allows a value to be provided to descendant nodes -- nothing else.

I'm not familiar with ninject, so I don't entirely understand what you're asking. Is there a flaw with AutoInject?

@ZumiKua
Copy link
Author

ZumiKua commented Jan 29, 2024

It has nothing to do with Ninject, in my code, SceneA acts as both a dependent and a provider, and it provides the type it depends on, which prevents SceneA from resolving its dependency from its parents. (SceneA considers itself to be the provider of IKernel, not its parents).

The problem is: can a node provide a value to itself? I can't say it's a flaw or a design choice, but IMO it doesn't seem to make much sense to allow a node to provide dependencies to itself?

@ZumiKua
Copy link
Author

ZumiKua commented Jan 29, 2024

In case I haven't made myself clear, what I am saying can be interpreted as:
Should we change this line to var node = ((Node)dependent).GetParent();?
https://github.com/chickensoft-games/AutoInject/blob/main/Chickensoft.AutoInject.Tests/src/Dependent.cs#L414

@jolexxa
Copy link
Member

jolexxa commented Jan 30, 2024

@ZumiKua okay, thanks for the explanation. I think I see what you're saying. We probably should make that change. I don't think I can think of a valid or intended use case where a node would need to provide its own dependency value.

@jolexxa
Copy link
Member

jolexxa commented May 3, 2024

Fixed in #11

@jolexxa jolexxa closed this as completed May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants