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 a GDScript Trait system. #23101

Closed
willnationsdev opened this issue Oct 17, 2018 · 93 comments
Closed

Add a GDScript Trait system. #23101

willnationsdev opened this issue Oct 17, 2018 · 93 comments

Comments

@willnationsdev
Copy link
Contributor

willnationsdev commented Oct 17, 2018

(Edit:
To minimize further XY problem issues:
The problem addressed here is that Godot's Node-Scene system / scripting languages don't yet support creating re-usable, grouped implementations that are 1) specific to the root node's features and 2) that can be swapped around and/or combined. Scripts with static methods or sub-nodes with scripts could be used for the latter bit, and for many cases this works. However, Godot generally prefers you to keep the logic for your scene's overall behavior stored in the root node while it uses data computed by the child nodes or delegates significantly-deviated sub-tasks to them, e.g. a KinematicBody2D doesn't manage animation so it delegates that to an AnimationPlayer.

Having a thinner root node that uses "component" child nodes to drive its behavior is a weak system in comparison. Having a largely empty root node that just delegates all of its behavior to child nodes runs counter to this paradigm. The child nodes become behavioral extensions of the root node rather than self-sufficent objects that accomplish a task in their own right. It's very clunky, and the design could be simplified/improved by allowing us to consolidate all of the logic in the root node, but also allow us to break up the logic into different, composable chunks.

I suppose the topic of this Issue is more about how to deal with the above problem than it is specifically about GDScript, yet I believe GDScript Traits would be the simplest and most straightforward approach to solving the issue.
)

For the uninformed, traits are essentially a way of blending two classes together into one (pretty much a copy/paste mechanism), only, rather than literally copy/pasting the text of the files, all you are doing is using a keyword statement to link the two files. (Edit: the trick is that while a script can only inherit one class, it can include multiple traits)

I'm imagining something where any GDScript file can be used as a trait for another GDScript file, so long as the traited type extends a class inherited by the merged-into script, i.e. a Sprite-extending GDScript can't use a Resource GDScript as a trait, but it can use a Node2D GDScript. I would imagine a syntax similar to this:

# move_right_trait.gd
extends Node2D
class_name MoveRightTrait # not necessary, but just for clarity
func move_right():
    position.x += 1

# my_sprite.gd
extends Sprite
is MoveRightTrait # maybe add a 'use' or 'trait' keyword for this instead?
is "res://move_right_trait.gd" # alternative if class_name isn't used
func _physics_process():
    move_right() # MoveRightTrait's content has been merged into this script
    if MoveRightTrait in self:
        print("I have a MoveRightTrait")

I can see two ways of doing this:

  1. Pre-parse the script via RegEx for "^trait <filepathOrIdentifier>" instances, load the referred to script, get its source code, strip out the header info (tool, extends, class_name), insert the text, assign the new source code, and THEN parse the script (this all happens during the script's reload process). We'd have to not support trait nesting or continuously re-examine the generated source code after every iteration to see if more trait insertions have been made.
  2. Parse the script normally, but teach the parser to recognize the keyword, load the referred to script, parse THAT script, and then append its ClassNode's content to the generated ClassNode of the current script (effectively taking the parsed results of one script and adding it to the parsed results of the other script). This would automatically support nesting of traited types.

On the other hand, people might want the trait GDScript to have a name but might NOT want that GDScript's class_name to show up in the CreateDialog (cause it's not meant to be created on its own). In this case, it may actually NOT be a good idea to make any script support it; only those that are specially marked (perhaps by writing 'trait' at the top of the file?). Anyway, stuff to think about.

Thoughts?

Edit: After some pondering, I believe option 2 would be much better since 1) we'd know WHICH script a script segment came from (for better error reporting) and 2) we'd be able to identify errors as they happen since the included scripts must be parsed in sequence, rather than just parsing everything at the end. This would cut down on the processing time it adds to the parsing process.

@MrJustreborn
Copy link

What's the benefit instead of: extends "res://move_right_trait.gd"

@willnationsdev
Copy link
Contributor Author

@MrJustreborn Because you can have multiple traits in a class, but you can only inherit one script.

@aaronfranke
Copy link
Member

If I understand correctly, this is basically what C# calls "interfaces", but with non-abstract methods? It might be better to call the feature interfaces instead of traits to be familiar to programmers.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 18, 2018

@aaronfranke Traits, basically the same thing as Mixins, have a completely different use-case from interfaces precisely because they include implementations of the methods. If an interface gave a default implementation, then it wouldn't really be an interface anymore.

Traits/Mixins are present in PHP, Ruby, D, Rust, Haxe, Scala, and many other languages (as detailed in the linked Wikis), so they should already be widely familiar with people who have a broad repertoire of programming language familiarity.

