godotengine / godot-proposals

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

Add a Trait system for GDScript #6416

Open vnen opened 1 year ago

vnen commented 1 year ago

Describe the project you are working on

The GDScript implementation.

Describe the problem or limitation you are having in your project

The main to reuse code in GDScript is via inheritance. One can always use composition, either via instances stored in class variables or as nodes in the scene tree, but there's a lot of resistance in adding a new prefix to accessing some functionality in the class, so many go to the inheritance route.

That is completely fine if the inheritance tree includes related scripts. However, it's not uncommon for users to create a library of helper functions they want to have available in every script, then proceed to make this the common parent of everything. To be able to attach the sub-classes to any node, they make this common parent extend Node (or even Object) and rely on the quirk of the system that allows attaching scripts to a sub-class of the type it extends (cf. godotengine/godot#70956).

The problem with this usage is that a script gets a bit of "multiple inheritance" which is not really supported by GDScript. One script can now extend another that ultimately extends Node but it can also use the methods of, say, a Sprite2D when attached to a node of such type, even when not directly on the inheritance line.

Here I put a more concrete example to understand the issue:[^self]

# base.gd
extends Node
class_name Base

# -------
# sprite_funcs.gd
# Attached to a Sprite2D.
extends Base
func _ready() -> void:
    # Here using `self` as a hack to use dynamic dispatch.
    # Because GDScript won't know what `centered` is, since it's not on the inheritance tree
    # and it does not know the attaced node.
    self.centered = true

# -------
# body_funcs.gd
# Attached to a RigidBody2D
extends Base

# This method can't be validated in signature to match the super class since RigidBody2D is not being inherited.
# So it won't catch a potential user error.
func _integrate_forces(state: PhysicsDirectBodyState2D) -> void:
    print(state)

All of this works against the user since they lose optimizations and compile-time checks for potential errors. It's a bigger loss if they use static types since not knowing the actual base class will make the compiler not know the types of properties and methods, making the checks less reliable.

Another problem for users of static typing is checking if a particular method implements a given interface. You can use has_method() but that's often not enough since you can't tell what the signature of the method is. Even with introspection, it would require a lot of boilerplate for each check and you still lose the benefits of static type check. You can only check for inheritance but since that is linear you cannot use for a class that needs to implement multiple interfaces.

Previous requests for a feature like this, or problems that could be solved by this feature:

[^self]: As an extra note, I would like to make self be transparent and work the same as not using it (unless used for scope resolution), since some people like to use it just for clarity and end up losing performance optimizations without realizing. But this is not related to this proposal in particular.

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

For solving this, I propose the creation of trait in GDScript.

What is a trait?

It is a reusable piece of code that can be included in other scripts, parallel to inheritance. The included code behave pretty much as copy and paste, it adds to the existing code of the script, not unlike the #include directive in C/C++. Note that the compiler is aware of traits and can provided better error messages than an actual #include would be able to.

Its contents is the same thing that can be included in a script: variables, constants, methods, inner classes.

Besides that, it can include method signatures, which are methods without an implementation defined, making the trait acting like an interface.

How is a trait defined?

In its own file, like a GDScript class. To avoid confusion, I propose using a new file extension for this, like .gdt (for GDScript Trait), so it won't be usable for things that requires a script. For instance, you won't be able to attach a trait file to a Node directly. I'm still considering whether trait files should be resources. They will only be used from the GDScript implementation, so the resource loader does not need to be aware of them. OTOH, it might make the filesystem scan more complex. This will be investigated further during implementation.

Traits can also be nested in trait files or in GDScript files, using the trait keyword:

# script.gd
class_name TraitCollection

trait NestedTrait:
    pass

That would be accessible with TraitCollection.NestedTrait.

Example of a trait file:

# trait.gdt
const some_constant = 0
var some_variable := "hello"
enum Values {
    Value1,
    Value2,
}

func some_func_with_implementation() -> void:
    print("hello")

func some_abstract_function() -> void # No colon at end.

# This is independent of the outer trait, it won't be automatically used.
trait InnerTrait:
    pass

I consider also using a trait_name keyword that would work similar to the class_name keyword, making the trait available by name in the global scope. I prefer not to reuse class_name for this as I believe it might be confusing, since the trait file is not a class.

Trait files cannot contain inner classes.

Traits can use the extends keyword to select a base class. That will mean that any script that does not derive from this class cannot use the trait:

# trait.gdt
trait_name MyTrait
extends Node2D

func flip():
    rotate(PI) # Works because `rotate()` is defined in Node2D.

# node3d.gd
extends Node3D
implements MyTrait # Error: MyTrait can only be used with scripts that extends Node2D.

# sprite2d.gd
extends Sprite2D
implements MyTrait # Valid: Sprite2D inherits from Node2D

The extends keyword can also be used with a custom GDScript type. Traits can also override methods of the base class. This means they need to match in signature and will give an error otherwise. Note that for native classes the overriding of methods can be unpredictable as the overrides won't be called internally in the engine, so this will give a warning (treated as error by default) as it does for when this happen with a GDScript class.

How to use a trait?

I propose adding the implements keyword[^implements]. This should be added at the beginning of the class, together with extends and class_name.

# file.gd
extends Node.gd
implements "trait.gdt"
implements GlobalTrait
implements TraitCollection.NestedTrait

