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

Add Trait System to GDScript #97657

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SeremTitus
Copy link

@SeremTitus SeremTitus commented Sep 30, 2024


GDScript Trait System

Based on the discussion opened in:

The GDScript trait system allows traits to be declared in separate .gdt files or within a trait SomeTrait block.

Traits facilitate efficient code reuse by enabling classes to share and implement common behaviors, reducing duplication and promoting maintainable design.

Syntax Breakdown

Declaring Traits

  • Traits can be defined globally or within classes.

  • In a .gdt file, declare a global trait using trait_name GlobalTrait at the top. trait used for inner traits.

  • Traits can contain all class members: enums, signals, variables, constants, functions and inner classes.

    Example:

    # SomeTrait.gdt
    trait_name SomeTrait
    
    # Trait types used for typing containers.
    var player: TraitPlayable
    var scene_props: Array[TraitMoveable]
    var collected: Dictionary[String, TraitCollectable]
    
    func _ready():
        pass  # Method with implementation, but can be overridden.
    
    # Method to be implemented by the class using the trait.
    # The return type must be `void`.
    func foo() -> void # Bodyless need to be implemented in class using trait.
    
    # The return type can be overridden, but it's not required to specify one.
    func some_method() # Bodyless need to be implemented in class using trait.
    
    # The return type can be overridden but must be a class that inherits from `Node`.
    func some_other_method() -> Node  # Bodyless need to be implemented in class using trait.
    

Using Traits in Classes

  • Use the uses keyword after the extends block, followed by the path or global name of the trait.

  • Traits can include other traits but do not need to implement their unimplemented functions. The implementation burden falls on the class using the trait.

    Example:

    # SomeClass.gd
    extends Node
    
    uses Shapes, Topology  # Global traits
    uses "res://someOtherTrait.gdt"  # File-based trait
    
    func _ready():
        var my_animals : Array = []
        my_animals.append(FluffyCow.new())
        my_animals.append(FluffyBull.new())
        my_animals.append(Bird.new())
        var count = 1
        for animal in my_animals:
            print("Animal ", count)
            if animal is Shearable:
                animal.shear()
            if animal is Milkable:
                animal.milk()
            count += 1
    
    trait Shearable:
        func shear() -> void:
            print("Shearable ok")
    
    trait Milkable:
        func milk() -> void:
            print("Milkable ok")
    
    class FluffyCow:
        uses Shearable, Milkable
    
    class FluffyBull:
        uses Shearable
    
    class Bird:
        pass

Creating Trait files.

  • In Editor go to FileSystem, left click and select "New Script ...". In the pop up select GDTrait as the preferred Language.
  • Alternatively in script creation pop up instead of selecting GDTrait from 'Language' dropdown menu change 'path' extention to '.gdt' and language will automatic change to GDTrait
    image

How Traits Are Handled

Cases

When a class uses a trait, its handled as follows:

1. Trait and Class Inheritance Compatibility:

The trait's inheritance must be a parent of the class's inheritance (compatible), but not the other way around, else an error occurs. Also note traits are pass down by inheritance, If a class is for instance "SomeTrait" also it here classes will be so.

Example:
    # TraitA.gdt
    trait_name TraitA extends Node

    # ClassA.gd
    extends Control
    uses TraitA  # Allowed since Control inherits from Node

2. Used Traits Cohesion:

When a class uses various traits, some traits' members might shadow other traits members ,hence, an error should occur when on the trait relative on the order it is declared.

3. Enums, Constants, Variables, Signals, Functions and Inner Classes:

These are copied over, or an error occurs if they are shadowed.

4. Extending Named Enums:

Named enums can be redeclared in class and have new enum values.

5. Overriding Variables:

This is allowed if the type is compatible and the value is changed.
Or only the type further specified. Export, Onready, Static state of trait variables are maintained. Setter and getter is maintained else overridden (setters parameters same and the ).

6. Overriding Signal:

This is allowed if parameter count are maintained and the parameter types is compatible by further specified from parent class type.

Example:

    # TraitA.gdt
    trait_name TraitA
    signal someSignal(out: Node)

    # ClassA.gd
    uses TraitA
    signal someSignal(out: Node2D) # Overridden signal

7. Overriding Functions:

Allowed if parameter count are maintained, return types and parameter types are compatible, but the function body can be changed. Static and rpc state of trait functions are maintained.

8. Unimplemented (Bodyless) Functions:

The class must provide an implementation. If a bodyless function remains unimplemented, an error occurs. Static and rpc state of trait functions are maintained.

9. Extending Inner Classes:

Inner classes defined in used trait can be redeclared in class and have new members provide not shadow members declared inner class declared in trait. Allow Member overrides for variables, Signals and function while extending Enum and its' Inner Classes.

Example:

    # Shapes.gdt
    trait_name Shapes
    class triangle: # Declared
        var edges:int = 3
        var face:int = 1
        func print_faces():
            print(face)


    # Draw.gd
    uses Shapes
    class triangle: # Redeclared
        var verticies:int = 3 # Add a new member
        var face:int =  2 # Overriding Variable
        func print_faces(): # Overriding Function
            print(face-1)

Special Trait Features

10. Trait can use other Traits:

A trait is allows to use another trait except it does not alter members of the trait it is using by overriding or extending.
However, cyclic use of traits (TraitA uses TraitB and TraitB uses TraitA) is not permitted and will result in error.

11. Tool Trait:

if one trait containing the @tool keyword is used it converts classes (except inner classes) and traits using it into tool scripts.

12. File-Level Documentation:

Member documentation is copied over from trait else overridden.


System Implementation Progress

  • Implement and verify How Traits Are Handled
  • Debugger Integration
  • Trait typed Assignable (variable, array, dictionary)
  • Trait type as method & signal parameters' type
  • Trait type as method return type
  • Trait type casting (as)
  • Class is Trait type compatibility check (is)
  • Make .gdt files unattachable to objects/nodes
  • Hot reloadable Classes using traits when trait Changes (for Editor and script documentation)
  • Write Tests
  • Write Documentation (user manual)

Bugsquad edit:

@DaloLorn
Copy link

Is there a specific reason you specified a void return type for foo()? Do abstract methods always need a strictly defined return type?

If not, might I recommend a different return type for clarity?

@AdriaandeJongh
Copy link
Contributor

AdriaandeJongh commented Oct 1, 2024

(Edited because I missed a part in the OP description)

Fantastic start on this feature. Thank you!

One comment: please use implements over uses, as per the original proposal.

@DaloLorn
Copy link

DaloLorn commented Oct 1, 2024

I'm not sure your reasoning lines up with your conclusion there, but I can't say I have much of a preference, what with it being a strictly cosmetic affair.

@btarg
Copy link

btarg commented Oct 1, 2024

This system seems very similar to the abstract keyword, which has already been suggested.. what makes this approach better than abstract classes?
I already use a lot of base classes with empty functions that are then overridden in my game currently. Will this improve the workflow comparatively?

@DaloLorn
Copy link

DaloLorn commented Oct 1, 2024

Abstract classes are still beholden to the class hierarchy: No class can inherit from two classes at a time.

There is some value in having both of these, I suppose, but traits are far more powerful.

@dalexeev
Copy link
Member

dalexeev commented Oct 1, 2024

This system seems very similar to the abstract keyword, which has already been suggested.. what makes this approach better than abstract classes?

See:

Also, as DaloLorn said, these are independent features that can coexist together. Traits offer capabilities that classic inheritance cannot provide.

@rrenna
Copy link

rrenna commented Oct 1, 2024

This looks great, will traits be able to constrain typed collections (ie. Array[Punchable], Dictionary[String, Kickable]) ?

@Dynamic-Pistol
Copy link

Amazing that somebody cared to make this,but since the original proposal is shut down,here is some feedback

  1. use impl instead of uses,this makes more sense and also is short too
  2. don't have a seperate file extension for traits,have .gd files be able to support different object types
  3. no trait_name, name it instead type_name
  4. traits should only have functions (abstract and virtual),no other stuff

@RadiantUwU
Copy link
Contributor

RadiantUwU commented Oct 2, 2024

  1. traits should only have functions (abstract and virtual),no other stuff

Considering work has already been made for signals, we should get to keep them too. (unless massive performance issues appear)

@Mickeon
Copy link
Contributor

Mickeon commented Oct 2, 2024

I am a bit concerned on the performance of this in general, but that would be something that can be solved over time. I am really, really ecstatic about this.