If we were to implement interfaces (which I'm not opposed to either, especially with optional static typing coming), it would effectively just be a way of specifying function signatures and then requiring that the relevant GDScript scripts implement those function signatures, with traits included (if those existed by that point).

@ghost
Copy link

ghost commented Oct 18, 2018

Maybe a keyword like includes?

extends Node2D
includes TraitClass

Though other names like trait, mixin, has, etc. are surely fine too.

I do like the idea too of having some option to exclude class_name type from the add menu. It can get very cluttered with small types that don't function on their own as nodes.

It may even be just a feature topic of its own.

@LikeLakers2
Copy link
Contributor

(Accidentally deleted my comment, woops! Also, obligatory "why don't you just allow multiple scripts, unity does it")

How will this work in VisualScript, if at all?

Also, could it be beneficial to include an inspector interface for traits, if traits were implemented? I imagine some use cases for traits may include uses cases where there are only traits and no script (at least, no script besides one that includes the trait files). Though, upon thinking about it more, I wonder if the effort put into making such an interface might even be worth it, compared to just making a script that includes the trait files.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 18, 2018

@LikeLakers2

How will this work in VisualScript, if at all?

If it is done the way I suggested, it wouldn't happen for VisualScript at all. GDScript only. Any trait system implemented for VisualScript would be designed completely differently because VisualScript isn't a parsed language. Doesn't preclude the possibility at all though (would just need to be implemented differently). Plus, maybe we should consider getting VisualScript inheritance-support first? lol

Also, could it be beneficial to include an inspector interface for traits, if traits were implemented?

There wouldn't be much of a point. The traits simply impart details onto the GDScript, handing it the properties, constants, signals, and methods defined by the trait.

I imagine some use cases for traits may include uses cases where there are only traits and no script (at least, no script besides one that includes the trait files).

Traits, as they are represented in other languages, are never usable in isolation, but must be included in another script in order to be usable.

I wonder if the effort put into making such an interface might even be worth it

Creating an Inspector interface in some way wouldn't really make much sense for GDScript alone. Adding or removing a trait would involve directly editing the source code of the Script resource's source_code property, i.e. it isn't a property on the Script itself. Therefore, either...

  1. the editor would have to be taught how to specifically handle the proper editing of the source code for GDScript files to do this (error-prone), or...
  2. all Scripts would have to support traits so that the GDScriptLanguage can provide its own internal process for adding and removing traits (but not all languages DO support traits, so the property wouldn't be meaningful in all cases).

@groud
Copy link
Member

groud commented Oct 18, 2018

What is the need for such feature? Is there anything this allows that you cannot do now? Or does it make some tasks significantly faster to deal with?

I'd rather keep GDscript a simple language than adding complex features almost never used.

@willnationsdev
Copy link
Contributor Author

It solves the Child-Nodes-As-Script-Dependencies problem that this guy had an issue with, but doesn't come with any of the same kind of baggage that MultiScript had because it is constrained to a single language. The GDScript module can isolate logic on how the traits relate to each other and the main script whereas resolving the differences between different languages would be a lot more complicated.

@Zireael07
Copy link
Contributor

With the absence of multiple imports/multiple inheritance, child-nodes-as-script-dependencies are the only way to avoid repeating the code A LOT, and this would definitely solve the problem in a nice way.

@willnationsdev
Copy link
Contributor Author

@groud @Zireael07 I mean, the more radical, cross-language approach would be to 1) completely redesign Object to use a ScriptStack to coalesce stacked scripts into a single script representation, 2) re-introduce MultiScript and create editor support that automatically converts the addition of scripts into multiscripts (or just outright making all scripts multiscript for simplicity's sake, in which case MultiScript's implementation would essentially be our ScriptStack), or 3) implement a sort of cross-language trait system for the Object type that can merge in Reference-extending scripts as traits, incorporating their content just like a typical script. All of those options are way more invasive to the engine though. This keeps everything simpler.

@fian46
Copy link

fian46 commented Oct 18, 2018

i don't think trait is necessary. what we need the most is fast engine release cycle. i mean we need to make the engine more flexible to add new feature as simple just add new dll or so files and the engine auto integrating with it self, like plugins style in most IDE. for example i really desperate need for websocket to work, i dont need to wait it until 3.1 to release. 3.1 too broken right now with so many bugs. it will be great if we have this feature. new class can auto inject to GDScript from random .dll or .so in some path. i don't know how much effort this to do in c++ but i hope this is not too hard 😁

@willnationsdev
Copy link
Contributor Author

@fian46 Well, if someone had implemented websockets as a downloadable GDNative plugin, then yeah, what you described would be the workflow. Instead, they opted to make it an integrated feature available in the vanilla engine. There's nothing stopping people from creating features that way, so your point is really unrelated to the topic of this Issue.

@fian46
Copy link

fian46 commented Oct 18, 2018

oops i dont know GDNative is exist 😂😂😂. Trait is awesome but is it easier to make fake trait class and instance it and later call the function like basic script ?

@DriNeo
Copy link

DriNeo commented Oct 18, 2018

