godotengine / godot-proposals

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

Add relevant traits to existing native classes (once traits are implemented) #12586

Open Meorge opened 4 weeks ago

Meorge commented 4 weeks ago

Describe the project you are working on

Various games and the Godot engine

Describe the problem or limitation you are having in your project

There have been times I've wanted to access properties or methods of an object that could be one of a few different types, where the properties/methods aren't all covered by a single ancestor class.

The Node2D.position and Control.position properties are examples that come to mind for me; it would be nice to be able to access and modify the position of a node whether it inherits from either of those two base classes.

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

Once GDScript traits have been merged, create an API that allows native/C++ classes to be marked as implementing traits. Then, particular traits could be defined and applied to existing classes.

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

The Node2D and Control classes could implement a trait Positionable2D. If the classes were written in GDScript, they might look like so:

class Node2D extends CanvasItem uses Positionable2D:
    ...

class Control extends CanvasItem uses Positionable2D:
    var size: Vector2 = Vector2.ONE
    ...

trait Positionable2D:
    var position: Vector2 = Vector2.ZERO

Then, a GDScript user could do something like the following:

func move_node(some_node):
    if some_node is Positionable2D:  # some_node could be a Node2D or a Control
        some_node.position += Vector2(3, 6)

I'm not entirely certain how the C++ API for assigning traits to classes would work at this point. If this idea gains traction, we can discuss that in more detail.

Possible groupings

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

Frequently, yes. For my example above, I think the same thing could be accomplished with multiple is checks:

if some_node is Node2D or some_node is Control:
    some_node.position += Vector2(3, 6)

This exact approach might fall apart for more complex operations, and if the user wants to ensure static typing, since some_node's type can't be defined as a union of either Node2D or Control.

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

I don't believe this would be possible to implement as an addon.

Mickeon commented 4 weeks ago

I was going to make a proposal like this eventually. It would benefit many places, although I do think that the example provided in this proposal does not showcase this well enough. Both Node2D and Control already inherit CanvasItem so a lot of their internals are actually quite similar.

What's a more powerful use case of built-in Traits is for classes with entirely different inheritance. AudioStreamPlayer & co., AnimatedSprite2D & AnimatedSprite3D, Label & Label3D, some InputEvent resources, etc. These classes share many aspects. Even though some can't be fully integrated into a trait (Vector2 vs. Vector3 properties), the potential is there.

I do think that this is an issue that should be first solved internally. The existing codebase has almost concept of multiple inheritance, so code is duplicated all over. An effort was made in the past to unify the audio stream players' logic, but it isn't quite ideal because the calling functions are still, inevitably, duplicated.

Meorge commented 3 weeks ago

Funnily enough, while frustration with Node2D and Control were what motivated me to actually sit down and write this proposal, I think the AudioStreamPlayer collection of nodes is what I've run into this problem more with before!

I can start putting together a list of nodes grouped by shared behaviors on this proposal. That way, if/when we have some sort of API for making native classes compatible with traits, we have some traits to implement.