I agree. There's no reason to exclude signals from traits if the work has been done.

@SeremTitus SeremTitus force-pushed the GDTraits branch 2 times, most recently from 36d7605 to 4088f53 Compare October 3, 2024 14:57
Copy link
Contributor

@RadiantUwU RadiantUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nitpicks, mostly you should rename uses with impl everywhere.

modules/gdscript/gdscript.cpp Show resolved Hide resolved
modules/gdscript/gdscript_analyzer.cpp Outdated Show resolved Hide resolved
modules/gdscript/gdscript_analyzer.cpp Show resolved Hide resolved
modules/gdscript/gdscript_analyzer.cpp Show resolved Hide resolved
modules/gdscript/gdscript_analyzer.h Show resolved Hide resolved
modules/gdscript/gdscript_parser.h Show resolved Hide resolved
modules/gdscript/gdscript_parser.h Show resolved Hide resolved
modules/gdscript/gdscript_parser.h Show resolved Hide resolved
modules/gdscript/gdscript_tokenizer.cpp Show resolved Hide resolved
modules/gdscript/gdscript_tokenizer.h Show resolved Hide resolved
@AdriaandeJongh
Copy link
Contributor

AdriaandeJongh commented Oct 4, 2024

As various others have suggested to use impl instead of implements, I wanted to make a case against impl. Take this example:

class_name SomeClass
extends BaseClass
impl TraitA
  • impl is inconsistently abbreviated next to the written-out extends and class_name keywords that will almost always be just above or below it.
  • for non-native English speakers, impl is harder to understand at first glance, and un-google-able for translation.
  • If contributors want Godot to be beginner friendly, writing out implements is the more friendly option here, because it requires less knowledge of the programming language and associated jargon.
  • as rightfully mentioned on mastodon in response to me yapping about engine UX, impl could imply "implies", "impels", "implant", "implode".

The 6 characters impl saves over implements is not worth the confusion and inconsistency. Instead, perhaps you would agree with me that this is much more readable and consistent:

class_name SomeClass
extends BaseClass
implements TraitA

@Dynamic-Pistol
Copy link

impl is inconsistently abbreviated next to the written-out extends and class_name keywords that will almost always be just above or below it.

what would you abbreviate extends and class_name to? only answers i can think of is for class_name, which could be changed to use the script name, and extends could just be extend

for non-native English speakers, impl is harder to understand at first glance, and un-google-able for translation.

Pretty sure non-native speakers are able to understand abbreviations, by your logic int and bool should be Integer and Boolean, this doesn't include all the abbreviated types that exist already

If contributors want Godot to be beginner friendly, writing out implements is the more friendly option here, because it requires less knowledge of the programming language and associated jargon.

traits aren't exactly beginner stuff, when someone starts with a language they learn they might learn traits ,but for gamedev you don't learn certain stuff until you get the basics/become a casual programmer , when i was a unity developer, i didn't learn about interfaces (which are extremely similar to traits) until i had advanced enough and realised i need some other solution to inheritance

as rightfully mentioned on mastodon in response to me yapping about engine UX, impl could imply "implies", "impels", "implant", "implode".

this makes no sense? let's take a look at some example code:

class_name Door
extends AnimatableBody3D
impl Interactable

what would "implies" mean in a progammer context?, "impels" isn't even abbreviated correctly, "implant"? seriously?, "implode" would be a function for gameplay

The 6 characters impl saves over implements is not worth the confusion and inconsistency. Instead, perhaps you would agree with me that this is much more readable and consistent

previous points still matter, also rust uses the impl keyword, and i am pretty sure it popularized the concept of traits and yet still 0 complains from it

@OscarCookeAbbott
Copy link

Is using a separate file extension necessary? And if not, would it be better to stick to .gd? From a UX perspective it seems a lot simpler and easier not to.

@eon-s
Copy link
Contributor

eon-s commented Oct 4, 2024

Is using a separate file extension necessary? And if not, would it be better to stick to .gd? From a UX perspective it seems a lot simpler and easier not to.