If a Godot script is an unnamed class, why not instancing "move_right_trait.gd" in "my_sprite.gd" ?
Sorry for my ignorance if I don't understand the issue.

@Eoin-ONeill-Yokai
Copy link
Contributor

I understand the use of traits in more strongly typed languages such as Rust or (interfaces in) C++, but in a ducked typed language isn't it a bit unnecessary? Simply implementing the same functions should allow you to achieve a uniform interface among your types. I guess I'm a bit unsure what the exact problem is with the way GDScript handles interfaces or how a trait system would even really help.

Couldn't you also use preload("Some-other-behavior.gd") and store the results in a variable to achieve basically the same effect?

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 18, 2018

@fian46 @DriNeo Well, yes and no. Loading scripts and using script classes take care of that already, but the problem extends beyond that.

@THEYOKAI

Implementing the same functions should allow you to achieve a uniform interface among your types

The problem is not achieving a uniform interface which you're right, duck-typing solves just fine, but rather organizing (combining/swapping) groups of related implementations efficiently.


In 3.1, with script classes, you can define static functions on a Reference script (or any type really) and then use that script as a namespace to globally access those functions in GDScript.

extends Reference
class_name Game
static func print_text(p_text):
    print(p_text)
# can even add inner classes for sub-namespaces

extends Node
func _ready():
    Game.print_text("Hello World!")

However, when it comes to non-static content, especially stuff that uses node-specific functions, it's troublesome to divide out one's logic.

For example, what if I have a KinematicBody2D and I want to have a "Jump" behavior and a "Run" behavior? Each of these behaviors would need access to the input handling and move_and_slide features of the KinematicBody2D. Ideally, I would be able to swap out each behavior's implementation independently and keep all of the code for each behavior in separate scripts.

Currently, all the workflows I'm familiar with for this simply aren't optimal.

  1. If you keep all of the implementations in the same script and just swap out which functions are being used...
    1. changing "behaviors" may involve swapping out several functions as a set, so you can't group the implementation changes effectively.
    2. All of the functions for every behavior (X * Y) are sitting in your single script, so it can get bloated very quickly.
  2. You can just replace the entire script, but then that means you have to create a new script for every combination of behaviors plus any logic that uses those behaviors.
  3. If you use child nodes as script dependencies, then that means you'd have these weird "component" Node2D nodes that grab their parent and call the move_and_slide method FOR it which is kind of unnatural, relatively speaking.
    • You have to make an assumption that your parent is going to implement that method or do logic to check that it has the method. And if you do a check, then you can either fail silently and potentially have a silent bug in your game or you can turn it into a tool script unnecessarily just so you can set a configuration warning on the node to visually tell you in the editor that there's a problem.
    • You also don't get proper code completion for the intended operations of the nodes since they derive from Node2D and the whole point is to drive the behavior of a KinematicBody2D parent.

Now, I'll admit that option 3 is the most effective workflow currently, and that its problems can largely be resolved by using the handy-dandy static typing for GDScript in 3.1. However, there is a more fundamental issue at play.

Godot's node-scene system generally has the form of users creating nodes or scenes that do a particular job, in their own enclosed system. You might instance those nodes/scenes within another scene and have them compute data that is then used by the parent scene (such is the case with the Area2D and CollisionShape2D relationship).