Using multiple traits will include them all in the current class. If there are conflicts, such as the same method name declared on multiple used traits, the compiler will give an error, also shown in-editor for the user. If the current class has a method with the same name as one used trait, it will be treated as an override and will be required to match signature. For variables, constants, and enums, duplication won't be allowed at all, as there are no way to override those. If multiple traits has the same variable, then it will be considered a conflict only if they have different types.

Conflicts between trait methods can be resolved by implementing an override method in the script and calling the appropriate trait method:

# trait_a.gdt
trait_name TraitA
func method(a: int) -> int:
    return 0

# trait_b.gdt
trait_name TraitB
func method(a: int) -> int:
    return 1

# script.gd
implements TraitA, TraitB
func method(a: int) -> int:
    return TraitA.method(a) # Or:
    # return TraitB.method(a)

Traits can also use other traits, allowing you to extend on an existing trait or create "trait packages" so you only need to use once in the classes and it includes many traits. It is not a conflict if multiple used traits happen to use the same trait. The nested inclusion won't make it being included twice (which differs from how #include works in C/C++).

Once a trait is used, its members can be accessed as if belonging to the same class:

# trait.gdt
var foo = "Hello"
func bar():
    print("World")

# script.gd
implements "trait.gdt"
func _init():
    print(foo) # Uses variable from trait.
    bar() # Uses method from trait.

Are traits considered types?

Yes. Traits can be used as type specifications in variables:

var foo: SomeTrait

They can also be used in type checks and casts:

var foo := value as SomeTrait
if bar is OtherTrait:
    print("do stuff")

To avoid polluting the global scope, the traits can also be preloaded like classes:

const SomeTrait = preload("traits/some_trait.gdt")
var foo: SomeTrait
if foo is SomeTrait:
    print("yes")

Warning: One thing to consider when using traits as types is that all are considered to be a basic Object with a script attached. That is, even if the instance you have extends, say, Node2D, you won't be able to use the static type checks for Node2D members. That also means that optimizations done by GDScript to call native methods faster won't be applied, since it can't guarantee at compile time that those will be available without knowing the actual super class. Using extends in the trait helps when it's meant to be applied to a particular type, as it will hint the compiler to what is available.

Warning: Consider too that when checking for traits with is and as, the check will be for the use of the exact trait. It won't check if the object implements the same interface as the trait. As an example:

# trait.gdt
func foo(bar: int) -> void

# a.gd
class_name A
func foo(bar: int) -> void:
    print(bar)

# b.gd
const MyTrait = preload("trait.gdt")
func _init():
    var a := A.new()
    print(a is MyTrait) # Prints 'false'.

While the class A implements a method foo with the same signature, it does not use "trait.gdt" so it won't be considered to be this trait when checking with is nor when casting with as. This is a technical limitation since performing an interface validation at runtime would be costly, and not validating at runtime would be unpredictable for users.

[^implements]: Note that implements won't be a reserved word to avoid breaking compatibility. The context will be enough to not confuse it with and identifier.

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

Internally, a trait file will be treated a bit differently from a script file, to allow the little differences in syntax (e.g. methods without body). They will use the same parser since the differences are small, but will be treated differently when relevant. This does add a burden for the parser, though I believe it is better than using a different parser that would do most of the same.

The analyzer, which is responsible for type-checking, will also run on traits. They will be required to be internally consistent on their own, even before being used in a class. This allows the errors to be detected when writing the trait, rather than when using it.

Fully defined functions will be compiled and stored in a GDScriptTrait object. This object won't be exposed to scripting and only used internally. It will be used for caching purposes to avoid compiling the same trait multiple times. The defined functions will be copied to the GDScript object that implements the trait, so it won't have any reference to the original GDScriptTrait object.

Used traits will be stored in the GDScript object as a set of strings. The stored string is the FQN (fully qualified name) that uniquely identifies the trait (essentially the file path plus identifiers for nested traits). This will be used for runtime trait checks and casts, created by the is and as keywords.

During parsing, the used traits will be collected and referenced in the resulting parse tree. The parse trees won't be directly mixed in order to allow the parser to detect the origin of code and thus provide more specific error messages.

The analyzer phase will further check if there are no conflicts and if types are correct, including checks for trait types. It will give specific errors and warnings where appropriate.

The GDScriptCache will be responsible for keeping the the GDScriptTrait objects and intermediary compilation steps when necessary. After a GDScript is compiled, the trait cache will be purged. This cache is only used for the compilation of a single file and re-used for multiple uses of the same trait by the script or by its dependencies (either by the implements keyword or using them as types), so after the compilation is complete it needs to be cleared as we can't know when or if other scripts will be compiled or whether they will use the same traits.

For variables/constants/enums, used traits will behave like copies and recompiled in the current class. This is so the constructor is done per class without having to rely on calling other functions compiled into traits. Usually the default values are not complex expression and it shouldn't be a problem compiling them multiple times (this case will be rare too).

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

Considering all the requests, it will likely be used often. The work around for this can be quite clunky, especially if you are trying to validate interfaces and not only reuse code.

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

It is an integral part of GDScript, it cannot be implemented externally.

trommlbomml commented 1 year ago

I am very happy with the idea how it works, exactly what I would Need and use a lot! Though one things looks a bit „wrong“ to me. Classes fulfill, Implement or provide a trait, not using it, like a derived class extends a Base class and thats why the keyword is extends, maybe we can stick Here Java like with extends for base classes and implements for traits.

Gnumaru commented 1 year ago

The link for https://github.com/godotengine/godot/pull/70956 is broken. I mean, you should have pasted the entire url for the pull request for the link to be clickable.

