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

Synchronous tree notification on child change #8850

Open
janosdebugs opened this issue Jan 10, 2024 · 7 comments · May be fixed by godotengine/godot#90707
Open

Synchronous tree notification on child change #8850

janosdebugs opened this issue Jan 10, 2024 · 7 comments · May be fixed by godotengine/godot#90707

Comments

@janosdebugs
Copy link

janosdebugs commented Jan 10, 2024

Describe the project you are working on

I'm working on creating a dependency injector for Godot that automatically injects dependencies when a node is added to the tree (e.g. logger, input subsystem, etc). This has the benefit of globally configuring dependencies across multiple classes without boilerplate code needed.

A node implementation using this system might look something like the following code. The actual implementation of Logger may be a subclass.

extends Node

## INJECT replaces the annotations from other languages, marking a member variable as injectable.
static var INJECT = ["logger"]

## logger will be automatically injected by the customized SceneTree child class.
var logger: Logger

I'm also happy to implement the change described in this proposal.

Describe the problem or limitation you are having in your project

Currently, there is no way to intercept the _enter_tree function being called and provide the necessary dependencies to the node that can then be used in _enter_tree and _ready. (This includes the node_added signal, which is emitted after the _enter_tree and _ready functions are processed on the node itself.) The only notification that is synchronous is the NOTIFICATION_PARENTED, which is only sent to the node itself but not to the tree. This requires nodes to explicitly include code to fetch their dependencies instead of having them populated from the outside, which presents a problem when testing or when different dependency implementations should be injected (e.g. a different logger implementation).

Describe the feature / enhancement and how it helps to overcome the problem or limitation

If a notification is issued to the tree before the _enter_tree is called, the tree can select the necessary dependencies for the node and inject them. The _enter_tree and _ready functions can then rely on the dependencies being present.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

There are several possible solutions. Changing the call order in node.cpp is not advisable as it would be a breaking change. Instead, a call should be added before the call to _enter_tree to the effect of:

data.tree->node_adding(this);

The tree could then send out a notification or provide a call function of its own.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, the only way to work around this is by adding a significant amount of boilerplate code for complex projects, such as:

@onready var logger = Logger.get_service(self)

Is there a reason why this should be core and not an add-on in the asset library?

It is not possible to implement in an add-on due to the missing synchronous hooks on the scene tree for nodes being added.

janosdebugs added a commit to janosdebugs/godot that referenced this issue Apr 15, 2024
janosdebugs added a commit to janosdebugs/godot that referenced this issue Apr 15, 2024
@janosdebugs
Copy link
Author

For those who are curious and have a little bit of time, a talk on why this is important: https://www.youtube.com/watch?v=RlfLCWKxHJ0

@AThousandShips
Copy link
Member

What makes this not work with SceneTree.node_added?

@janosdebugs
Copy link
Author

janosdebugs commented Apr 15, 2024

@AThousandShips if I observed correctly, the node_added in the scene tree is only fired after the _enter_tree and _ready are processed on the node itself, so the node can't rely on its dependencies already being present in these functions.

@AThousandShips
Copy link
Member

I'd say this is very niche, I don't see general use of this, I'd say generally it could be worked around when it's needed, but I don't think it'd be generally required

@janosdebugs
Copy link
Author

janosdebugs commented Apr 15, 2024

That is unfortunate, dependency injection is pretty standard in other areas of software engineering and not having the ability in Godot makes the code a lot harder to test.

Edit: can I ask this proposal to be left open so the Godot testing community can weigh in?

@AThousandShips
Copy link
Member

What's unfortunate? I'm not making decisions here, I'm stating my opinion clearly, not speaking for anyone else :)

@janosdebugs
Copy link
Author

Thank you @AThousandShips , I was assuming your answer was an "official" position as a Godot team member. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants