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

re-added MultiScript #8502

Merged
merged 1 commit into from
Apr 24, 2017
Merged

re-added MultiScript #8502

merged 1 commit into from
Apr 24, 2017

Conversation

karroffel
Copy link
Contributor

@karroffel karroffel commented Apr 23, 2017

Based on the talk I had with @reduz in Paris:

The very first Godot version (when it was open sourced) had "MultiScript" which lets you use multiple scripts on one object.
With the addition of mulitple new scripting languages (VisualScript, soon C# and GDNative) it can be of use to combine scripts rather than delegating (with huge maintainance cost) or creating child nodes
which could impact performance.

I used the code from 0b806ee as the base and made it work with the current master.

@@ -1696,9 +1696,9 @@ void GDScriptLanguage::reload_tool_script(const Ref<Script> &p_script, bool p_so
//same thing for placeholders
#ifdef TOOLS_ENABLED

while (E->get()->placeholders.size()) {
for (Set<PlaceHolderScriptInstance *>::Element *P = E->get()->placeholders.front(); P; P = P->next()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that when I tried to save a GDScript that's a part of a multiscript the editor froze.

This was an endless loop before, this fixes that.

The very first Godot version (when it was open sourced) had "MultiScript" which lets you use multiple scripts on one object.
With the addition of mulitple new scripting languages (VisualScript, soon C# and GDNative) it can be of use to combine scripts rather than delegating (with huge maintainance cost) or creating child nodes
which could impact performance.

I used the code from 0b806ee as the base and made it work with the current master.
Copy link
Contributor

@neikeq neikeq left a comment

Choose a reason for hiding this comment

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

This is madness.

@avril-gh
Copy link
Contributor

awesome , very useful 💯 been missing this functionality in godot

@reduz
Copy link
Member

reduz commented Apr 24, 2017 via email

@nunodonato
Copy link
Contributor

Wow I never though this was possible, quite cool!

It may be cleaner to add children with scripts, but its so no nice because then the path to access the nodes changes depending on that.

I'm willing to give this a try and split my huge .gd scripts into smaller logical ones :)

@akien-mga akien-mga merged commit 4c14700 into godotengine:master Apr 24, 2017
@karroffel
Copy link
Contributor Author

Because one object can only have one script I create a dummy object for each script that delegates the call to the "real" object.

That means that a script need to extends Object.
The dummy object is of type Multi, I had this type added to ClassDB so a script can explicitly say that it's part of a MultiScript.

Question: Should I re-enable that type so scripts are easier identifyable as "part of a MultiScript" or just keep it as object?

@eon-s
Copy link
Contributor

eon-s commented Apr 24, 2017

I'm curious too about the benefit, uses and meaning of the multiple scripts (having that documented would be nice).

karroffel added a commit to karroffel/godot that referenced this pull request Apr 24, 2017
I changed the loop in godotengine#8502, turns out it fixed the error I was facing but introduced a new one. This fixes both
@novemberist
Copy link

Though with the addition of the Multiscript-entry, I can't seem to create a Visual Script anymore. I can't select it from the menu and trying to create a new vscript with Multiscript selected throws an error at me.

@karroffel
Copy link
Contributor Author

@novemberist I noticed that as well, my code didn't touch anything VS related, it actually can't load the VS files.

I don't think it's the fault of this PR, but I'd love to be corrected! 😉

@novemberist
Copy link

@karroffel actually already existing .vs scripts can be loaded by multiscript (even though there is still something wrong with running them #8040 )

@karroffel
Copy link
Contributor Author

@novemberist Yeah, I can reproduce, not sure why, but I encountered a similar error when I first started with GDNative.

For a temporary fix, you could disable a scripting language you don't need. For example module_multiscript_enabled=no when building Godot.

@avril-gh
Copy link
Contributor

avril-gh commented Apr 24, 2017

Its not like i insist for multi scripts attached to one node. Indeed we can create multiple nodes, each with one script and its same good. So just chose whats more clean / better performance.

I'm curious too about the benefit, uses and meaning of the multiple scripts.

Aside from simple manipulating node to which script is attached there are uses going far beyound it.
For example, imagine complex NPC character which is self sufficient and full autonomous. He not only manipulates its 'body / animations / ect' but also 'live' within environment and scenario he is put in. So within this NPC object, there might be also AI scripts that defines 'how and why' he will act depending on scenario progress and situation he find himself in. (so thats not even about his own nodes). There might be many topics / threads of scenario and its good to separate them into smaller unrelated parts. Having one big script for everything could end up with a big mess where even author could be gone.

@eon-s
Copy link
Contributor

eon-s commented Apr 25, 2017

@avril-gh but the first script extends the node (in some way, can be changed on runtime), adds/modifies properties and methods; what is the relation of the n+1 script with the node and the other scripts?

@avril-gh
Copy link
Contributor

but the first script extends the node (...), adds/modifies properties and methods; what is the relation of the n+1 script with the node and the other scripts?

It is common practice that script that is attached to a node extends it and operate on its methods/properties, but its not obligatory.

This could be limitation, and exacly what i LOVE in godot is its mega flexibility, where combining nodes into scenes then instancing one into another, is best example of godot's limitles awesomeness.
Its no different for scripts. Node is just a holder of a script which might operate on anything it want.

For example, Lets say i make NPC which is self sufficient object (scene)
Whats more i'll make him in totaly separate project where game world does not exists.
All code related to him (including his own navigation) is only within him.
Updating him, changing his AI, appearance, behavior, whatever, will be done only within himself and not require even one change to game world. Its logical and easy to maintain even in large projects.
Then i drop him into game world - and he will just works.

One can say - navigation node, navmesh and script, they all should be bound. Well, no...
Navigation node, navmesh and all the rest - are part of game world (surface).
However surface does not walk. Therefore logicaly, navigation script as part of NPC AI is - within NPC.

character

Whats more, as i said above. I just drop NPC to game world and he must work.
He obviously know nothing about world he are droped into because they are totaly separated objects.
So, NPC must find navmesh and camera if he use it ect. I do it simply by 'taging' important items with group like 'nav', 'camera' ect and then NPC do

nav = get_tree().get_nodes_in_group("navigation")[0]

so i have navigation node, wherever it been, same with camera, and use it simply like

path = nav.get_simple_path(begin, end, true)

even tho, i do it in script attached to simple node, which extends Node and is within NPC not in navmesh nor extends it. (look NPC picture above)

This way its modular and easy to maintain. (All changes within NPC without touching game world)
As you can see on attached image, theres also something else script and possibly more when required,
it might be related to other things that drive this NPC like AI how he act depending on story progress, another script defining his battle AI, yet another defining something else...

They all are separate scripts to keep on topic in logical way, become modular and easy to maintain in large projects. At least thats how i use 'multi-scripts'.

@eon-s
Copy link
Contributor

eon-s commented Apr 26, 2017

@avril-gh making scenes independent of the environment is what I usually do (more for testing purposes) and after reading, trying and thinking a bit more I see it could make more flexible frameworks and better team work in some cases, also cleaner code/scene-tree respect of loading sub-scripts/adding nodes (although this has some disadvantages, I may open an issue about that).

Noticed that the resulting node is like a fusion of the classes, this means if I add 2 service-like scripts I will be performing daemon fusion? (sorry)

@avril-gh
Copy link
Contributor

Noticed that the resulting node is like a fusion of the classes (...) ?

Exacly... As we speak, more and more wonders about possible problems appear and theres perhaps another ones awaits to jump in.

Considering that current design 'one node - one script' is clean, safe and does not limits game developer who still may put as many nodes with scripts as he wants, - attempt to forcefuly attach many scripts to one node knowing that it will raise complexity and possible conflicts in exchange for no special gain - is like doing it just for an art.

and this way, we came to the same conclusion which @reduz found long a go allready.

I don't think this is that useful, which is why it was removed.. also it has some obvious limitations. It's easier and cleaner to add a child node with another script.

@karroffel
Copy link
Contributor Author

It was worth a try, took me like an hour to port to 3.0.

At least now people can't say we didn't try 😂

Another idea: What about introducing a special syntax for "subscripts" in GDScript?
It could look like this

extends Node

subscript input "res://AI_input.gd"
subscript sound "res://default_sound_manager. gd"

func _ready():
    input.connect("move_forward", self, "move_forward")

func move_forward(some_arg):
    ...

The subscript syntax would automatically create child nodes and add the scripts to it and automatically onready get the nodes.
Exported variables could be put in their own section in the property inspector, making it more modular to add new "components".

@eon-s
Copy link
Contributor

eon-s commented Apr 26, 2017

@karroffel subscripts sound cool, it can clean a lot of scenes and plugins

karroffel added a commit to karroffel/godot that referenced this pull request May 11, 2017
removes MultiScript which was re-added in godotengine#8502 (aka 4c14700).
This feature didn't turn out to be as useful as most expected. It causes more troubles than it does good.
akien-mga pushed a commit that referenced this pull request May 26, 2017
I changed the loop in #8502, turns out it fixed the error I was facing but introduced a new one. This fixes both

(cherry picked from commit 67886ba)
@karroffel karroffel deleted the multiscript branch June 21, 2017 16:29
groscalin pushed a commit to groscalin/godot that referenced this pull request Jul 15, 2017
I changed the loop in godotengine#8502, turns out it fixed the error I was facing but introduced a new one. This fixes both
groscalin pushed a commit to groscalin/godot that referenced this pull request Jul 15, 2017
removes MultiScript which was re-added in godotengine#8502 (aka 4c14700).
This feature didn't turn out to be as useful as most expected. It causes more troubles than it does good.
@Xrayez
Copy link
Contributor

Xrayez commented Jun 19, 2021

For those stumbling upon this PR, since this feature got removed in Godot in #8718, I still got curious about the feature and decided to resurrect it in Goost (using Godot 3.3), see goostengine/goost#92 (with dedicated script editor, and the ms extension for scripts to be serializable).

Also, the implementation prior to removal in #8718 was buggy, I've fixed some things to make not crash in some cases.

Mixing in GDScript and VisualScript works, haven't tested other languages, but I think this would be really nice feature for combining GDScript + NativeScript functionalities (probably that's why karroffel got inspired to re-add this feature in the first place).

@dreamsComeTrue
Copy link
Contributor

Are you crazy?! That's insane we now have multiscript support ! 😄
What's missing is some documentation, on what is the order of execution of them, and also - is it possible to communicate between them?
Great work though, one step closer to making Unity getting shy even more 😛

@Xrayez
Copy link
Contributor

Xrayez commented Jun 21, 2021

To eliminate confusion, Godot won't likely have this feature in future versions, but I'm going to merge the feature in Goost, and eventually make a release of Godot + Goost extension once Godot 3.4 comes out (or so), and lets see how it goes!

As discussed above, it's not like this feature will revolutionize the way scripting is done in Godot. The particular implementation is suboptimal, but the idea is really great (that's why this PR exists and goostengine/goost#92), but unfortunately "multi-script" does have limitations which can only be eliminated on the core level (the most prominent one is lack of autocompletion in GDScript, since all classes must extend Object or Mixin class for this to work. It would be more beneficial that GDScript implements traits in GDScript 2.0.

That said, I wouldn't recommend using "multi-script" on main classes like characters (anything which is going to be used often in a game, really).

But even with those limitations, this could be used for:

  • embedding reusable properties in custom resources
  • make reusable code between projects which does not change often
  • you have a team of people which code in GDScript, and another one in VisualScript/C#. Now, you can combine both functionalities using MixinScript.
  • combine several existing node components into one script, so you will only have to deal with one component per node instead of ten of them, say GameObject node that you add as child to characters and props (in a way combining both Godot's node composition approach and having those components have multiple scripts). This way, you can also minimize the amount of code needed to expose those several child components via onready keyword, because you'll only have one. You could have the same GameObject added as child to each host node of interest, but can attach different mix of scripts as well.

What's missing is some documentation, on what is the order of execution of them, and also - is it possible to communicate between them?

If a script is placed below the other and both have the same method, only the method defined in the class above will be called. This is a property of mixins, name resolution is done implicitly, unlike traits. That's why I've renamed MultiScript to MixinScript in goostengine/goost#92, and added ability to re-order the scripts via editor.

Communicating between scripts is possible by using owner property defined in Mixin class. The owner property represents the node/object which has MixinScript attached. And since you can access the owner of the script via other scripts, you can do anything.

I've already tried to describe this in goostengine/goost#92, the PR does have some minimal documentation for future built-in classes.

@me2beats
Copy link

There can I read about the main issues with having multiscripts?

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.