YuriSizov commented 1 year ago

Would abstract functions be exclusive to traits? I don't see why they would, but it's worth clarifying. If they can be declared in normal classes, then that's worth a dedicated proposal, perhaps.

vnen commented 1 year ago

I am very happy with the idea how it works, exactly what I would Need and use a lot! Though one things looks a bit „wrong“ to me. Classes fulfill, Implement or provide a trait, not using it, like a derived class extends a Base class and thats why the keyword is extends, maybe we can stick Here Java like with extends for base classes and implements for traits.

I don't think implements is a good fit because the first use case for traits is code reuse. It's expected the implementation is in the trait itself, so the class does not really "implement" anything, it actually does "use" the implementation provided. While traits can double as interfaces it won't always be the case.

Of course every name/keyword can be discussed. I believe includes could be an alternative, since it works similarly to a file inclusion.

vnen commented 1 year ago

Would abstract functions be exclusive to traits? I don't see why they would, but it's worth clarifying. If they can be declared in normal classes, then that's worth a dedicated proposal, perhaps.

@YuriSizov yes, they will be exclusive to traits. This proposal only adds them to traits, so the classes still remain without abstract methods.

Adding abstract methods would require abstract classes as well, which are not available and beyond the scope of traits. So this idea is more fit to the proposal for abstract classes: #5641 (I'm not saying it won't happen, just that it is a different discussion).

It works for traits because those already can't be instantiated. Classes using traits that have abstract methods are required to provided an implementation.

WolfgangSenff commented 1 year ago

So this is similar to an interface in other languages? I love this idea and I think you'll see massive usage of it.

jabcross commented 1 year ago

Can you expand a bit on what are the alternatives (like multiple inheritance) and the tradeoffs? To me, a trait seems almost identical to a class, except it can be used in multiple inheritance, can define interfaces, and can't be instanced. Wouldn't it be easier just to have multiple inheritance and just reuse classes in the exact same way?

jabcross commented 1 year ago

For the problem with extra prefixes when using composition, I quite like the Odin approach of importing a member's members into the namespace using the using keyword:

https://odin-lang.org/docs/overview/#using-statement-with-structs

This basically lets you do foo.x instead of foo.pos.x if you write using pos in foo's definition

okla commented 1 year ago

Great, just today I thought about the lack of such feature in GDScript)

Not sure that a new file extension is a good idea since it always complicates everything about the language: third-party tools support (IDEs, linters, build systems etc), even explaining the difference to novices. Maybe simply using a trait keyword would be enough?

I'm also not sure about theglobal_name, shouldn't it be trait_name to better match classes?

Or even more, if there will be a new kind of scripts in addition to classes, maybe declaration of both of them should be changed to something like class Foo extends Node – named class class extends Node – unnamed class trait Foo extends Node – named trait trait extends Node – unnamed trait First word in a script always points to its kind, no need for another file extension, no need for separate (class|global|trait)_name line. Probably a breaking change but a pretty simple one to be automatically applied by the project converter.

BTW in Swift this feature called protocols and they use the term "conforms to" for classes that implement them.

CarpenterBlue commented 1 year ago

My vote goes to smurfs. Really bad jokes aside, how does new file extension complicate anything? Shouldn't that be just registering new file to the IDEs and whatever else that deals with the language it's still the same GDScript no? There is no need for separate implementations or special treatment.

Anutrix commented 1 year ago

I've read the proposal and from what I can understand, this is like preloading/loading another script(https://godotengine.org/qa/53560/can-you-split-a-class-across-multiple-files-in-gdscript) but with added benefit that they'll node-type-checked when adding them to a script so that you can't accidentally add a trait meant for another class. Please correct me if I'm wrong about this.

Based on this logic, rather than uses word, I would suggest has or contains keyword for using it. Maybe is keyword also works if traits are supposed to sound like multiple inheritance logic. Like Cat script/class and Dog script/class both have Animal trait (assuming all 3 have same base class).

Note: I'm using up class/script/file interchangeably but I mean the same.

bitbrain commented 1 year ago

I really like the proposal. I can see myself using this to define "interface style" traits. I would also use this to extend the logic of some scripts.

A few questions:

okla commented 1 year ago

it's still the same GDScript no?

And that's why IMHO it doesn't need different extension. Extension hints how to read/parse a file, not how to interpret its content after parsing. For example, tscn and scn are different because the content of such files are different, textual vs binary representation of the same data.

Working with a large C project on a daily basis, it's obvious how its separation into c and h complicates things for no reason. You should always treat them differently everywhere, even though they contain exactly the same C code to the point where any program can be written without using headers at all. It can take months for people new to the language to understand this. Probably that's why most modern languages go with a single file extension for everything.

Scony commented 1 year ago
  1. To be more in line with GDScript, I'd give users a way to control when they load trait exactly, so the syntax like:

    trait SomeTrait = "traits/some_trait.gdt"

    would be replaced by:

    const SomeTrait = load("res://traits/some_trait.gdt")
    # or
    const SomeTrait = preload("res://traits/some_trait.gdt")
  2. Regarding file extension - @okla you're mistaken - .gd and .gdt are different because the root of the parse tree (global script scope) is different. A good example is an empty file. Imagine you're loading an empty file like this:

    const Empty = preload("res://Empty.gd")

    is it a class or trait? Well, you cannot tell unless you hint with .gd or .gdt. Even if an empty file would not be empty and contained some constructs like functions etc. it would still be indistinguishable from a class. So unless we want to have scripts that may act as a class or trait we need to have separate extensions.

