General MixinScript improvement discussion

[ghost]

Mixin Script Observations

I took a bit of time to play around with MixinScripts. I didn't do anything complex just basic stuff trying to test out various interactions.

Here are some observations:

1. _init() runs for all mixins but all other default functions appear to only run for the top most Mixin
2. This means that _init can only initialize local variables before node is ready while _ready will only run for the top Mixin and I couldn't find a way to manually call the _ready of other Mixins either
3. owner provides all the members of all the scripts in the MixinScript but only in a mixed up "squashed" state
4. I could not find a way to access specific members of other Mixins from within a Mixin

A few ideas that could improve the MixinScript system

I think MixinScript has a lot of potential but I think it can be improved on to make it a more robust and usable system overall. The idea of combining various scripts into one can be very powerful. Is the closest GDScript had to multiple inheritance.

1. Allow access to any Mixin from any Mixin in the same MixinScript

Expose the MixinScript mixins property in C++ is listed as Vector<Ref<Script> > mixins; in MixinScript.h as a Mixin class get_mixins() or mixins without the ability to set it. This would allow for some crazy next level meta interaction between Mixins (at your own risk) including the ability to call functions that are not "squashed" like calling _ready or _input from manually as a kind of super.

EDIT: So I have played around and I found that there is a way to access all mixins from everywhere. The owner is actually the parent node (simialr to get_parent()) and doing owner.script gives you full access to the MixinScript but all the scripts are Mixins. Doesn't seem to recognize the members though but source_code shows appropriate source.

2. Add a global class_name list that can be checked against or an internal trait system or both.

Class list system

MixinScript would collect all the scripts with a global class_name in an internal array which can be checked against externally to see if that MixinScript belongs to a specific class (kinda like a multiple inheritance system)

For example say you have two scripts with class_name Monster and class_name Skeleton added as Mixins to a MixinScript attached to a node called enemy. The code to check if this enemy is a Skeleton would look something like this if Skeleton in $enemy.script.class_list: do stuff. I have tested this in GDScript and it allows creating a list of class_name references so it should also work internally as well.

Trait system

Mixin class would have a built in variable trait pre-set to "" empty string. All you would need to do is change that trait to something meaningful and the MixinScript would collect that as a traits variable which can be checked against if "monster" in ms.script.traits similar to how you would check in a multiple inheritance system.

.

I also have some other wacky ideas but I think that's it for now.

[Xrayez]

• _init() runs for all mixins but all other default functions appear to only run for the top most Mixin

• This means that _init can only initialize local variables before node is ready while _ready will only run for the top Mixin and I couldn't find a way to manually call the _ready of other Mixins either

This problem is described in godotengine/godot#8546. This is in fact one of the reasons why I renamed MultiScript to MixinScript in order to alleviate the ambiguity. The shadowing behavior is documented as well:

If scripts share the same properties, methods or signals, only the first occurrence of those will be used throughout scripts. This means that name conflict resolution is implicit, so you may want to change the order of mixin scripts via editor or using move_mixin() method at run-time.

At the very least, I think it makes sense to emit a warning if there are multiple callbacks defined for the same callback. The "squashed" state you mention is what describes this behavior. Imagine that you defined the same method in multiple files and mixed them together. In text-based languages, how would you resolve the name conflict? You would either throw a compilation error/warning, or choose to pick the first method included.

Of course, all of this could be further clarified and documented, regardless of future improvements.

---

Class list system

MixinScript would collect all the scripts with a global class_name in an internal array which can be checked against externally to see if that MixinScript belongs to a specific class (kinda like a multiple inheritance system)

I think this is already possible if I understand correctly:

extends Node2D

func _ready():
	var creeper = preload("res://creeper.tscn").instance()

	if Monster in creeper.script.mixins:
		print("It is a monster!")

	if Player in creeper.script.mixins:
		print("It is a player!")

Test project:
mixin_trait_check.zip

[ghost]

I did a bit of more in depth research regarding notifications.

Only specific nodes receive the notifications (not just the 1st one)

_init() (not exactly a node notification) runs for every Mixin

_exit_tree() is called on all the Mixins too

_enter_tree() it's called at the Mixin where is defined the earliest in the Mixin order. So if you have an enter_tree defined in script2 but not in script1 then it runs the script 2 one.

_ready() has the same behavior as above.

