godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Add support for context variables / dependency injection (or context mechanism in general) #5467

Open and3rson opened 2 years ago

and3rson commented 2 years ago

Describe the project you are working on

A 2D platformer with Goal-Oriented Action Planning and large number of reusable components (actions, goals, sensors, enemies, behaviors, items, interactable objects).

Describe the problem or limitation you are having in your project

TL;DR: Providing dependencies to nodes down the tree without knowing who needs them is impossible in Godot out of the box.

Most of the time, things in Godot have clear dependencies. For example:

extends Node
export var nav_mesh_path: NodePath
onready var nav_mesh = get_node(nav_mesh_path)

However, sometimes things need to depend on something way higher in the tree, meaning node's behavior is context-specific and may vary based on something provided by one of its parents. For example, consider this tree structure:

- root
  - game (instanced scene)
    - player (instanced scene)
    - world
      - room1 (instanced scene, during runtime)
        - nav_mesh
        - objects
          - tilemap
          - npc_spawner (instanced scene, instances more `enemy` nodes in runtime)
        - enemies
          - enemy1 (instanced scene)
            - ai
          - enemy2 (instanced scene)
            - other_ai
      - room2 (instanced scene, during runtime)
        - nav_mesh
        - enemies
          - enemy3 (instanced scene)
            - ai

Let's say we want to provide nav_mesh to ai, other_ai, and npc_spawner (which will spawn enemies with their own ais).

This is not possible to do in the editor, because some nodes are instantiated during runtime. Thus we need hacks like node.nav_mesh_path = $../nav_mesh.get_path() within _enter_tree().

The only ways to do this are either:

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

Python solved a similar issue by using contextvars: https://docs.python.org/3/library/contextvars.html React solves this by using contexts: https://reactjs.org/docs/context.html

In Godot, there's no such thing as "context", and I believe there should be one.

Furthermore, context variables provide a way to achieve dependency injection which, in my mind, is one of the main principles that Godot's nodes follow.

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

Method 1: get_context()

Every node might expose get_context() method which returns a mutable dictionary. Any changes to this dictionary will be visible by all its children, and modifications will only propagate downwards. I feel like this could be most "godot-ish" solution.

Method 2: ContextVar (akin to Python)

A new class can be implemented which limits its value only to its children. Any changes to ContextVar will be seen only within the current subtree. ContextVar can then be shared by any convenient means: get_tree().set_meta(), singleton script, export var, etc.

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

This enhancement will make it possible to write nodes that are aware of the context they're being used in, and this is something that's not currently possible without modifying the engine itself or exploiting workarounds.

Possible workarounds:

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

As mentioned, similar features have first-class support in Python & React.

Last but not least, I can implement this if the proposal gets accepted.

Zireael07 commented 2 years ago

My solution is basically your bullet .1, just with groups and not iter_nodes()

and3rson commented 2 years ago

Here's a quick PoC I've made to demonstrate a use case for this (as you can see, it would really play well if this was part of Node functionality, and would also be much faster due to being part of engine code).

Furthermore, it would bring zero overhead due to the usage of context being fully optional, so it's fully backward-compatible.

# context.gd - global singleton
extends Node

var contexts = {}

func register_provider(provider: Node, context: Dictionary):
    # Register "Provider" (node that provides some context, i.e. key/value pairs)
    contexts[provider] = context
    # Unregister automatically
    provider.connect('tree_exited', self, 'unregister_provider', [provider])

func get_value(consumer: Node, key: String):
    # Find closest parent provider that defines requested key
    # We want to take value from the deepest parent Provider, since Providers can be nested
    var topmost_level = 0  # Find provider with highest level of nesting
    var value

    for provider in contexts:
        var context = contexts[provider]
        if provider.is_a_parent_of(consumer):
            # Consumer is (in)direct child of this provider
            var provider_level = provider.get_path().get_name_count()
            if provider_level > topmost_level and key in context:
                # Provider defines the value we want, and is closer to us than other providers
                topmost_level = provider_level
                value = context[key]
    if topmost_level == 0:
        # No providers found (or none provide the requested value)
        push_warning('No providers found for key %s (requested by node %s)' % [key, consumer.get_path()])
    return value

func unregister_provider(provider: Node):
    contexts.erase(provider)

Usage:

# room1 node:
extends CanvasModulate
func _enter_tree():
    Context.register_provider(self, {"nav_mesh": $nav_mesh_in_room_1})
    Context.register_provider(self, {"ai_reaction_time": 0.25})

# room2 node:
extends CanvasModulate
func _enter_tree():
    Context.register_provider(self, {"nav_mesh": $nav_mesh_in_room_2})
    # Room with lights turned off, slower reaction time:
    Context.register_provider(self, {"ai_reaction_time": 1.0})