winston-yallow commented 1 year ago

However, traits cannot have "child nodes" nor access nodes via scene tree lookup by the looks of it. What would be the best way to access nodes within a scene tree inside a trait? @bitbrain

I think traits like in this proposal can access the scene tree, as long as they extend a Node derived class. As traits could use all methods from classes they extend, this should be valid (or rather, I would wish this to be a valid trait, and I don't think anything is preventing this):

func start_idle():
    get_node("AnimationPlayer").play("idle")

I would also love if @export and similar work in traits too, and if I understood the proposal correctly that is the case since variables and so on are compiled in the context of the class script that uses the trait.

vnen commented 1 year ago

So this is similar to an interface in other languages? I love this idea and I think you'll see massive usage of it.

@WolfgangSenff it is like interfaces with some extra features.


Can you expand a bit on what are the alternatives (like multiple inheritance) and the tradeoffs? Tot me, a trait seems almost identical to a class, except it can be used in multiple inheritance, can define interfaces, and can't be instanced. Wouldn't it be easier just to have multiple inheritance and just reuse classes in the exact same way?

@jabcross Multiple inheritance has 2 issues that traits avoid: 1) The infamous diamond problem. Since traits does not inherit anything they don't carry the baggage. Traits are also flattened when included, if there are conflicts there will be an error. 2) Exactly because they don't carry the baggage of inheritance, they can be applied to classes of different types. With multiple inheritance, if you have class A extends Node2D and class B extends Node3D, if you have a class C extends B, C then to which node can you attach C? Well, you can't really attach to anything because if it uses methods from Node2D it can't be used in a Node3D without crashing, and vice-versa. The trait extends keyword is only for restricting the class that use them and not actually inheriting anything, so if you replace class for trait in my example, there will be no class that could use both traits at once.

For the problem with extra prefixes when using composition, I quite like the Odin approach of importing a member's members into the namespace using the using keyword:

odin-lang.org/docs/overview/#using-statement-with-structs

This basically lets you do foo.x instead of foo.pos.x if you write using pos in foo's definition