So it doesn't get called in the first one but rather in the first one that has one of those callbacks defined. Which would be quite difficult to track.

Properties or methods should emit a warning when shadowed

I believe all other properties or methods are not that important even when shadowed by other scripts.

They do need to emit a warning, however. This would tell the user about the status or functionality of the script. This should be a setting in Project Settings somewhere near GDScript warnings.

Forced calls for _ready or other key calls

ready is very important as it sets the conditions/variables and tells the script that is okay to grab nodes. Without this information you're in the dark. This is why I think ready needs to be force called for all mixins. See below.

In my local repo I added this hack to force calls on all the Mixins that are not called. This hack in fact works quite well! Better than I expected. I also helps that GDScript, VisualScript and GDNative use the exact same calls names while C# uses similar but uppercase _Ready and _EnterTree. I looked around and most other script addons and modules Nim, JavaScript, Python use the exact same naming convention too.

  // hack to enforce the calling of _ready, _enter_tree on all scripts
  if (p_notification == 13 && i != 0) {
        if (instance->has_method("_ready")) instance->call("_ready");
        else if (instance->has_method("_Ready")) instance->call("_Ready");
  } else if (p_notification == 10) {
        if (instance->has_method("_enter_tree")) instance->call("_enter_tree");
        else if (instance->has_method("_EnterTree")) instance->call("_EnterTree");
  }

Script objects can be forced to have specific base classes which is where the autocomplete comes from.

I noticed VisualScript has the ability to manually choose any base class and still be used as a Mixin and gains access to all the chosen class specific autocomplete but I think loses access to owner.

I believe autocomplete might come from base class.

I think we might be able to make some kind of base class chain to allow autocomplete. MixinScript could use owner base class which should probably give both the internal and parent methods/properties. The Mixins chould extend MixinScript instead which would remove the need for having an owner perhaps. I need to experiment with this some more.

Script can run without extending Mixin

The need to enforce extends Mixin is not necessary unless you want to access other parameters from the same MixinScript.

if you are creating a kind of component system with standalone script might make more sense to extend nothing at all.

But I do think extending anything other than Mixin will cause a whole slew of issues .

[Xrayez]

_exit_tree() is called on all the Mixins too

_enter_tree() it's called at the Mixin where is defined the earliest in the Mixin order. So if you have an enter_tree defined in script2 but not in script1 then it runs the script 2 one.

For reference, these are handled by notifications internally, if you use _notification():

extends Mixin

func _notification(what):
	match what:
		Node.NOTIFICATION_READY:
			print("Ready")
		Node.NOTIFICATION_ENTER_TREE:
			print("Enter tree")
		Node.NOTIFICATION_EXIT_TREE:
			print("Exit tree")

This will work for all mixins reliably. So it might be actually a design flaw in GDScript itself.

---

Forced calls for _ready or other key calls

ready is very important as it sets the conditions/variables and tells the script that is okay to grab nodes. Without this information you're in the dark. This is why I think ready needs to be force called for all mixins.

With the ability to override these callbacks via _notification() as seen above, this only becomes a convenience issue, so I'm still not sure about this hack and a potential feature-creep. If those callbacks have to be handled for each notification, this may also impact performance.

I'd say MixinScript is fairly advanced feature for advanced users, so I think the _notification() approach should be documented at least.

Does the hack work with VisualScript?

I'm not particularly against this hack if it greatly improves usability, but we should only consider this as a last resort.

---

They do need to emit a warning, however. This would tell the user about the status or functionality of the script. This should be a setting in Project Settings somewhere near GDScript warnings.

Yes, but those should only be enabled in debug builds as it may impact performance.

---

The need to enforce extends Mixin is not necessary unless you want to access other parameters from the same MixinScript.

In the original PR in Godot that added MultiScript, the Multi object wasn't exposed and it worked correctly. However, GDScript errors would still mention Multi object when you use the wrong base class.

But Mixin object is still needed for:

• Dispatching calls from Mixin to owner for call(). In this sense, Mixin acts as a proxy.
• Exposing owner property.

By the way, this is another reason why I renamed MultiScript to MixinScript, because "Mixin" is a noun. 🙂

[ghost]

For reference, these are handled by notifications internally, if you use _notification():

extends Mixin