# ai node (located arbitrarily deep inside both room1, room2, and other places):
extends Node2D
# Dependency injection without explicit NodePaths, yay!
onready var nav_mesh: NavigationMeshInstance = Context.get_value(self, "nav_mesh")
onready var nav_mesh: float = Context.get_value(self, "ai_reaction_time")

Explanation:

Here's what it could look like if this was a built-in functionality (API can be similar to (has|set|get)_meta):

# room1 node:
extends CanvasModulate
func _enter_tree():
    set_context("nav_mesh", $nav_mesh_in_room_1)
    set_context("ai_reaction_time", 0.25)

# room2 node:
extends CanvasModulate
func _enter_tree():
    set_context("nav_mesh", $nav_mesh_in_room_2)
    # Room with lights turned off, slower reaction time:
    set_context("ai_reaction_time", 1.0)

# ai node (located arbitrarily deep inside both room1, room2, and other places):
extends Node2D
onready var nav_mesh: NavigationMeshInstance = \
    get_context("nav_mesh")  # Print error if not defined, similarly to get_meta
# Failsafe example:
onready var ai_reaction_time: float = \
    get_context("ai_reaction_time") if has_context("ai_reaction_time") else 0.5

Taking it even to an even more extreme level, there could even be a syntax supporting this:

WARNING - yet another quirky yolo-syntax proposal below. Sorry, I just couldn't get it off my head.

# room1 node:
extends CanvasModulate
func _enter_tree():
    # "Wow, another useless syntax!" - But isn't it pretty?
    &nav_mesh = $nav_mesh_in_room_1
    &ai_reaction_time = 0.25

# room2 node:
extends CanvasModulate
func _enter_tree():
    &nav_mesh = $nav_mesh_in_room_2
    # Room with lights turned off, slower reaction time:
    &ai_reaction_time = 1.0

# ai node (located arbitrarily deep inside both room1, room2, and other places):
extends Node2D
onready var nav_mesh: NavigationMeshInstance = &nav_mesh  # Print error if not defined, similarly to get_meta

Okay, the last example is probably too extreme. Please disregard that.

So, I think (has|set|get)_context would be an awesome addition with a clean, familiar interface (being very similar to meta functionality.) Let me know what you think!

kleonc commented 2 years ago

func get_value(consumer: Node, key: String):
    # Find closest parent provider that defines requested key
    # We want to take value from the deepest parent Provider, since Providers can be nested
    var topmost_level = 0  # Find provider with highest level of nesting
    var value

    for provider in contexts:
        var context = contexts[provider]
        if provider.is_a_parent_of(consumer):
            # Consumer is (in)direct child of this provider
            var provider_level = provider.get_path().get_name_count()
            if provider_level > topmost_level and key in context:
                # Provider defines the value we want, and is closer to us than other providers
                topmost_level = provider_level
                value = context[key]
    if topmost_level == 0:
        # No providers found (or none provide the requested value)
        push_warning('No providers found for key %s (requested by node %s)' % [key, consumer.get_path()])
    return value

func unregister_provider(provider: Node):
    contexts.erase(provider)

A little simpler (both conceptually and computationally):

func get_value(consumer: Node, key: String):
    var context_provider: Node = consumer
    while context_provider:
        var context = contexts.get(context_provider)
        if context != null and key in context:
            return context[key]
        context_provider = context_provider.get_parent()
    # No providers found (or none provide the requested value)
    push_warning('No providers found for key %s (requested by node %s)' % [key, consumer.get_path()])
    return null

Note that if the context you're proposing would be added I think the implementation would look something like that. There's no point in storing a "merged context" or something like that for each node, it'd be too much of an overhead. It's a matter of checking subsequent nodes up the tree for the given value and returning the first one found (if any).


If only String keys would be enough then it could be achieved using already available metadata in the Object class. Of course Object have no concept of hierarchy, so it makes no sense for it to have a method like get_meta_recursively (or a recursive parameter added to get_meta). But I guess adding utility methods like get_meta_recursively/has_meta_recursively to Node would fulfill this use case (they would just call the relevant methods up the tree). Whether adding something like that to the core is worth/needed I'm not sure.

It would also have another limitation as for Object's metadata null is treated as an invalid value, meaning null can't be stored as a meta's value (e.g. set_meta("meta_name", null) is equivalent to remove_meta("meta_name")).

Currently e.g. something like this can be done:

class_name Meta # Exposing it as a globally available AutoLoad could be done instead if preferred.

static func is_in_context(context_provider: Node, meta_name: String) -> bool:
    return _get_meta_owner_in_context(context_provider, meta_name) != null

