godot
godot copied to clipboard
godotengine
GDScript: Add Trait System
- Supersedes: https://github.com/godotengine/godot/pull/97657.
- Closes https://github.com/godotengine/godot-proposals/issues/6416.
Focusing on:
- Traits are declared as members of a class.
- Trait hold all members that a class can except for inner classes.
- Method Overriding: Allowing classes to override methods defined in traits.
- Enforcing the implementation of bodyless methods in classes that are from traits.
- The 'uses' keyword is maintained as it short and clear, however, find a poll below.
# Keeps code DRY (Don't Repeat Yourself)
class_name Player extends Node
uses Damageable, Movable
func _ready():
died.connect(on_death)
print("Player spawned with health:", health, "and speed:", speed)
# Using Movable trait
move(Vector2.RIGHT)
# Using Damageable trait
take_damage(50)
print("Is player dead?", is_dead())
take_damage(60) # Triggers death
func on_death():
print("Player has died.")
trait Damageable:
signal died
const MAX_HEALTH := 100
var health: int = MAX_HEALTH
func take_damage(amount: int) -> void:
health -= amount
print("Took damage:", amount, "Health now:", health)
if health <= 0:
died.emit()
func is_dead() -> bool:
return health <= 0
func on_death() # Unimplemented method – must be overridden
trait Movable:
@export var speed: float = 100.0
func move(direction: Vector2) -> void:
print("Moving in direction:", direction.normalized() * speed)
We're considering naming for trait inclusion syntax. Please react to this comment with your preference: 🎉 — implements 🚀 — uses (current)
note: The GDScript team will have the final say
Please note that PRs are generally not the right place to discuss feature design, we should only discuss implementation here. This is usually not a problem for small features and enhancements, but a significant problem for large features like this.
Unfortunately, GitHub Issues and PRs do not support tree-like or even two-level comments. This makes it difficult to follow long discussions in case of large and popular feature requests. For this reason, we locked the original proposal, but users started discussing and suggesting ideas in the previous PR instead. This may have been one of the factors why the previous PR grew to a practically unreviewable state and included features that should have been moved to the next iterations.
We would not like to repeat this situation with the new PR, so I suggest to move the discussion of the feature design to a discussion in the godot-proposals repository. GitHub Discussions supports two-level comments, so we can allocate a separate branch for each topic. Including for bikeshedding on keywords.
I agree discussion should move to the godot-proposals repository. But to answer the question
In other words, there any potential for the keyword to conflict or clash with interfaces in the future?
No. Interfaces define a contract or set of method signatures that a class must implement, without providing any implementation. Traits, in our case, are like interfaces with inclusion of reusable method implementations and other members (such as variables, constants, and signals) that can be included in classes. Classes can use multiple traits.
Enforcing the implementation of bodyless methods in classes that are from traits.
Not sure if default methods are allowed in the future. Hope to see it after this get merged
From a quick look at some of the PR files (specifically the keywords), it looks like traits can only be declared as class members. How does referencing traits work from another class? For example, if I have a file my_traits.gd:
# my_traits.gd
class_name MyTraits
trait Damageable:
func take_damage():
print("Ouch, that hurt!")
trait Movable:
func move():
print("You're moving me")
And then I have another file player.gd:
# player.gd
class_name Player
extends CharacterBody2D
# Which one do we need to use?
uses Damageable, Movable # Easier to read.
uses MyTraits.Damageable, MyTraits.Movable # Consistent with inner classes and such.
func _ready():
...
In other words, do we need to reference the class_name of traits' parent classes in order to use them? How does this work for unnamed classes?
Personally, I think it'd be nice to bring back trait_name from the previous PR. If a .gd file uses a trait_name keyword, then it could be understood that the file is defining a trait and not a class.
(I'm posting this here and not in the discussion thread since it seems to me like a detail of this implementation, but I recognize the line is blurry. If you think it'd fit better there, let me know and I can certainly move over there to continue.)
Personally, I think it'd be nice to bring back
trait_namefrom the previous PR. If a.gdfile uses atrait_namekeyword, then it could be understood that the file is defining a trait and not a class.
In this pr , we will not. It may come back in the future if this pr is successful . Also .gd file by design is a class just named(global) or unnamed so maybe trait_name in a .gdt file.
For the case this pattern is appropriate :
uses MyTraits.Damageable, MyTraits.Movable
Great, thanks for the clarification! I'm working on updating my docs PR to match how this version of traits works, so that hopefully the feature and its documentation can launch simultaneously 😄 It is still posted at https://github.com/godotengine/godot-docs/pull/10393
Since this alters the token enum value, when you rebase next, please bump the TOKENIZER_VERSION number in gdscript_tokenizer_buffer.h here: https://github.com/godotengine/godot/blob/db57f282fa3994fc727638dc5c17679f30cb06f4/modules/gdscript/gdscript_tokenizer_buffer.h#L42
Since this alters the token enum value, when you rebase next, please bump the
TOKENIZER_VERSIONnumber in gdscript_tokenizer_buffer.h here:
This is a bit premature advice. As long as there is a small chance that this might be included in 4.5, there is no need to increase the tokenizer version (dev versions are not considered releases). If this PR will be included in 4.6 and later, then increasing the version makes sense. But it is not necessary, we can check before each Godot release if there were changes in the tokenizer, and if so, increase the version by 1.
My uttermost personal thanks for making the leanest PR possible for this feature.
Should we encourage people to test this PR to ensure it all works as intended?
Should we encourage people to test this PR to ensure it all works as intended?
100% go for it, at this stage the only changes I may be making are bug fixes or code optimizations
Great job!
I tried to migrate my project which heavily depends on traits from older PR, found some problems:
- apparently traits can't use other traits when stored in separate files and with no class_name defined? This doesn't necessarily might be a bug, just a consequence of how GDScript is scoping things.
t_parent.gd:
trait TParent:
func print_parent() -> void:
print("print_parent")
t_child.gd:
trait TChild:
uses TParent # <--- Parse Error: Could not find trait "TParent".
func print_child() -> void:
print("print_child")
When putting traits in same file or with each having separate class_name and referenced with uses TParentClass.TParentTrait they seem to work without problems. I can live with that for now since I guess .gdt coming in following PR could fix that.
- Mixed "extends" in subtraits
That's quite problematic for my project. I have:
class_name TTargetableTeamMember
trait Trait:
uses TTeamMember.Trait, TMapObject.Trait
TTeamMember.Trait extends Node (could be a minimap icon in 2D, could be an unit in 3D), TMapObject.Trait extends Node3D since game playfield map is 3D. I expect the resulting trait to extend the narrower one, hence Node3D and that's how the previous PR worked. Still, I get:
Parse Error: "TTeamMember.Trait" trait extending "Node" does not match class inheritance.
Parse Error: "TMapObject.Trait" trait extending "Node3D" does not match class inheritance.
I understand that those limitations might be coming from limited scope of this new PR, so no worries if those things will come in later. The 2nd thing blocks my further tests because the codebase doesn't parse correctly.
- apparently traits can't use other traits when stored in separate files and with no class_name defined? This doesn't necessarily might be a bug, just a consequence of how GDScript is scoping things.
There no global traits in this pr. To get to a trait go through global class if it is in another file
2. Mixed "extends" in subtraits
The errors are correct. When a trait extends it can use properties and methods from the class it extends. So if the Trait is used it expects also that class to have the same properties and methods. In your case, Fix by making your class extend Node3D @tehKaiN here is an example:
trait Movable extends Node2D: # this trait can not be used in class that is not a descendant of Node2D
func move_right():
position.x += 10 # position is from Node2D
class Player extends Sprite2D: # here extends can also be Node2D. It will work
uses Movable # usable because Sprite2D is a descendant of Node2D
func _ready():
move_right() # Call method from the trait
ok, my problem was that the TTargetableTeamMember didn't have extends at all - previous PR inferred it from subtraits, but I'm fine with changed behaviour, albeit it might be a bit tedious in refactoring.
Got another ones for you ;)
- Can't cast from trait to custom node class
class_name MultiTest
extends PhysicsBody3D
uses Trait1
trait Trait1:
extends PhysicsBody3D
func trait1_method() -> void:
print("trait1")
func do_something_with(something: MultiTest.Trait1) -> void:
#var a := something as MultiTest # Doesn't work
var a := something as Node as MultiTest # Works but awkward
Same with is operator, e.g. ((team_member as Node) is TurretTower
asreturns non-null even if object doesn't use given trait
Create empty node2d scene, add script to that node:
extends Node2D
func _ready() -> void:
var a := self as TNestedObject.Trait
if a:
print("Wrong!")
It will print the messsage even if it shouldn't.
Also, sometimes I get ERROR: modules\gdscript\language_server\gdscript_extend_parser.cpp:710 - Index p_position.character = 40 is out of bounds (line.size() = 37).. That's bad, I assume. I'll try to find repro for it.
Trait types, type checking 'is', type casting 'as' is not part of this pr ... I will work on drafts prs to add them back as we wait on this pr to be reviewed and merged.
Sure thing, I understand that you've limited scope of PR to increase chance of merging it. However, point 4 seems to be quite dangerous, so perhaps an error could be added when one tries to use as/is instead of returning false positives?
@SeremTitus Do you mean descendants instead of ancestors? I'm a bit confused 😅 Thanks for the great work though!
trait Movable extends Node2D: # this trait can not be used in class that is not a ancestors of Node2D func move_right(): position.x += 10 # position is from Node2D class Player extends Sprite2D: uses Movable # usable because Sprite2D is a ancestors of Node2D func _ready(): move_right() # Call method from the trait
@SeremTitus Do you mean
descendantsinstead ofancestors? I'm a bit confused 😅 Thanks for the great work though!
You're absolutely right — the terminology in the comment is mixed up! Let's clear it up in a edit. Also It does not have to be descendants only it can be of the same class
- apparently traits can't use other traits when stored in separate files and with no class_name defined? This doesn't necessarily might be a bug, just a consequence of how GDScript is scoping things.
There no global traits in this pr. To get to a trait go through global class if it is in another file
- Mixed "extends" in subtraits
The errors are correct. When a trait extends it can use properties and methods from the class it extends. So if the Trait is used it expects also that class to have the same properties and methods. In your case, Fix by making your class extend Node3D @tehKaiN here is an example:
trait Movable extends Node2D: # this trait can not be used in class that is not a descendant of Node2D func move_right(): position.x += 10 # position is from Node2D class Player extends Sprite2D: # here extends can also be Node2D. It will work uses Movable # usable because Sprite2D is a descendant of Node2D func _ready(): move_right() # Call method from the trait
Small suggestion, use another keyword instead of extends. Extends might cause confusion implying Movable is a node, which is not the case (unless I understood it wrong)
trait Movable requires Node2D
trait Movable implies Node2D
trait Movable implements Node2D #Java way, although technically it would be Node2D that "implements Movable and not the other way around, but I couldn't think of a better way
trait Movable concerns Node2D
et caetera
How does this implementation handle collisions between traits, and between traits and the base class? Is any collision an immediate parse error?
How does this implementation handle collisions between traits, and between traits and the base class? Is any collision an immediate parse error?
@Shadows-of-Fire, for collisions errors are generated
Should trait inheritance work with custom classes?
Trait inheritance works with built-in classes
(class Player extends Sprite2D uses MyTraits.Interactable
where
trait extends Node2D),
but fails when custom classes are in the chain
(class Player extends BaseCharacter uses MyTraits.Interactable
where
BaseCharacter extends CharacterBody2D).
Is this expected behavior, or should the inheritance validator resolve custom class chains like BaseCharacter -> CharacterBody2D -> Node2D -> Node?
Most game projects use custom base classes, so this affects real-world usage. Example: class_name BaseCharacter extends CharacterBody2D then class NPC extends BaseCharacter uses MyTraits.Interactable (trait extends Node) gives "does not match class inheritance" error.
what happens when trait Trait1 extends Trait2? I don't see this mentioned anywhere/there being any tests for it.
See https://github.com/godotengine/godot/pull/107227#issuecomment-2970419289 - a trait that uses another trait should work fine, I guess that extending trait is invalid in this context.
How does the traits system interact with GDExtension?
Also, now that 4.5 has released it would be awesome to get this into 4.6 early for testing coverage :)
Is this expected behavior, or should the inheritance validator resolve custom class chains like BaseCharacter -> CharacterBody2D -> Node2D -> Node?
It's all good now , @AndreiSchapov thanks for you contribution
How does the traits system interact with
GDExtension?
No it doesn't, it's a GDScript only feature
@SeremTitus can you describe your collision handling approach for this implementation? I would like to drill down the details on what it currently handles and how it handles it. I'll try to lay out the cases but not yet imply what the behavior I expect is.
The "cases" of collisions are, given a class C, and traits T1 and T2:
- A collision between a variable defined by
CandT1. - A collision between a function defined by
CandT1. C implements T1, T2and: a. A collision between a variable defined by bothT1andT2. b. A collision between a function defined by bothT1andT2.
I would like to know what your implementation currently accounts for, and what it considers "resolvable" (i.e. allowable collisions, where things can merge happily) and what it considers "unresolvable" (i.e. an error state).
Also, I would like to reiterate @eyalzus12 's question:
what happens when trait Trait1 extends Trait2? I don't see this mentioned anywhere/there being any tests for it.