godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.26k stars 101 forks source link

Add `@child_of` class annotation to enforce a node having a particular parent type #12587

Open Meorge opened 4 weeks ago

Meorge commented 4 weeks ago

Describe the project you are working on

Various projects and the Godot engine

Describe the problem or limitation you are having in your project

Sometimes, it is helpful to create a custom node class that acts on its parent, so that a unified behavior can be applied to a variety of different nodes/scenes even though that behavior may not be "core" to the scenes.

A practical example that I've run into with projects is performing a "shake" effect on various 2D objects. I will create a "Shaker" node, with code something like the following:

class_name Shaker
extends Node

func shake():
    for i in 10:
        # get_parent() here is only guaranteed to be a Node,
    # so accessing position doesn't benefit from static typing.
        get_parent().position.x += randf_range(-3, 3)
        get_parent().position.y += randf_range(-3, 3)
        await get_tree().process_frame

This code can't benefit from static typing, though, because get_parent() is only guaranteed to return Node. If I want to be able to use this node for both Node2Ds and Controls, then I can't store the return result of get_parent() in a variable of a given type (#12586 may be able to help with this).

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

Implement a new class-level annotation, @child_of(Type). This will tell Godot that the given class expects its parent node to be the given type. From this:

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

@child_of(Node2D)
class_name Shaker
extends Node

func shake():
    for i in 10:
        # get_parent() here is guaranteed to be a Node2D.
        get_parent().position.x += randf_range(-3, 3)
        get_parent().position.y += randf_range(-3, 3)
        await get_tree().process_frame

With #12586, this could even be made to support Node2D and Control parents simultaneously:

@child_of(Positionable2D)
class_name Shaker
extends Node

func shake():
    for i in 10:
        # get_parent() here is guaranteed to be Positionable2D.
        # Thus, it would work for both Node2D- and Control-inheriting nodes.
        get_parent().position.x += randf_range(-3, 3)
        get_parent().position.y += randf_range(-3, 3)
        await get_tree().process_frame

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

It can be worked around with more scripting:

class_name Shaker
extends Node

func shake():
    if get_parent() is Node2D:
        var parent: Node2D = get_parent()
        for i in 10:
            parent.position.x += randf_range(-3, 3)
            parent.position.y += randf_range(-3, 3)
            await get_tree().process_frame
    elif get_parent() is Control:
        var parent: Control = get_parent()
        for i in 10:
            parent.position.x += randf_range(-3, 3)
            parent.position.y += randf_range(-3, 3)
            await get_tree().process_frame

However, as you can see this requires a lot of code duplication. Furthermore, the IDE code completion and documentation suggestions will not reflect how this node is meant to be used - get_parent() would show up as returning Node, even though for all practical purposes, the user wants it to only specifically return Node2D and/or Control since the central purpose of this node is to act on properties specific to those subclasses.

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

Annotations can't be added via addons, nor can the behavior of the GDScript compiler/interpreter/language server/etc be modified.

Meorge commented 4 weeks ago

I've had this idea rolling around in my mind for a while now, but was tenative about posting it because I wasn't sure how well it meshed with the "official" intended strategy of scene composition. If there is a "Godot-ier" way to do things like the Shaker example, please let me know! 🙂

ydeltastar commented 4 weeks ago

Sounds like the purpose of _get_configuration_warnings().

Meorge commented 4 weeks ago

_get_configuration_warnings() can provide an in-editor warning if the parent type doesn't match, but can't provide any guarantees about the return type of get_parent(), which is the most important part to me for this use case. It would also require adding a lot of repetitive boilerplate code to any nodes that hope to use this composition behavior.

KoBeWi commented 4 weeks ago

Making a method return different type based on an annotation would require some unreasonable hacks in the language. Though it could work if we had support for generics.

Here's a more robust and safe workaround:

@onready var parent: Node2D = get_parent()

parent is guaranteed to be Node2D, so the lines using it will be safe. Also wrong parent type will result in hard error.

Lazy-Rabbit-2001 commented 4 weeks ago

I should say that for guaranteeing a node from being attached under other types of nodes, you can use a @tool script. For completion, consider EditorPlugin.