static func get_from_context(context_provider: Node, meta_name: String, default_value = null):
    var meta_owner: Node = _get_meta_owner_in_context(context_provider, meta_name)
    if meta_owner != null:
        return meta_owner.get_meta(meta_name)
    if default_value == null:
        # As for Object's meta error only if no default value was provided (and treat `null` as an invalid value).
        push_error('No meta named %s found in the context of node %s' % [meta_name, context_provider.get_path()])
    return default_value

static func _get_meta_owner_in_context(context_provider: Node, meta_name: String) -> Node:
    while context_provider and not context_provider.has_meta(meta_name):
        context_provider = context_provider.get_parent()
    return context_provider

And your example @and3rson using it would look like:

# room1 node:
extends CanvasModulate
func _enter_tree():
    set_meta("nav_mesh", $nav_mesh_in_room_1)
    set_meta("ai_reaction_time", 0.25)

# room2 node:
extends CanvasModulate
func _enter_tree():
    set_meta("nav_mesh", $nav_mesh_in_room_2)
    # Room with lights turned off, slower reaction time:
    set_meta("ai_reaction_time", 1.0)

# ai node (located arbitrarily deep inside both room1, room2, and other places):
extends Node2D
onready var nav_mesh: NavigationMeshInstance = \
    Meta.get_from_context(self, "nav_mesh")  # Print error if not defined, similarly to get_meta
# Failsafe example:
onready var ai_reaction_time: float = \
    Meta.get_from_context(self, "ai_reaction_time", 0.5)

Of course it has the annoyance of needing to call Meta's static method and having to pass self/whatever reference to it (contrary to the potential built-in methods added to Node).

So since "metadata system" is already in place I think there's probably no need to add a whole new "context system". As mentioned "context system" functionality could be achieved by extending the "metadata system" by adding methods like get_meta_recursively/has_meta_recursively (or whatever the names would be) to Node. Whether metadata's String-only keys and non-null values are sufficient is probably a separate topic to discuss.

and3rson commented 2 years ago

@kleonc This makes a lot of sense, thanks for the feedback!

(get|has)_meta_recursively surely could solve this. Even more, (get|has)_meta could have a third/second optional argument recursive which is false by default. I'm sure there would be a lot of use cases for this.

The only downside that I see with using "meta" for this is that it might persist when someone decides to save the scene during runtime (or from an addon), which would potentially lead to issues when loading the scene later. In other words, technically "meta" functionality is the best fit here, but conceptually it's not a correct use for it (since context values are conceptually not a part of node's metadata.)

kleonc commented 2 years ago

(get|has)_meta_recursively surely could solve this. Even more, (get|has)_meta could have a third/second optional argument recursive which is false by default. I'm sure there would be a lot of use cases for this.

As already mentioned (get|has)_meta are defined in Object and Object has no concept of hierarchy (contrary to Node which has get_parent/get_child/etc. methods). So from the Object's point of view terms like "recursive", "parent", etc. have no meaning. In the context (!) of Node methods like (get|has)_meta_recursively do make sense (they could be named differently of course).

The only downside that I see with using "meta" for this is that it might persist when someone decides to save the scene during runtime (or from an addon), which would potentially lead to issues when loading the scene later. In other words, technically "meta" functionality is the best fit here, but conceptually it's not a correct use for it

Good point, indeed I haven't thought of the metadata persistence.

and3rson commented 2 years ago

Now that I've thought about this even more, here's another extremely convenient usage of this feature.

In editor, a new option could be added for any field:

sshot1

Clicking "Bind to context value" would then show a pop-up where a name of a context value can be entered. When entered, it can be serialized similarly to how _edit_pinned_properties_ is. Let's say we want to "bind" jump_speed to some context value called global_jump_speed:

[node name="kinematic" type="Node2D"]
script = SubResource( 19 )
__meta__ = {
"_edit_pinned_properties_": [ "max_accel" ],  # <- "Pin value" example (current functionality)
"_context_binds_": {"jump_speed": "global_jump_speed"}  # Suggested way of serializing the context value
}
max_accel = 2500

Here's how it could be displayed in the editor:

sshot2

Then, in one of the parents of this "kinematic" node:

func _enter_tree():
    self.set_context("global_jump_speed", 1234)
    # Method 1: add "kinematic" (or any other scene that has "kinematic" as child) as child node in editor
    # Method 2: instantiate kinematic in GDScript
    var kinematic = preload('res://scenes/kinematic.tscn').instance()
    self.add_child(kinematic)

Now, once "kinematic" is entering the tree (or becoming ready), it can lookup the context value for jump_speed by name global_jump_speed, retrieve value 1234 from there, and set jump_speed with it.

Why?

I feel like this would be a nice Godot-ish addon to the editor: any node could have its properties initialized with dynamic context-provided values without any additional code or boilerplate things like func _ready(): self.jump_speed = get_jump_speed_from_somewhere(). Additionally, this would make dependency injection extremely simple.