However, the vanilla engine's usage and general best practices recommendation is to keep the behavior of your scene locked to the root node and/or its script. Hardly ever do you have "behavior component" nodes that actually tell the root what to do (and when it is there, it feels very clunky). The AnimationPlayer/Tween nodes are the only arguable exceptions I can think of, but even their operations are directed by the root (it's effectively delegating the control to them temporarily). (Edit: Even in this case, animating and tweening aren't the job of the KinematicBody2D, so it makes sense that those tasks would be delegated. Movement, however, like running and jumping is its responsibility) It's simpler and more natural to allow a trait implementation to organize the code as it keeps the relationships between the nodes strictly data-up/behavior-down and keeps code more isolated into its own script files.

@OvermindDL1
Copy link

Eh, marking yourself as 'implementing an interface/trait' also should fulfill an * is * test though, which is convenient to test functionality of something.

@willnationsdev
Copy link
Contributor Author

@OvermindDL1 I mean, I gave an example of doing a test like that, but I used in instead since I wanted to distinguish between inheritance and trait-usage.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 18, 2018

I guess I kinda walked into an XY problem here, my bad. I had just come from 2 other issues (#23052, #15996) that addressed this topic in one way or another and figured I'd submit a proposal, but I didn't really give all the context.

@QbieShay
Copy link
Contributor

@groud this solution will solve one of the problems raised against #19486.

@willnationsdev great idea, I am looking forward to it!

@AfterRebelion
Copy link

From my limited understanting, the thing that this Trait system wants to accomplish is enable something similar to the workflow shown in this video: https://www.youtube.com/watch?v=raQ3iHhE_Kk
(Take into account, i'm talking about the workflow shown, not the feature used)

In the video, it is compared with other kinds of workflows, with their advantages and disadvantages.

At least to my knowledge, this kind of workflow is impossible in GDScript right now, because of how inheritance works.

@willnationsdev
Copy link
Contributor Author

@AfterRebelion The first few minutes of that video where he isolates the modularity, editability, and debugability of the codebase (and the related details of those attributes) is indeed the pursuit of having this feature.

At least to my knowledge, this kind of workflow is impossible in GDScript right now, because of how inheritance works.

This bit isn't quite true, because Godot actually does this very well in regards to node hierarchies and designing scenes. Scenes are modular by nature, properties can be exported (and even animated) directly from the editor without ever dealing with code, and everything can be tested/debugged in isolation because scenes can executed independently.

The difficulty is when the logic that would typically be outsourced to a child node must be executed by the root node because the logic relies on the root node's inherited features. In these cases, the only way to use composition is to have the children start telling the parent what to do rather than having them mind their own business while the parent uses them.

This isn't a problem in Unity because GameObject doesn't have any real inheritance that users can take advantage of. In Unreal it might be a bit(?) of a problem since they have similar node/component-based internal hierarchies for Actors.

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Oct 19, 2018

Okay, let's play Devil's Advocate here for a bit (@MysteryGM, you might get a kick out of this). Spent some time thinking about how I might write such a system in Unreal and that's giving me a new perspective on it. Sorry for the folks who were thinking this would be a good idea / were excited about it:

Introducing a trait system adds a layer of complexity to GDScript as a language that may make it more difficult to maintain.

On top of that, traits as a feature make it more difficult to isolate where variables, constants, signals, methods, and even subclasses may actually be coming from. If your Node's script suddenly has 12 different traits, then you wouldn't necessarily know where everything is coming from. If you see a reference to something, then you'd have to look through 12 different files to even know where a thing is located in the codebase.

This actually then tanks the debuggability of GDScript as a language since any given problem may require you to pick apart on average 2 or 3 different locations in the code base. If any of those locations are hard to find cause they say they are located in one script but are actually located somewhere else - and if the code's readability doesn't clearly state which thing is responsible for the data/logic - then those 2 or 3 steps are being multiplied into an arbitrarily large, highly stressful set of steps.

The increasing size and scope of a project magnifies these negative effects even further and make the use of traits a pretty untenable quality.


But what can be done to resolve the problem? We don't want child "component logic" nodes telling scene roots what to do, but we also can't rely on inheritance or changing whole scripts to solve our problem.

Well, what would any non-ECS engine do in this situation? Composition is still the answer, but in this case a full Node is unreasonable when taken to scale / complicates the dynamic of the ownership hierarchy. Instead, one can just define non-Node implementation objects that abstract away the concrete implementation of a behavior, but which are all still owned by the root node. This can be done with a Reference script.

# root.gd
extends KinematicBody2D

export(Script) var jump_impl_script = null setget set_jump_impl_script
var jump_impl
func set_jump_impl_script(p_script):
    jump_impl = p_script.new() if p_script else null

export(Script) var move_impl_script = null setget set_move_impl_script
var move_impl
func set_move_impl_script(p_script):
    move_impl = p_script.new() if p_script else null

func _physics_process():
    # use logic involving these...
    move_impl.move(...)
    jump_impl.jump(...)

If the exports worked in such a way that we could edit them in the Inspector as enums for classes that derive a particular type like we can for new Resource instances, then that'd be cool. The only way to do that now would be to fix Resource script exports and then make your implementation script extend Resource (for no other reason). Although, having them extend Resource would be a good idea if the implementation itself requires parameters that you'd like to be able to define from the Inspector. :-)

Now, what would make THIS easier would be having a snippet system or macro system so that creating those re-used declarative code sections is easier for developers.

Anyway, yeah, I think I kinda identified glaring issues with a trait system and a better approach to solving the problem. Hooray for XY problem Issues! /s

Edit:

So, the above example's workflow would involve setting the implementation script and then using the script's instance at runtime to define the behavior. But what if the implementation itself requires parameters that you'd like to set statically from the Inspector? Here's a version based on a Resource instead.

# root.gd
extends KinematicBody2D

# if you use a Resource script AND had a way of specifying that the assigned Resource 
# must extend that script, then the editor would automatically assign an instance of 
# that resource script to the var. No separate instancing or setter necessary.

export(Resource) var jump_impl = null # set jump duration, max height, tween easing via Inspector
export(Resource) var move_impl = null # similarly customize movement from Inspector

# can then create different Resources as different implementations. Because they are resources,
# one can edit them even outside of a scene!
func _physics_process():
    move_impl.move(...)
    jump_impl.jump(...)

@willnationsdev
Copy link
Contributor Author

Related: #22660

@willnationsdev
Copy link
Contributor Author

@AfterRebelion

Take into account, i'm talking about the workflow shown, not the feature used

It's ironic that, after you clarify this and I agree with the optimal workflow, and then disagree with later portions of the comment, I then follow it up by basically saying how the "feature used" in that video is actually the ideal way to tackle this problem in Godot anyway. Haha.

@AfterRebelion
Copy link

# root.gd
extends KinematicBody2D

export(Script) var jump_impl_script = null setget set_jump_impl_script
var jump_impl
func set_jump_impl_script(p_script):
jump_impl = p_script.new() if p_script else null
...

Wow, didn't know export was so powerful. Expected it to only be able to interact with primitives, and other data structures.

This makes my earlier comment invalid.
As you said, if some kind of macro is implemented to make its implementation easier, it would be the best way to implement that workflow without the need for MultiScript. Maybe not as versatile as Unity, because you would still need to declare every possible script beforehand, but still a good option.

@vnen
Copy link
Member

vnen commented Mar 24, 2019

I am no sure if it was discussed, but I think it should be possible to invoke trait specific version of a function.

I was already considering this, but I'm not sure about allowing a class to use conflicting traits (that is, traits that define method with the same name). The order of using statements should make no difference.

Also I am not entirely sure about the "no runtime cost". How would be handled dynamically loaded scripts (not available during export) with classes which are using traits which were defined before export? Am I misunderstanding something? Or that case is not considered "runtime"?

It's not about exporting. It will definitely impact loading time, since compilation happens at loading (though I don't think it'll be very significant), but it shouldn't impact when the script is being executed. Ideally scripts should be compiled on export, but that's another discussion.

@busy8
Copy link

busy8 commented May 29, 2019

Hello everyone.

I am new to Godot and have been getting used to it over the past few days. As I tried to figure out the best practices to use to make reusable components I had decided on a pattern. I would always make the root Node of a sub-scene, that is intended to be instanced in another scene, export all the properties that I intend to be set from the outside. As much as possible, I wanted to make knowledge of the internal structure of the instanced branch unnecessary to the rest of the scene.

To make this work the root node has to "export" properties and then copy the values to the appropriate child in _ready. So, for example, imagine a Bomb node with a child Timer. The root Bomb node in the sub-scene would export "detonation_time" and then it would do $Timer.wait_time = detonation_time in _ready. This allows us to set it nicely in Godot's UI whenever we instance it without having to make children editable and drill down to the Timer.

However

  1. It's a very mechanical transformation so it seems like something similar could be supported by the system
  2. It probably adds a slight inefficiency over setting the appropriate value directly in the child node.

Before I go on, this might seem tangential to what is being discussed because it doesn't involve allowing a sort of "private" inheritance (in C++ parlance). However, I actually like Godot's system of building behaviors by composing Scene elements instead of more inheritance-like engineering. Those "written-in" relationships are unchanging and static. OTOH, the scene structure is dynamic, you can even change it at run time. Game logic is so very subject to change during development that I think Godot's design fits the use case very well.

It is true that child nodes are used as behavioral extensions of root nodes but that does not cause them to lack self-sufficiency, IMO. A timer is perfectly self-contained and predictable in behavior regardless of what it is used to time. Whether you use a spoon to drink soup or eat ice cream, it suitably performs its function even though it's acting as an extension of your hand. I view root nodes as maestros that coordinate the behaviors of child nodes so they don't have to know directly about each other and are THEREFORE able to remain self-contained. Parent/root nodes are desk-bound managers that delegate responsibilities but don't do much direct work. Since they're thin, it's easy to create a new one for slightly different behavior.

However, I think root nodes should also act as the primary INTERFACE to the functionality of the entire instanced branch. All properties that can be tweaked in the instance should be "settable" in the root node of the branch even if the ultimate owner of the property is some child node. Unless I'm missing something, this has to be manually arranged in the current version of Godot. It would be nice if this could be automated somehow to combine the benefits of a dynamic system with easier scripting.

One thing I'm thinking about is a system of "dynamic inheritance", if you will, available to subclasses of Node. There would be two sources of properties/methods in such a script, those of the script it extends and those "bubbled up" from children within the scene structure. So my example with the Bomb would become something like export lifted var $Timer.wait_time [= value?] as detonation_time within the bomb.gd script's member variables section. The system would essentially generate $Timer.wait_time = detonation_time in the _ready callback and generate the getter/setter that will allow $Bomb.detonation_time = 5 from the parent of Bomb node to result in $Timer.wait_time = 5 being set.

In the OP's example with MoveRightTrait we'd have the node to which mysprite.gd is attached have MoveRightTrait as a child node. Then in mysprite.gd we'd have something like lifted func $MoveRightTrait.move_right [as move_right] (perhaps 'as' could be optional when the name will be the same). Now calling move_right on a script object created from mysprite.gd would automatically delegate to the appropriate child node. Perhaps signals could be bubbled so they can be attached to a child node from the root? Perhaps whole nodes could be bubbled with just lifted $MoveRightTrait [as MvR] without func, signal or var. In this case all methods and properties on MoveRightTrait would be accessible from mysprite directly as mysprite.move_right or through mysprite.MvR.move_right if the 'as MvR' is used.

That's one idea of how to simplify creation of an INTERFACE to a scene structure in the root of an instanced branch, increasing their "black box" characteristic and getting scripting convenience along with the power of Godot's dynamic scene system. Of course, there would be many side details to consider. For example, unlike base classes child nodes can be removed at run time. How should the bubbled/lifted functions and properties behave if called/accessed in that error case? If a node with the right NodePath is added back do the lifted properties start working again ? [YES, IMO] Also it would be an error to use 'lifted' in classes not derived from Node since there would never be children to bubble/lift from in that case. In addition, name clashes are possible with duplicated "as {name}" or "lifted $Timer1 lifted $Timer2" where nodes have properties/methods with the same name. The script interpreter would ideally detect such logical problems.

I feel like this would give us a lot of what we want even though it's really just syntactic sugar that saves us from having to write forwarding functions and initializations. Also, because it's fundamentally simple conceptually, it shouldn't be that hard to implement or explain.

Anyway, if you got this far thanks for reading!

@busy8
Copy link

busy8 commented May 29, 2019

I used "lifted" everywhere but that's just illustrative.
Something like using var $Timer.wait_time as detonation_time or using $Timer is obviously just as good. In any case you get to conveniently pseudo-inherit from child nodes, creating a coherent single point of access to the desired functionality in the root of the branch to be instanced. The requirement on the reusable pieces of functionality is that they extend Node or a subclass of it so that they can be added as children to the larger component.

Another way of looking at it is that the "extends" keyword on a script that inherits from a Node gives you your "is-a" relationship while using the "using" or "lifted" keyword on a script to "bubble up" a descendant node's members gives you something akin to "implements" [hey, possible keyword] that exists in languages with single inheritance but multiple "interfaces" (eg. Java). In unrestricted multiple inheritance (like c++) base classes form a [static, written-in] tree. By analogy I'm kind of proposing layering convenient syntax and boilerplate elimination over Godot's existing Node trees.

If it is ever determined that this is something worth exploring, there are aspects of the design space to consider:

  1. Should we allow only immediate children in a "using". IOW using $Timer but not `using $Bomb/Timer'? This would be simpler but would force us to write boilerplate in some cases. I say that a full NodePath ROOTED in the Node to which the script is attached should be legal [but NO references to parents/siblings allowed].
  2. Should there be an option that find_node's the "using"-ed node instead of following a written in NodePath? For example using "Timer" with a string for the pattern would be slower but the forwarding architecture would continue to work if a referenced node's position in the sub-tree changes at run time. This could be used selectively for child nodes that we expect to move around beneath the root. Of course syntax would have to be worked out especially when using a particular member (eg. using var "Timer".wait_time as detonation_time is icky).
  3. Should there be a way query for certain functionality [equivalent to asking if an interface is implemented or a child node is present]? Perhaps "using" entire nodes with aliases should allow testing the alias to be a query. So using MoveRightTrait as DirectionalMover in a script would result in node.DirectionalMover returning the child MoveRightTrait. This is logical because node.DirectionalMover.move_right() calls the method on the child MoveRightTrait. Other nodes without that statement would return null. So the statement if node.DirectionalMover: would become a test for the functionality by convention.
  4. The State Pattern should be implementable by replacing a "using"-ed node with another that has variant behavior but the same interface [duck typing] and same NodePath as referenced in the "using" statement. With the way the scene tree works this would work out almost for free. However, the system would have to track signals connected through a parent and restore connections in the replaced child.

@mnn
Copy link

mnn commented May 30, 2019

I have been working with GDScript for some time now and I have to agree, some kind of trait/mixin and proxy/delegation feature is direly needed. It is quite annoying having to setup all this boilerplate just to connect properties or expose methods of children in the root of the scene.

Or adding levels of the tree only to simulate components (it gets quite cumbersome rather quickly, because then you break all node paths with each new component). Maybe there is a better way, something like meta/multi script allowing multiple scripts on one node? If you have idiomatic solution, please share...

Throwing C++ (GDNative) into mix makes things even worse, because _ready and _init behave differently there (read: initialization with default values half-works or doesn't work at all).

@lmerriam
Copy link

lmerriam commented Jun 14, 2019

This is the main thing I have to work around in GDScript. I often need to share functionality across nodes without structuring my entire inheritance structure around it -- for example, my player and shopkeepers have an inventory, my player+items+enemies have stats, my player and enemies have equipped items, etc.

Currently I implement these shared 'components' as classes or nodes loaded into the 'entities' that require them, but it's messy (adds lots of searches for nodes, makes duck-typing nearly impossible, etc) and alternative approaches have their own drawbacks so I haven't found a better way. Traits/mixins would absolutely save my life.

What it comes down to is being able to share code across objects without using inheritance, which I think is both necessary and not possible to do cleanly in Godot right now.

@raymoo
Copy link
Contributor

raymoo commented Jun 29, 2019

The way I understand Rust traits (https://doc.rust-lang.org/1.8.0/book/traits.html), is that they are like Haskell typeclasses, where you require some parameterized functions to be defined for the type you are adding a trait to, and then you get to use some generic functions defined over any types that implement a trait. Are Rust traits something different from what's proposed here?

@AfterRebelion
Copy link

@Zireael07
Copy link
Contributor

This one will probably be migrated wholesale, as it's had extensive discussion here.

@Xrayez
Copy link
Contributor

Xrayez commented Sep 17, 2019

I stumbled upon this issue a year ago, but only now I begin to understand the potential usefulness of the trait system.

Sharing my current workflow in hopes to inspire someone to understand the problem better (and perhaps suggest a better alternative other than implementing traits system).

1. Create a tool to generate component templates for each of used node type in the project:

@willnationsdev #23101 (comment)

Now, what would make THIS easier would be having a snippet system or macro system so that creating those re-used declarative code sections is easier for developers.

Walking in your footsteps... 😅

tool
extends EditorScript

const TYPES = [
	'Node',
	'Node2D',
]
const TYPES_PATH = 'types'
const TYPE_BASENAME_TEMPLATE = 'component_%s.gd'

const TEMPLATE = \
"""class_name Component{TYPE} extends {TYPE}

signal host_assigned(node)

export(bool) var enabled = true

export(NodePath) var host_path
var host

func _ready():
	ComponentCommon.init(self, host_path)"""

func _run():
	_update_scripts()


func _update_scripts():

	var base_dir = get_script().resource_path.get_base_dir()
	var dest = base_dir.plus_file(TYPES_PATH)

	for type in TYPES:
		var filename = TYPE_BASENAME_TEMPLATE % [type.to_lower()]
		var code = TEMPLATE.format({"TYPE" : type})
		var path = dest.plus_file(filename)

		print_debug("Writing component code for: " + path)

		var file = File.new()
		file.open(path, File.WRITE)
		file.store_line(code)
		file.close()

2. Create a static method to be reused for initializing components to host (root for instance):

class_name ComponentCommon

static func init(p_component, p_host_path = NodePath()):

	assert(p_component is Node)

	# Try to assign
	if not p_host_path.is_empty():
		p_component.host = p_component.get_node(p_host_path)

	elif is_instance_valid(p_component.owner):
		p_component.host = p_component.owner

	elif is_instance_valid(p_component.get_parent()):
		p_component.host = p_component.get_parent()

	# Check
	if not is_instance_valid(p_component.host):
		push_warning(p_component.name.capitalize() + ": couldn't find a host, disabling.")
		p_component.enabled = false
	else:
		p_component.emit_signal('host_assigned')

This is how a component (trait) looks like once generated with the first script:

class_name ComponentNode2D extends Node2D

signal host_assigned(node)

export(bool) var enabled = true

export(NodePath) var host_path
var host

func _ready():
	ComponentCommon.init(self, host_path)

(Optional) 3. Extend the component (trait)

@vnen #23101 (comment)

That's my idea. I believe it's simple enough and cover pretty much all use cases for code reuse. I would even allow the class using the trait to override the trait methods (if it's possible to do on compile-time). Traits could also extend other traits.

Walking in your footsteps... 😅

class_name ComponentMotion2D extends ComponentNode2D

const MAX_SPEED = 100.0

var linear_velocity = Vector2()
var collision

export(Script) var impl
...

Actually, the exported Scripts are used in these components to drive the behavior of specific host/root node types per component. Here, ComponentMotion2D will have mainly two scripts:

  • motion_kinematic_body_2d.gd
  • motion_rigid_body_2d.gd

So the children still drive the host/root behavior here. The host terminology comes from me using state machines, and here's where traits wouldn't be perfect fit perhaps, because the states are better organized as nodes imo.

The components themselves are "hardwired" into the root by making them onready members, effectively decreasing the boilerplate code (with the expense of actually having to reference them as object.motion)

extends KinematicBody2D

onready var motion = $motion # ComponentMotion2D

@TheColorRed
Copy link

Not sure if this would help solve the problem, but C# has a thing called extension methods that extend the functionality of a class type.

Basically the function must be static, and the first parameter is required and must be self. It would look like this as a definition:

extension.gd

# any script that uses this method must be an instance of `Node2D`
static func distance(self source: Node2D, target: Node2D):
	return source.global_position.distance_to(target.global_position)

# any script that uses this method must be an instance of `Rigidbody2D`
# a `Sprite` instance cannot use this method
static func distance(self source: Rigidbody2D, target: Node2D):
	return source.global_position.distance_to(target.global_position)

Then when you want to use the distance method, you then just do this:

player.gd

func _ready() -> void:
	print(self.distance($Enemy))
	print($BulletPoint.distance($Enemy))

@willnationsdev
Copy link
Contributor Author

I am familiar with it, but that does not help solve the problem. Hehe, thanks though.

@vnen
Copy link
Member

vnen commented Oct 9, 2019

@TheColorRed extension methods were already proposed, but I don't think they are feasible in a dynamic language. I also don't think they solve the root problem that initially started this discussion.


On another note, I'll probably open many of the proposals for GDScript as GIPs (this included, if @willnationsdev don't mind).

I still believe that traits makes the most sense to share code horizontally in an OO language (without multiple inheritance, but I don't want to go that way).

@mnn
Copy link

mnn commented Oct 9, 2019

I don't think they are feasible in a dynamic language

Is GDS dynamic though? Extension methods could be limited to only typed instances and would work exactly same as in other languages - just a syntactic sugar during compilation replacing method call with static method (function) call. Honestly, I would prefer pimps (ext. methods) before prototypes ala JS or other dynamic ways of attaching methods to classes or even just instances.

@jabcross
Copy link
Contributor

jabcross commented Oct 9, 2019

Whatever we decide to do, I hope we don't decide to name it "pimps".

@vnen
Copy link
Member

vnen commented Oct 9, 2019

Is GDS dynamic though?

There are plenty of definitions of "dynamic" in this context. To be clear: the variable type may not be known at compilation time, so the method extension check needs to be done at runtime (which will hurt performance one way or another).

Extension methods could be limited to only typed instances

If we start doing this we might as well make GDScript typed only. But that's a whole other discussion that I don't to get here.

Point is: things shouldn't start or stop working because the user added types to a script. It's almost always confusing when it happens.

Again, I don't think it solves the issue anyway. We are trying to attach the same code to multiple types, while an extension method will add it only to one type.

Honestly, I would prefer pimps (ext. methods) before prototypes ala JS or other dynamic ways of attaching methods to classes or even just instances.

Nobody proposed (yet) attaching methods dynamically (at runtime) and I don't want that either. Traits would be applied statically at compilation time.

@jabcross
Copy link
Contributor

I originally made a comment about Haxe and its mixin macro library, but then I realized most users won't use a third-party language anyway.

@supagu
Copy link
Contributor

supagu commented Jan 20, 2020

I've recently run into a need for this.

I have some objects the user can interact with but cannot share the same parent, but they need similar groups of API's

for example I have some classes that cant inherit from the same parent, but use a similar set of API's:
Warehouse: Finances, Deletion, MouseInteraction + others
Vehicle: Finances, Deletion, MouseInteraction + others
VehicleTerminal: Finances, Deletion, MouseInteraction + others

For the Finances I've used composition, as this requires the least boiler plate code as get_finances_component() is a sufficient API as it doesn't really care about the game object's at all.

The others:
MouseInteraction and Delection I have just had to copy and paste as it needs to know about the game objects, some composition doesn't work here unless I did some weird delegation:

Warehouse:
  func delete():
      get_delete_component().delete(self);

but that doesn't really allow me to override how delete works where if it was an inherited class I could have the ability to re-write some of the deletion code if needed.

@Xrayez
Copy link
Contributor

Xrayez commented Jan 20, 2020

MouseInteraction and Delection I have just had to copy and paste as it needs to know about the game objects, some composition doesn't work here unless I did some weird delegation

I access components via onready nodes currently. I'm doing something similar:

# character.gd

var input = $input # input component

func _set(property, value):
    if property == "focused": # override
        input.enabled = value
    return true

So this:

character.input.enabled = true

becomes this:

character.focused = true

@MathiasBaumgartinger
Copy link

MathiasBaumgartinger commented May 1, 2020

As @Calinou kindly pointed out my issue godotengine/godot-proposals#758 is closely related to this. What do you think of the proposal to be able to add a trait to a group? This could drastically the need for scripts and other overhead.

@ghost
Copy link

ghost commented May 1, 2020

Just would be great to have a way to inject shareable code into classes, and if they have exported values have these appear in the inspector, and have the methods and properties available and detected by code completion.

@clayjohn
Copy link
Member

Feature and improvement proposals for the Godot Engine are now being discussed and reviewed in a dedicated Godot Improvement Proposals (GIP) (godotengine/godot-proposals) issue tracker. The GIP tracker has a detailed issue template designed so that proposals include all the relevant information to start a productive discussion and help the community assess the validity of the proposal for the engine.

The main (godotengine/godot) tracker is now solely dedicated to bug reports and Pull Requests, enabling contributors to have a better focus on bug fixing work. Therefore, we are now closing all older feature proposals on the main issue tracker.

If you are interested in this feature proposal, please open a new proposal on the GIP tracker following the given issue template (after checking that it doesn't exist already). Be sure to reference this closed issue if it includes any relevant discussion (which you are also encouraged to summarize in the new proposal). Thanks in advance!

Note: this is a popular proposal, if someone moves it over to Godot Proposals, please try to summarize the discussion as well.

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

No branches or pull requests