func _notification(what):
	match what:
		Node.NOTIFICATION_READY:
			print("Ready")
		Node.NOTIFICATION_ENTER_TREE:
			print("Enter tree")
		Node.NOTIFICATION_EXIT_TREE:
			print("Exit tree")

This will work for all mixins reliably. So it might be actually a design flaw in GDScript itself.

I have tried using _notification but are a lot of issues with this approach:

1. Using _notification in GDScript is slower than forced calls in C++
2. _notification still doesn't fix the issue with readying all Mixins because owner only has the "squashed" state so can only call multi level calls just like any other script.
3. The multilevel call means only the top call is called. Which is not guaranteed to be first script. Is actually the first script where is defined. If _ready is in script2 but not script1 then script2 becomes your top call for ready. Multilevel calls will call the first occurence.
4. Scripts that were never readied remain in the dark as to when grab nodes from the tree.
5. You spend more time creating complex (and potentially slow) inter-compatibility code for each Mixin than actually writing scripts.
6. And if you change the order of the Mixins you need to re-write the inter compatibility code.

Note that I think that onready also doesn't work either based on my tests. I need to test this again but I think only the top level onready will work.

With the ability to override these callbacks via _notification() as seen above, this only becomes a convenience issue, so I'm still not sure about this hack and a potential feature-creep. If those callbacks have to be handled for each notification, this may also impact performance.

See my point 2 above. You can't still manually call other readies from the owner and you can only call the top level ready. Tihs means you would need to make a specific ready for each Mixin and call them from the top level call but you cannot guarantee is going to be the first one.

I'd say MixinScript is fairly advanced feature for advanced users, so I think the _notification() approach should be documented at least.

I agree :)

Does the hack work with VisualScript?

Yes works perfectly. In fact I think it also works with GDNative, JavaScript module, Nim addon, and possibly Python addon as they all use the same _ready name.

I'm not particularly against this hack if it greatly improves usability, but we should only consider this as a last resort.

I think it has to be better thought out. See end of this comment for additional ideas.

Yes, but those should only be enabled in debug builds as it may impact performance.

Yes completely agree.

The need to enforce extends Mixin is not necessary unless you want to access other parameters from the same MixinScript.

In the original PR in Godot that added MultiScript, the Multi object wasn't exposed and it worked correctly. However, GDScript errors would still mention Multi object when you use the wrong base class.

I believe the system should actually not enforce Mixin. See below.

Mixin inter-compatibility should be handled by the MixinScript rather than by the user

Users should ideally put any scripts together and just work (in dream land :)) ) Practically this means MixinScript should abstract some of the complexities of Mixins and allow users to spend more time scripting.

Only add scripts that extend nothing or Mixin that's it.

I think the Mixin requirement should be optional to gain access to the rest of the scripts. It should only be used by scripts that act more like controllers for the etire MixinScript.

Additionally base level class should be set for mixin scripts to the parent node class in order to get local autocomplete from the parent into each Mixins. Haven't figured out how to do this yet.

All nodes should be force readied

Force readying works well because it already reduces much of the complexities and works out of the box.

1. Forced C++ calls are much faster
2. Require less complex inter-compatibility for readying nodes
3. All _ready functions are called (not just top level) which lets all nodes know when is time to start grabbing nodes.
4. The top level call end ups being called twice. Is impossible to guarantee from C++ where the user put their top level. Is not always the first script. So you would need to call them all then the top level calls automatically again.

Multi-super

In Godot 4 they implemented something called super (as you have said).

I think giving access to shadowed calls may create more powerful scripting. In addition to the squashed owner, all Mixins should also be able to individually call functions from specific Mixins (like a mixin super).

For example mixins[3]._ready() or equivalent.

With forced calls

• The yellow shows duplicate calls (first a forced call then a normal Godot pushed call) . Note that the order is also different . enter_tree runs reverse multi_level so it ends up running last while ready runs regular then again forced.
• The red shows VS working as well
• The green shows the interaction with <class_name> in .mixin
• Without forced calls it would simply say hello1 and node 1 entered and there would be no ability to call them individually from script unless you create alternative readies with unique names for each Mixin (too much work)

[sairam4123]

@Xrayez Give Unity's Component System a look, it might help you build this MixinScript.

[Xrayez]

There's https://github.com/GodotECS/godex btw.

[sairam4123]

I know, I just wanted to let you know that you can use Unity's Component System as a base idea.