Extensions can be useful for quick search, filter, etc., without the need to check the content of the file nor adding extra prefix/suffix to file names (so it's better in UX terms), also can help other tools like the filesystem to implement custom icons.

@DaloLorn
Copy link

DaloLorn commented Oct 4, 2024

Put me down as another vote in favor of "implements", for what it's worth. I'm indifferent on "implements" versus "uses", but I'm not nearly so indifferent on "impl" versus "implements": The majority of Adriaan's concerns have not been addressed to my satisfaction.

@JJulioTLG
Copy link

JJulioTLG commented Oct 4, 2024

As various others have suggested to use impl instead of implements, I wanted to make a case against impl. Take this example:

class_name SomeClass
extends BaseClass
impl TraitA
  • impl is inconsistently abbreviated next to the written-out extends and class_name keywords that will almost always be just above or below it.
  • for non-native English speakers, impl is harder to understand at first glance, and un-google-able for translation.
  • If contributors want Godot to be beginner friendly, writing out implements is the more friendly option here, because it requires less knowledge of the programming language and associated jargon.
  • as rightfully mentioned on mastodon in response to me yapping about engine UX, impl could imply "implies", "impels", "implant", "implode".

The 6 characters impl saves over implements is not worth the confusion and inconsistency. Instead, perhaps you would agree with me that this is much more readable and consistent:

class_name SomeClass
extends BaseClass
implements TraitA

I think uses is fine.

@antimundo
Copy link

I agree on "impl" been a very confusing keyword.

@RadiantUwU
Copy link
Contributor

RadiantUwU commented Oct 7, 2024

To resolve the class reference error: Compile godot, then run ./bin/godot<TAB> --doctool, then push the result.

@SeremTitus SeremTitus force-pushed the GDTraits branch 2 times, most recently from f27a5d9 to 8d2b91c Compare October 9, 2024 17:35
@Meorge
Copy link
Contributor

Meorge commented Nov 27, 2024

Do user guides and tutorials fall outside of the scope of "documentation" for the purposes of this PR? It sounds like most, if not all, of the syntax has at least been locked down, so perhaps we could work on producing manual pages that describe how and why to use traits, so they're ready for when traits themselves are merged?

@SeremTitus
Copy link
Author

Radiant pointed out today that, according to the "System Implementation Progress" checklist, tests and documentation have yet to be done. Is this something that other contributors could help with to move the PR along, or is it best left to the original authors of the PR?

@Meorge any help will be good, I have written some tests but intend to include more while the documentation task actually refers to what will appear on the user manual

@dillydadally
Copy link

dillydadally commented Dec 6, 2024

I personally far far far prefer uses over implements:

  • It's easier to write and easier to understand
  • It's actually the obvious and correct usage in the English language. As evidence, multiple people said "uses a trait" on this very page already, and no one said "implements a trait" at all. Even if we change the keyword, people will still say "uses a trait" when talking about it because it's natural English. Shouldn't we strive to use natural English when we can?
  • Meanwhile, "Implements" is not a word used frequently in English and I've never liked it in the programming languages I've used. It's esoteric and verbose and confusing to new programmers.
  • It's usually used for interfaces as well, and this is a trait system, not an interface system, even if some people are looking forward to using it for that.
  • The only reason I can think of to use Impl over Uses is because that's what Rust uses, but lets be honest - while Rust is an awesome language for other reasons, it's kind of accepted that its syntax is ugly. Meanwhile, PHP, a language that originally became popular partially due to its simple syntax, uses the keyword uses for traits and has since before Rust was even invented. Uses arguably has more precedent than impl.

On a personal level I dislike it because for whatever reason I tend to confuse the keywords extends and implements in languages that use both of them and have to really think which to use in which circumstances.

I haven't heard anyone give an actual reason they prefer implements over uses either. Is it just because they're planning on using traits as Interfaces? Or because they're coming from Rust? I'm interested in the reasoning because to me uses is both easier to write and clearer.

@dillydadally
Copy link

dillydadally commented Dec 6, 2024

I'm wondering if conflict resolution between trait functions is still a feature as mentioned in the proposal? I don't see it listed above. Was it forgotten or was there a reason the community decided to remove the feature? Or maybe that's being saved for a later proposal? This is a wonderful start and I really appreciate it for what it is!

I mention it because I felt like even the original proposal wasn't far enough and eventually a syntax for full explicit conflict resolution for variables as well as methods would be ideal. I feel like in an ideal world, Traits could be something shared in an asset marketplace and dropped into projects to add complex functionality. I'd even love it if there was a Trait Node in the future where you could also add child nodes that would be added to the scene like inherited classes. That way you could literally compose an entire scene by dragging and dropping Traits (gdt files) into the scene tree and linking exported trait variables together. All of this vision though is nearly impossible if Traits can't handle conflict resolution. I also see a future where large rewrites are required because two traits have one small conflict. I feel like it's a really important feature to eventually add.

@Meorge
Copy link
Contributor

Meorge commented Dec 6, 2024

To me, "uses" doesn't sound like it would really be correct English. I don't think I've ever heard someone say in English that someone "uses" a trait; they "have" a trait, or they "are" a trait. If there is an enemy that you can shoot, we don't say that "the enemy uses shootable" (perhaps there's a better phrasing I'm missing right now?). It's a bit more correct (but admittedly verbose, as you said) to say "the enemy implements behavior when it is shot". I suppose the most natural English would be to say "the enemy is shootable", which would translate to

class_name SomeEnemy
extends Node2D
is Shootable

which, while even less verbose than uses, also uses a keyword that has a different meaning in other contexts, and also might be ambiguous as to what the requirements are for the trait criteria to be satisfied.

To summarize my thoughts:

  • uses is quick to type, but doesn't fully convey what it's doing in my opinion. It sounds more like it's directly importing functionality (like methods or constants) from another file.
  • implements is definitely longer, but not significantly so, and you only type it once at the top of the file so it's not going to bloat scripts that much. It also is closer to correct English in terms of its functionality and how it reads. "Extends" in the context that GDScript uses it is also a bit esoteric, but is used because it describes the behavior well enough in fewer words than a more "proper" English solution.
  • is would be the shortest and the most correct English-wise, but it might be ambiguous with that keyword being used elsewhere for other purposes.

@dillydadally
Copy link

dillydadally commented Dec 6, 2024

To me, "uses" doesn't sound like it would really be correct English. I don't think I've ever heard someone say in English that someone "uses" a trait

Multiple people used that exact verbiage on this very page. That was what I was trying to point out.

It sounds more like it's directly importing functionality (like methods or constants) from another file.

I feel like that's kind of what you're actually doing. It's not a true "includes", but you're importing methods and constants into a class from another file.

@Meorge Thank you for your thoughts. Could you or anyone else comment on what attracts you to implements? It's obvious it has some support but I'm at a loss as to why it's a popular choice and haven't seen anyone explain it. Is it the precedent set by Rust or its relation to interfaces or some other reason? It just seems like a long abstract word to me that doesn't fit because it's typically used for interfaces in almost every other language and I feel like I'm missing something. Maybe it's my familiarity with both PHP and Delphi, which use the uses keyword.

@tehKaiN
Copy link

tehKaiN commented Dec 6, 2024

IIRC pascal/delphi uses uses for imports from other file so that you can use types/functions/etc from them in this file - this is way different than telling that given object satisfies constraints imposed by a trait.

I don't know if taking PHP as a reference is a good idea since this language is a stitchup of different languages with caveats here and there - and I'm talking from years of professional experience with PHP before I managed to escape webdev. I heard that it's getting cleaned up with recent version, but still I wouldn't refer to it as an example of good programming language design.

The word implements comes indeed from the interface realm and many programming languages, not only rust, refer to the act of an object adhering to it by that word - perhaps not in the syntax itself, but in the documentation.

From delphi docs, since you're refering to this language:

An object interface, or simply interface, defines methods that can be implemented by a class

I know that traits aren't exactly interfaces, but they are closely related - they act like one if a trait contains only declarations of methods and no definitions of them. Interfaces in C# can also contain fn implementations, provided they only refer to public fields, which makes them kinda like traits.

I understand though that the word is a bit nerdy and perhaps could be replaced with something more friendly. is is an interesting candidate for me. The other is supports from Delphi, but for me it doesn't sound too good either, and having one less keyword is better, I think, especially as the is word shows up in conditionals like if my_node is TDamagable which will be used with traits rather frequently.

@dalexeev
Copy link
Member

dalexeev commented Dec 6, 2024

Please note that the pull request is not intended to discuss the design of the feature, only its implementation (we should unlock the proposal).

As for implements vs uses, in my opinion implements is more suitable for interfaces, since they cannot contain an implemented method (each class implementing an interface must fully define all its methods). While traits can contain both abstract and implemented methods. A class can use a non-abstract trait without implementing any of its methods.

@DaloLorn
Copy link

DaloLorn commented Dec 6, 2024

The waters there have been heavily muddied with the concept of default implementations. While some languages still hold that an interface must be strictly abstract, there's a fair few in common use today - e.g. Java/Kotlin, C#, Swift, or Python - where interfaces or interface-like constructs are permitted to implement some of their methods.

@Meorge
Copy link
Contributor

Meorge commented Dec 6, 2024

Regarding the keyword to use, I agree that might be better to discuss on the proposal page.

For the progress that needs to be made for the actual feature to be merged, I'm not sure how much there would be to add to the class reference but I could see a lot of value in preparing the user manual over at godot-docs with sections on traits and how to use them. I'll have to familiarize myself with the trait system before trying to write docs on it, but it's something I'd be happy to do 🙂

@moonheart08
Copy link

moonheart08 commented Dec 6, 2024

The functionality described here is an interface, not traits.

An analogy:
A trait is literally a "trait of the object", you can invent a new word (trait) and have everything that fits it already be it, and use it to describe objects you don't have on your shelf to modify. I could implement a trait for some provided godot object and have it Just Work.

An interface (what you have here) is taking something you own off your shelf and adding a new kind of plug or label to it for other stuff to use.

I think a lot of folks coming from newer languages that have "full" traits (which allow things like implementing a trait for all possible types described by a generic) would be a bit confused seeing this.

Apologies for not putting this on the proposal page, it is currently locked.

@AlyceOsbourne
Copy link

I think arguing over implements vs uses is largely irrelevant, I think getting tied up in semantics is in part why we have taken so long on settling on an implementation. I personally don't care if its uses or implements, I just want interfaces. I am very happy that we will be allowing default implementations, as that allows is to simplify the implementation of many behaviours, allowing for extension where needed. I think "pure" interfaces would be way less useful, especially for the way I think about them.

@Meorge
Copy link
Contributor

Meorge commented Dec 7, 2024

I think arguing over implements vs uses is largely irrelevant, I think getting tied up in semantics is in part why we have taken so long on settling on an implementation. I personally don't care if its uses or implements, I just want interfaces.

While I definitely agree with the sentiment, and I'm really looking forward to having this in GDScript ASAP too, I think we're at the point with this feature now where these are the kinds of conversations we need to have - we can't push them off much further. Once this gets into a release (especially a non-dev one), it'll be really hard to change the terminology, so we want to make sure we do it right now.


Since the proposal has not been opened, one argument against uses that occurred to me is that it may be better suited to using/importing other namespaces, if/when namespaces come to GDScript. While importing namespaces could be done with a different keyword like imports, it might become a bit confusing which does what:

class_name SignPost
extends Node2D
uses Attackable, Interactable  # these are traits/interfaces that the class takes advantage of, NOT namespaces
imports DialogueTools  # these are namespaces that the class imports

I suppose from context here you could infer that "Attackable" isn't a namespace, and "DialogueTools" isn't a set of things for this class to use, but the semantic meaning of the two lines still feels fuzzier than it should be. C#, a language that is often discussed alongside GDScript, uses using for namespaces and similar things as well, so it may be confusing to people switching between the two to have the meaning of that word change.

My current preference is implements or is. The latter is best IMO, especially if we encourage users to name their traits/interfaces as adjectives like so:

class_name SignPost
extends Node2D
is Attackable, Interactable

The big downside I see here is that it's already a keyword used in other contexts, but those contexts are pretty substantially different (part of an expression, has a left operand, etc) and semantically, they mean very similar things (membership/adherence to a class/trait/interface).

@Meorge
Copy link
Contributor

Meorge commented Dec 9, 2024

I've started a branch of godot-docs for adding trait support/discussion to the relevant spots in the user manual 🙂

godotengine/godot-docs#10393

I'm going off of what's currently in the GDTraits branch, which means for example it's currently using the uses keyword. If that ends up getting changed, we'll need to make sure to go through and update the docs to match.

Aside from the discussion on the keyword for trait use, we'll need to decide on how traits fit into the GDScript style guide. I'm not sure if that would be a better discussion for this thread, or the PR on godot-docs.

@Meorge
Copy link
Contributor

Meorge commented Dec 10, 2024

While working on the documentation, I found behavior with static variables in traits that seems like it might not make sense?

With this trait and class:

trait SomeTrait:
    static var some_var: int = 0

class MyClass:
    uses SomeTrait

You cannot do

print(MyClass.some_var)

because some_var is static to the trait itself, so you would have to do

print(SomeTrait.some_var)

This is tricky IMO, because overall the behavior is consistent with how classes work. But I'm struggling to think of a use case where a static member of a trait itself would be necessary, and I feel like there could be plenty of use cases for classes that use particular traits inheriting their static members the same way their instances inherit non-static members.

@Meorge
Copy link
Contributor

Meorge commented Dec 11, 2024

Another thing: we should probably have a template specifically for traits that doesn't include _ready and _process methods. Perhaps, to keep it from being blank, we could provide a default trait_name line and maybe some commented-out descriptions of what traits can do.

@SeremTitus
Copy link
Author

While working on the documentation, I found behavior with static variables in traits that seems like it might not make sense?

Nice catch, that was a bug. It is now fix and have write a test for it.
Here example how it should work

trait SomeTrait:
	static var some_static_var = 0
	static var other_static_var = 0
	static var third_static_var = 0
	static func some_static_func():
		print("some static func")
	static func other_static_func():
		print("other static func")
	static func third_static_func():
		print("third static func")

class SomeClass:
	uses SomeTrait
	# Overridden static variable
	var other_static_var = 1 # using 'static' keyword is optional
	static var third_static_var = 1
	# Overridden static function
	func other_static_func(): # using 'static' keyword is optional
		print("overridden other static func")
	static func third_static_func():
		print("overridden third static func")

func _ready():
	print(SomeClass.some_static_var)
	print(SomeClass.other_static_var)
	print(SomeClass.third_static_var)
	SomeClass.some_static_func()
	SomeClass.other_static_func()
	SomeClass.third_static_func()
	print("ok")

If static function/variable is declared in a trait, when redeclared/overridden in class should using 'static' keyword is optional ? (currently it is)

because some_var is static to the trait itself, so you would have to do

print(SomeTrait.some_var)

This is not correct traits are not object and should not be instanced or called, I will look for way to better communicate this to prevent misuse.

@Meorge
Copy link
Contributor

Meorge commented Dec 11, 2024

If static function/variable is declared in a trait, when redeclared/overridden in class should using 'static' keyword is optional ? (currently it is)

Personally, I strongly feel that the static keyword should not be optional. If the user wants to redefine its value for a class that uses a trait, then I think one of these should work:

class MyClass:
    uses MyTrait
    static var some_static_var  = 2  # inherited from MyTrait
    # OR
    some_static_var = 2  # also inherited from MyTrait

The static var count = ... makes it clear to whoever's reading this class file that count is a static variable, whereas if it was simply var count = ... they would naturally assume it's an instance variable and be confused when they're not allowed to do instance.count.

Simply having count = ... works with the idea that we've already declared the variable itself in the trait, and now we're just modifying the value here, like how var local_value = ... initializes the variable and local_value = ... changes the value. I could see others being opposed to this syntax being up at the class member definition level, though, and can understand their reasoning, so overall I think just requiring the static keyword makes the most sense.

@Meorge
Copy link
Contributor

Meorge commented Dec 12, 2024

I pulled the recent changes to verify the static variable behavior, and while it looks like it's closer to what is expected (at least to me), I think I may have found an inconsistency with it. I used this code:

extends Node2D

trait HasStatic:
    static var static_var = 3

class UsingClass:
    uses HasStatic
    
class DifferentUsingClass:
    uses HasStatic

func _ready():
    UsingClass.static_var = 2
    print("HasStatic's value: ", HasStatic.static_var)
    print("UsingClass's value: ", UsingClass.static_var)
    print("DifferentUsingClass's value: ", DifferentUsingClass.static_var)

This prints out:

HasStatic's value: <null>
UsingClass's value: 2
DifferentUsingClass's value: <null>

I would expect it to print out:

HasStatic's value: 3
UsingClass's value: 2
DifferentUsingClass's value: 3

Does this sound like unexpected behavior, or am I missing something with how static variables are expected to work in traits?

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

Successfully merging this pull request may close these issues.

Add a Trait system for GDScript