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

[GDnative] Improve inheritance system in gdnative pluginscript #15761

Merged

Conversation

touilleMan
Copy link
Member

Currently parent class is passed to pluginscript by it name, which is not a good idea:

  • this means there is no namespaces whatsoever (e.g. you cannot have vehicle/ai.xx and human/ai.xx, assuming .xx is a pluginscript-powered script language)
  • it creates a mess if a script has the same name than a default Godot class (i.e. Node2D)

To solve this, pluginscript consider plain name as default Godot class, and otherwise wants a path (i.e. res://vehicle/ai.xx) as parent class attribute.
This way it even allow to inherit from another type of script !

ha ! and this PR also fix memory leaks with forgotten destructor. BTW I don't know if there is a better way to pass data returned from the binding without having to recreate another instance configured through pointer casting, I've tried to use reinterpret_cast but without any luck...

@touilleMan touilleMan force-pushed the pluginscript-improve-inheritance branch from 7992ce5 to 069f4a0 Compare January 15, 2018 22:51
@touilleMan touilleMan changed the title Improve inheritance system in gdnative pluginscript [GDnative] Improve inheritance system in gdnative pluginscript Jan 15, 2018
@touilleMan
Copy link
Member Author

@karroffel sorry to ping you like that but given I updated the PR title to add the [GDNative] tag, you could have missed it...

Given we are very close to final 3.0, I would perfectly understand if this PR has to wait for 3.1 to be merged. However this would mean a breakage in the PluginScript API (parent script passed by name vs by path) which, depending of the success of PluginScript, can be something we want to avoid:

  • if as of 3.1 release only Godot-Python uses PluginScript: we can break the API and no-one care ;-)
  • if other module uses PluginScript: I can rework this PR to add a fallback (if the name is not found, try to search in the already defined pluginscript scripts before erroring)

@karroffel
Copy link
Contributor

Looks okay to me :)

@akien-mga akien-mga added this to the 3.0 milestone Jan 17, 2018
@akien-mga akien-mga merged commit 1699978 into godotengine:master Jan 17, 2018
@touilleMan
Copy link
Member Author

\o/

Let's hope this is my last PR to 3.0 now 😄

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.

None yet

4 participants