godot-cpp icon indicating copy to clipboard operation
godot-cpp copied to clipboard

[WIP] Changes for type-checking

Open Zylann opened this issue 6 years ago • 5 comments

  • Allow to use Ref<T> on GodotScripts extending reference types
  • Type checks on primitives for calls from Godot
  • Type checks on Godot objects using is_class
  • Type checks on GodotScripts using tags and dynamic_cast
  • _notification() is now virtual due to the above, don't register it
  • Added argument count check
  • Rect3 Variant type renamed to AABB
  • Files reorganized a bit

Also introduces usage of standard library because I couldn't use a Godot container for typetag inheritance lookup.

Important note: This has been my work-in-progress so far, some parts are untested. Also, GodotScript<T> could disappear in a future refactoring after we discussed it at GodotCon (@karroffel & @reduz) so this PR may become partially irrelevant and merged into a branch.

Zylann avatar Feb 07 '18 21:02 Zylann

This is awesome!

Have you thought of any ways of doing it without Object meta? I wonder how feasible it'd be to use an opaque pointer to the class's NativeScriptDesc as the typetag instead of something provided by the bindings. That struct has at least some advantages over an arbitrary typetag:

  • has a pointer to its base NativeScriptDesc, so a nativescript API function could test inheritance without a TagDB
  • is created during godot_nativescript_register_class, so could be returned to the binding by that same function to store per class
  • can be retrieved from an Object instance's NativeScript instead of from get_meta (unfortunately it's not much faster, since NativeScript::get_script_desc() does a Map lookup. Maybe NativeScript should cache this in the first place...)
  • is still guaranteed unique per class (unlike NativeScript itself)

Just one possibility though, maybe there are even better ways.

(I guess the reason a NativeScript doesn't cache its NativeScriptDesc is to support reloading the library. That does complicate this suggestion)

sheepandshepherd avatar Feb 08 '18 03:02 sheepandshepherd

I'm working on the first NativeScript extension for 3.0.1, it will have an API for setting and getting type tags. So the meta information doesn't have to be used anymore.

I talked briefly about this with Zylann on IRC, we will wait with this PR until my update goes live since it will make some things in this PR not needed anymore.

karroffel avatar Feb 08 '18 11:02 karroffel

Is this still a valid pull request? @Zylann don't know if you switched to nativescript 1.1 yet but could you have a look at what you want to do with this?

BastiaanOlij avatar Nov 19 '18 10:11 BastiaanOlij

@BastiaanOlij I didn't really do anything with GDNative over the past months, when I did this PR Nativescript 1.1 wasn't really a thing yet. This PR would only be relevant to people using the old API with GodotScript<T>, and even then it has edge cases. If the old API is meant to go away, so does this PR, because the new API has different approach. I just left it here for history.

Zylann avatar Nov 19 '18 12:11 Zylann

@Zylann thanks, I'll leave it here for now then but I'm guessing we can kill it off around the time we release 3.1 official.

BastiaanOlij avatar Nov 20 '18 04:11 BastiaanOlij

Closing as obsolete. Also some of the changes mentioned like renaming Rect3 to AABB are already done.

aaronfranke avatar Jul 08 '23 17:07 aaronfranke