Deprecate GDExtension's object_cast_to and classdb_get_class_tag, in favour of is_class casts.#119254
Conversation
dsnopek
left a comment
There was a problem hiding this comment.
Thanks!
I agree 100% with marking these as deprecated, but have some notes about the message text.
9950eb7 to
6bb95a6
Compare
|
Thanks, these changes look good to me! However, it just occurred to me that we should also wrap the implementation and registration of these functions in |
6bb95a6 to
de743ef
Compare
|
You're totally right, I forgot about that part 😅 |
de743ef to
578ae9b
Compare
dsnopek
left a comment
There was a problem hiding this comment.
Looks good to me!
Since this only marking something as deprecated (which should be safe) and the replacement is due to a change in Godot 4.7, I think we should target this for Godot 4.7
|
Could you describe the migration path outside of godot-cpp? godotengine/godot-cpp#1974 now uses |
|
|
Yes, downcasts are expected to be implemented purely on the binding side. You can expect the pointer to be stable ( |
Bromeon
left a comment
There was a problem hiding this comment.
Thanks, that makes sense.
Should the deprecation message mention that the original pointer can be used? (The previous API was counter-intuitive, could have just returned a bool instead of a pointer).
578ae9b to
7c8df51
Compare
|
I added something along the lines, please take a look. |
|
Looks good to me, thanks! Btw, nice branch name 😀 |
|
Thanks! |
Object::is_classto takeStringNameinstead ofString, for better performance #118582Objecttypes. Optimizeis_class#105793Object::cast_to<T>()usingObject::is_class()now that it takes aStringNamegodot-cpp#1974Changes the recommended way to cast to
object && object->is_class(T::get_class_name_static()) ? object : nullptr, which is simpler and more predictable than the previous class-tag based approach.We can do this now because
is_classbecame perfectly performant — in fact, it can be faster than class-tag casting by #118582 and #105793