That could be done in parallel, since traits are not only to replace composition (it probably doesn't in many cases anyway). But that's another proposal.


Not sure that a new file extension is a good idea since it always complicates everything about the language: third-party tools support (IDEs, linters, build systems etc), even explaining the difference to novices. Maybe simply using a trait keyword would be enough?

@okla IMO it makes easier for IDEs because the files allow and forbid different things. The IDE would have to be aware of the preamble of the file before deciding what rules to apply. Also, Godot sees resources by extension, so if traits also use .gd they will use the same loader as GDScript and then it will be more complex for the loader which will have to pre-parse the file to a certain point to decide whether it is a class or a trait.

I'm also not sure about theglobal_name, shouldn't it be trait_name to better match classes?

Perhaps. I guess it would be more familiar to GDScript users.

Or even more, if there will be a new kind of scripts in addition to classes, maybe declaration of both of them should be changed to something like class Foo extends Node – named class class extends Node – unnamed class trait Foo extends Node – named trait trait extends Node – unnamed trait

It has been discussed before, it is difficult to use class because it's already used by inner classes. The parser would have to do a bunch of lookahead to disambiguate, which is more complex.

If anything, I would add a @trait script annotation (like @tool). That kind of annotation must be in the beginning of the file, so it would be easier to detect it's presence or absence.


Based on this logic, rather than uses word, I would suggest has or contains keyword for using it. Maybe is keyword also works if traits are supposed to sound like multiple inheritance logic. Like Cat script/class and Dog script/class both have Animal trait (assuming all 3 have same base class).

@Anutrix I considered includes which is synonym to contains. I guess is could work as it does not introduce a new keyword and correlates to the result of obj is Trait expression.

vnen commented 1 year ago

@bitbrain

  • what happens if two traits define the exact same function signature but with different implementations? Will this cause a compiler error? For example in Java, implementing two interfaces that each come with the same default method implementations will throw an error

I mentioned this in the proposal: the same method name in multiple included traits will give an error. This also happens if they share the signature.

  • a lot of the times, gdscript code will access nodes from the scene tree to do stuff with them. However, traits cannot have "child nodes" nor access nodes via scene tree lookup by the looks of it. What would be the best way to access nodes within a scene tree inside a trait?

You can use extends Node in the trait to limit the usage for only classes that inherit Node (directly or indirectly). This way the trait can use methods and properties available in Node.


@Scony

  1. To be more in line with GDScript, I'd give users a way to control when they load trait exactly, so the syntax like:
trait SomeTrait = "traits/some_trait.gdt"

would be replaced by:

const SomeTrait = load("res://traits/some_trait.gdt")
# or
const SomeTrait = preload("res://traits/some_trait.gdt")

The problem is that the trait is not an object, at least not how GDScript sees it, and it probably won't even be a resource. The trait won't be available at runtime, so it can't be stored into a constant. It could be done as a "syntax sugar" for the familiarity, but I feel that would be more confusing since you cannot access the trait in any way and there's nothing you can do with its "value". TBH I would prefer adding the syntax class A = "res://a.gd" as a counterpart, which I think it's pretty neat.

Also, there's no way to control when a trait is loaded because it can only be done at compile time. They will always be loaded together with the script.


@winston-yallow

I would also love if @export and similar work in traits too, and if I understood the proposal correctly that is the case since variables and so on are compiled in the context of the class script that uses the trait.

I do think this will work, unless I find some problem during implementation.

dalexeev commented 1 year ago

I would prefer trait to be a resource:

  1. const MyTrait = preload("res://my_trait.gdt") will work. This is consistent with class preloading, you can use it for type hints. uses/includes is more like extends, it doesn't need preload.
  2. No need for the slightly ugly var my_var: "res://my_trait.gd" syntax.
  3. When exporting a project, non-resource files are ignored by default. If we make .gdt a resource, then we don't need to add any exceptions.
  4. ShaderInclude is a resource. More consistency!

That also means that optimizations done by GDScript to call native methods faster won't be applied, since it can't guarantee at compile time that those will be available without knowing the actual super class.

If I understood this sentence correctly, then it seems to me that it would be better if the traits and their inclusions check if the trait's method shadows the native method:

# trait_a.gdt
# ===========
global_name TraitA
extends Object

# OK. `Object` does not have `add_child()` method, although its descendants may have it.
func add_child(node):
    pass

# trait_b.gdt
# ===========
global_name TraitB
extends Node

# The method "add_child" overrides a method from native class "Node".
# This won't be called by the engine and may not work as expected.
# (Warning treated as error.)
func add_child(node):
    pass

# script_1.gd
# ===========
extends Object
# OK. None of the `TraitA` methods shadow native methods.
uses TraitA

# script_2.gd
# ===========
extends Node
# The method "add_child" from trait "TraitA" overrides a method from native class "Node".
# This won't be called by the engine and may not work as expected.
# (Warning treated as error.)
uses TraitA
Bromeon commented 1 year ago

I would even go one step further, and discuss if "trait" is the right keyword.

Like an interface, a trait defines one or more method signatures, of which implementing classes must provide implementations.

This is also how e.g. the Rust language uses traits. But that doesn't align with this proposal -- the idea here seems to be primarily code reuse and not polymorphism/interfacing.

Instead, what do you think about "mixin"?

In object-oriented programming languages, a mixin (or mix-in) is a class that contains methods for use by other classes without having to be the parent class of those other classes. How those other classes gain access to the mixin's methods depends on the language. Mixins are sometimes described as being "included" rather than "inherited".

Mixins encourage code reuse and can be used to avoid the inheritance ambiguity that multiple inheritance can cause (the "diamond problem"), or to work around lack of support for multiple inheritance in a language. A mixin can also be viewed as an interface with implemented methods. This pattern is an example of enforcing the dependency inversion principle.

bitbrain commented 1 year ago

@Bromeon my brain actually used the term mixin in a prior comment and I had to edit my comment to replace it with "trait" - mixin definitely makes more sense here as a name.

dalexeev commented 1 year ago

I would even go one step further, and discuss if "trait" is the right keyword.

Good question. What is described in this proposal is very similar to traits in PHP ("language assisted copy and paste"), only with additional static checks.

Pennycook commented 1 year ago

I would even go one step further, and discuss if "trait" is the right keyword.

I had a similar reaction. I immediately thought of C++ type traits. I'd love a feature like type traits, but I agree that it doesn't seem to align with what is being proposed here.

I'm drawing attention to C++ specifically because Godot itself is implemented in C++, and there seems to be quite a lot of potential for confusion here.

poiati commented 1 year ago

Good job, @vnen. All the static typing systems, when implemented post a language launch (e.g. python, ruby) come with some interface concept, given that's the only way we can actually depend on abstractions instead of concrete types. I think this is REALLY important.

I don't think implementation is a good fit because the first use case for traits is code reuse. It's expected the implementation is in the trait itself, so the class does not really "implement" anything, it actually does "use" the implementation provided. While traits can double as interfaces it won't always be the case.

I want to discuss this further :point_up:. I personally like the implements keyword more. Look at rust traits, this is the closest implementation I can refer to based on your spec. The baseline is:

so the class does not really "implement" anything

It DOES, even if you provide a default implementation from the trait, it still implements that trait and thus, from a typing system perspective it can act as an object of that type.

I dislike uses as it is not really bound to the concept of interfaces or protocols. The way I see uses being used through history is more related to Mixins, importing other modules, etc...

You are right that your proposal can be used in two ways:

  1. Abstract behaviors into interfaces/protocols (today that's just possible through duck typing, we need to "bypass" the typing system)
  2. Reuse concrete method definitions (sort of Mixin like).

IMO, usage number 1 is by far the most important point of this, so maybe we should build this feature around that concept. implements would make much more sense in that case.

poiati commented 1 year ago

Mixins are usually conceptually different than protocols or interfaces. Mixins can be implemented anywhere and do not require a type system (look at Ruby, PHP...). On the other hand, interfaces do not make any sense in a language without static typing (see duck typing).

Please, let's not do just Mixins here, or name it like that, that's not what we need (IMO, of course). We need a more refined concept, like the one proposed by the author here, IMO we might need to change some keywords though, like the use one. The concept of traits as defined here can be used in both ways, but we must design it in a way this is not confusing.

winston-yallow commented 1 year ago

If there are conflicts, such as the same method name declared on multiple used traits, the compiler will give an error

I am not sure how I feel about this. I think it should only be a warning, and have a clearly defined order of lookups. So a function call would have a precedence like this:

  1. Override in class script
  2. Implementations from traits
    • traits included later override implementation from traits included earlier
  3. Default implementations from extended base class

To illustrate this more, let's take this example

extends Base
uses Foo
uses Bar

Calling a function would first look in this script for the function, then in Bar, after that in Foo and finally in the Base

2plus2makes5 commented 1 year ago

Wow i had a similar proposal that for some reason i have never submitted, but i had no idea of the existence of trait/mixin!

I have a doubt though, imo a trait should be basically a shortcut to copy-pasting a piece of code to another piece of code plus the addition of some functions like has_trait(), get_trait_list() and similar. Simple and clean and can be beneficail to a vast amount of situations.

Admittely i'm ignorant about trait/mixin, but is it really a good idea to treat traits as types? It seems to me that creating trait variables would make things a lot more complex, confusing and error prone only to get some advantages in some situations. Am i wrong?

winston-yallow commented 1 year ago

Admittely i'm ignorant about trait/mixin, but is it really a good idea to treat traits as types @2plus2makes5

That kinda is required to make static typing work. If you can't use traits as types, then there would be no way to get static typing for arbitrary objects of potentially different classes that all implement the same trait.

Creating trait variables wouldn't be required, it would just be something that people can do if they want to. And as with all typing in Godot, it's optional. So you don't need to use any of these features, you can just use traits to re-use some method implementations without worrying about using trait types.

dalexeev commented 1 year ago

How will this work with user documentation? There are no interfaces in Godot API (ClassDB, MethodInfo, PropertyInfo). It turns out that the documentation system should be expanded?

Yagich commented 1 year ago

Unless I misunderstand, I think for documentation it would be fairly straightforward - add a new heading for classes, something like "Traits implemented" that just link to doc pages for these traits, which would be parsed as global classes do for docs?

dalexeev commented 1 year ago

I think for documentation it would be fairly straightforward - add a new heading for classes, something like "Traits implemented" that just link to doc pages for these traits

Are traits considered types?

Yes. Traits can be used as type specifications in variables:

var foo: SomeTrait

Also, as far as I understand, a trait can be used as a type hint in a function signature.

2plus2makes5 commented 1 year ago

Admittely i'm ignorant about trait/mixin, but is it really a good idea to treat traits as types @2plus2makes5

That kinda is required to make static typing work. If you can't use traits as types, then there would be no way to get static typing for arbitrary objects of potentially different classes that all implement the same trait.

Creating trait variables wouldn't be required, it would just be something that people can do if they want to. And as with all typing in Godot, it's optional. So you don't need to use any of these features, you can just use traits to re-use some method implementations without worrying about using trait types.

Thanks for the explanation!

winston-yallow commented 1 year ago

How will this work with user documentation? There are no interfaces in Godot API (ClassDB, MethodInfo, PropertyInfo). It turns out that the documentation system should be expanded?

Not sure if maybe some of the current systems can be repurposed, but otherwise it would be possible to have a TraitDB similar to the ClassDB. Though I think the easier way would be to just use the class DB for classes and traits.

vnen commented 1 year ago

I am not sure how I feel about this. I think it should only be a warning, and have a clearly defined order of lookups. So a function call would have a precedence like this:

  1. Override in class script
  2. Implementations from traits

    • traits included later override implementation from traits included earlier
  3. Default implementations from extended base class

@winston-yallow this could work, but only if method signature matches. If the methods have different signatures then there's no way to reconcile between different traits. I still think it's a mistake to rely on this behavior, it's more likely that you are getting an override without realizing.


How will this work with user documentation? There are no interfaces in Godot API (ClassDB, MethodInfo, PropertyInfo). It turns out that the documentation system should be expanded?

@dalexeev I had not thought about this TBH. I do believe we can add pages for traits and just update the doc system in-editor to allow the use of "Trait" in the description instead of "Class". For all other intents and purposes it pretty much behaves like a class.

For usages of the trait it might be more complicated. Ideally it would have a section for this, like extends do. However, if it's not possible, we might be able to hijack the description field to include the traits used by the class.

winston-yallow commented 1 year ago

this could work, but only if method signature matches. If the methods have different signatures then there's no way to reconcile between different traits. I still think it's a mistake to rely on this behavior, it's more likely that you are getting an override without realizing. @vnen

I'm aware that the signatures would need to match. But why do you think it's still a mistake to rely on this behaviour?

To give a usecase:
I could imagine an addon providing an Interactable trait. But users of that addon may want a slightly different implementation for one method in that trait. So they create their own trait FancyInteractable, and they don't want to re-implement all methods from the addons Interactable trait. Just one of the provided methods.

With what I proposed, they could do this to have all methods from the addons Interactable trait, and the one override from their own FancyInteractable:

# addon/interactable.gdt
global_name Interactable
func a(): return "A"
func b(): return "B"
func c(): return "C"

# fancy_interactable.gdt
global_name FancyInteractable
func a(): return "Foo Bar"

# node.gd
extends Node
use Interactable
#warning-ignore:trait-method-override
use FancyInteractable

I think this mostly is a concern when using traits provided by external stuff like addons, because if you have full control over your traits you could probably structure them in a way that does not require overrides. So I think it should be possible to do this, but have the default warning be treated as error. It should be manually set to warning if people are sure they want to use this.

vnen commented 1 year ago

@winston-yallow in this case you should make the FancyInteractable trait use the Interactable trait, which would allow it to override the method. Traits can override methods of other traits they use, just like a class can override trait methods. Then you only need to use the FancyInteractable in the class. It will pass the is Interactable test as well.

The problem with allowing multiple methods, even if signature matches, is that there is no clear preferred method to select. The order of inclusion is just too flimsy to rely on and might make some situations impossible anyway (if there are multiple "duplicate" methods across traits, there might be no order that satisfies your preferences).

You can think of "trait" as the English word: a characteristic. When a class uses a particular trait, it's adopting that characteristic (which comes packed with functions, not much unlike inheritance). If a class uses two traits, it has both characteristics at once. Those two traits are more likely to be completely unrelated to each other, so if they define a method with the same name, it probably means they are used in different contexts and might have very different implementations. In this case, there is no "preferred" implementation, you would just want to use one or the other in different contexts. Since many times this context is unpredictable, it's better to give an error at the source.

The main exception to this is when the traits are supposed to be interchangeable, like your example. But in this case it's a better practice to make one trait use the other, so the override is done intentionally (and linearly as well).

The only way I believe this would somewhat valid, is if you have multiple traits that "inherit" from the same one and want to mix and match the functions:

# addon/interactable.gdt
global_name Interactable
func a(): return "A"
func b(): return "B"
func c(): return "C"

# fancy_interactable.gdt
global_name FancyInteractable
uses Interactable
func a(): return "Foo Bar"

# super_interactable.gdt
global_name SuperInteractable
uses Interactable
func a(): return "Super"
func b(): return "Duper"

# node.gd
extends Node
uses SuperInteractable
uses FancyInteractable

Although, even in this case, if something expects the SuperInteractable type, having the FancyInteractable functionality may be unwanted.

I believe the only acceptable way to allow this would be to have explicit selection of the preferred method, similar to PHP's insteadof operator.

winston-yallow commented 1 year ago

@vnen ah I see, then I kinda misunderstood the meaning of "multiple traits can't contain the same method". If traits can override methods from traits they use, then this is indeed not a concern.

I'm not sure if php like explicit selection is needed, considering that this is probably an edge case without much actual world usage.

vonagam commented 1 year ago

What is presented here matches term Mixin better than Trait, wiki links for concepts - Mixin, Trait.

Two main advantages of real traits over proposed ones:

No requirement for absence of name collisions of members between Traits. Such requirement, for example, makes an addition of any new member to a public Mixin in an addon a potentially breaking change for a user.

Ability to specify Trait implementation for not owned classes (or for owned but still separately) and built-in types which makes it much more powerful.

I would have preferred to have proper traits, but when looked just as an implementation of the mixin pattern then it looks pretty solid. Good to have too.

vnen commented 1 year ago

@vonagam

What is presented here matches term Mixin better than Trait, wiki links for concepts - Mixin, Trait.

AIUI "mixin" is exclusively about reusing code, not with interface/protocol behavior (i.e. there's not is a relationship). While "trait" includes both code reuse and interface behavior, which is what I'm proposing here.

Two main advantages of real traits over proposed ones:

No requirement for absence of name collisions of members between Traits. Such requirement, for example, makes an addition of any new member to a public Mixin in an addon a potentially breaking change for a user.

In the wiki page itself it says that naming conflicts must be explicitly disambiguated, which I also mentioned in my previous comment as potential solution for this case. I just don't think for now it's worth the trouble and I'm also not sure about the proper syntax for this. I'd rather wait until the use case is concrete.

Also, inheritance can also break this way too. If a super class includes a new method with the same name but different signature, it will create a new error in the user code. With traits, they must have some disambiguation even if implicit, so the user might suddenly get a method replaced without realizing (or an error if disambiguation must be explicit, which is much less surprising).

Ability to specify Trait implementation for not owned classes (or for owned but still separately) and built-in types which makes it much more powerful.

I don't see where this is a required characteristic of traits. Sure some languages can do it, but I don't think this is implied from the "trait" name.

This particularly difficult to implement in GDScript because unlike most common languages, GDScript does not see the whole project when compiling. Instead, each file is compiled individually when loaded (plus its dependencies). So if some "extension trait" is not part of the dependency chain it will not be seen at all and thus won't be applied.

vonagam commented 1 year ago

AIUI "mixin" is exclusively about reusing code, not with interface/protocol behavior (i.e. there's not is a relationship).

The wording of definition of Mixin on wiki uses words like "Typically" and "not be necessarily applied" so while type relationship is rare, it is still inside the boundary of that definition.

I do like the inclusion of type relationship here.

If a super class includes a new method with the same name but different signature, it will create a new error in the user code.

Yes, inheritance has the same problem. Proper Traits as described on wiki do not have such problem - "the behavior of the code may unexpectedly change if the order in which the mixins are applied is altered, or if new methods are added to the parent classes or mixins.".

So if some "extension trait" is not part of the dependency chain it will not be seen at all and thus won't be applied.

Hm... as I understand, a trait implementation should be included in the file of the trait itself or in the file of a class implementing it, so it should not end up being omitted.

vnen commented 1 year ago

If a super class includes a new method with the same name but different signature, it will create a new error in the user code.

Yes, inheritance has the same problem. Proper Traits as described on wiki do not have such problem - "the behavior of the code may unexpectedly change if the order in which the mixins are applied is altered, or if new methods are added to the parent classes or mixins.".

Well, you completely ignored my following sentence saying that with traits conflicts must be explicitly disambiguated and thus the traits also have the same problem. A new method in a trait might create such conflict and generate an error. This is even true for Rust.

I don't know why Wikipedia says traits don't have this drawback, but since the sentence has no mentioned sources take it with a grain of salt.

Also, typical mixin implementation will make later mixins override methods from previous mixins, like @winston-yallow asked. That's what make order to be important. In this proposal I disallow this, like it is disallowed in traits (without explicit disambiguation), so order does not matter.

All in all, I still think this proposal relates more to traits than to mixins.

winston-yallow commented 1 year ago

I agree. Having used both mixins and traits in other languages, this feels more like a trait system. It sure has it's own quirks/implementation details, but overall it fits the concept of traits better than mixins.

vonagam commented 1 year ago

Hm.... having looked into differences between mixins and traits, I would say that important distinction is that mixins can have members other than methods - all members of mixins do get included/mixed into the class, unlike traits which separate implementations from an entity code. And since mixins include variables it makes it impossible to implement those for classes/types that you do not control.

Some languages have only mixins and so they can call them "traits", but it does not help later if you would want to introduce real ones.

I don't think that making is to return true for mixins makes them traits, it is just convenient way to check relationship but other languages that have mixins or traits has to answer the obvious question "how do I check that I have instance of mixin/trait". Maybe some languages do not answer that - but this is on them.

But for record, I am not demanding name change or anything, just my feedback based on my understanding on difference between those two related concepts.

GregX999 commented 1 year ago
# addon/interactable.gdt
global_name Interactable
func a(): return "A"
func b(): return "B"
func c(): return "C"

# fancy_interactable.gdt
global_name FancyInteractable
uses Interactable
func a(): return "Foo Bar"

# super_interactable.gdt
global_name SuperInteractable
uses Interactable
func a(): return "Super"
func b(): return "Duper"

# node.gd
extends Node
uses SuperInteractable
uses FancyInteractable

In this example, is the proposal that the an error would generated simply by have the two uses statements there (using two traits that both have a method a())? (Unless I missed something, it seems this is that case.)

What about allowing both traits to be "used", but then being required to specify which a() is intended by writing SuperInteractable.a() or FancyInteractable.a()? If trying to use just a(), the editor/compiler could raise an error about the method a() being "Ambiguous" (or something similar).

vnen commented 1 year ago

In this example, is the proposal that the an error would generated simply by have the two uses statements there (using two traits that both have a method a())? (Unless I missed something, it seems this is that case.)

Yes, it would be an error.

What about allowing both traits to be "used", but then being required to specify which a() is intended by writing SuperInteractable.a() or FancyInteractable.a()? If trying to use just a(), the editor/compiler could raise an error about the method a() being "Ambiguous" (or something similar).

A syntax like SuperInteractable.a() would only work inside the script itself, externally it does not have a way to select the object. Unless you mean doing object.SuperInteractable.a(). In this sense, I would prefer the casting way: `(object as SuperInteractable).a().

Essentially, there are two solutions for this (not counting the "just error" version): 1) Disambiguate at use. This is what PHP does: if there are conflicts the class can select which trait it is going to use (per method). It may be a problem if you want functionality of both traits. 2) Disambiguate at call. Which Rust does by requiring to cast to the trait when calling in case of ambiguity.

In general I would prefer to resolve at call time, since it's a more flexible solution. But we cannot forget that GDScript is primarily a dynamically typed language, so this would become a runtime error (which does mean an extra runtime check with untyped contexts). So resolving at use time would allow an error to be shown in editor by forcing to disambiguate at the source. There are pros and cons for both solutions, it is a matter of picking trade-offs.

As a default I decided to disallow this because then a decision could be made in the future when the case appears in actual projects.

me2beats commented 1 year ago

Can two traits have _ready()/_input()/physics_process() ?

script.gd:

uses "trait1.gdt"
uses "trait2.gdt"

trait1.gdt:

func _input():
    print("hello from trait 1")

trait2.gdt:

func _input():
    print("hello from trait 2")

Will it be an error or both messages will be printed

vnen commented 1 year ago

Can two traits have _ready()/_input()/physics_process() ?

It's the same as the discussion before: conflicts will show an error. Unless we have explicit disambiguation in place.

For overrides it would only work if it's disambiguated at use. If it's at call, then the user would have to define their own function that calls both traits if that's the wanted behavior.

vnen commented 1 year ago

For the comments here it seems that some disambiguation system is warranted. I'll give some thought to it and update the proposal.

me2beats commented 1 year ago

For example if we have b.gd that extends a.gd and both them have _ready() then it will be called for both. Same with _input etc. I found this very handy

(This was in Godot3)

bend-n commented 1 year ago

Im pretty sure thats a bug...

Your supposed to do ._ready() in gd3 iirc.

Like what order are they called in?

Xananax commented 1 year ago

Can't traits be (optionally) namespaced?

extends Node

var enemy: Trait = preload("enemy.gdt")
var has_health: Trait = preload("health.gdt")
var player_seeker: Trait = preload("player_seeker.gdt")
use "some_global_trait.gdt"

func _ready():
  enemy._ready()
  has_health._ready()
  player_seeker._ready()

This way, there's no ambiguity in the case of multiple traits with similarly named functions, and you get complete control. Everything is explicit, and you can run commands at will.

For illustration, here's how you could get similar functionality in current GDScript

class_name HasHealth extends Area2D
var target: Node2D
@export health := 100

func _init(initial_target: Node) -> void:
  target = initial_target

func _ready() -> void:
  create_shape_around(target)
  target.add_child(self)

func _on_hit(strength: int) -> void:
  health -= strength
  if health <= 0:
     target.queue_free()

What Traits add to a pure GDScript implementation is:

  1. automatic self forwarding.
  2. automatic check on the type of target.
  3. automatic exports forwarding.
  4. and most importantly: interface-like behavior, which is impossible to reproduce.