-
-
Notifications
You must be signed in to change notification settings - Fork 620
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
Correct type for char16
and char32
meta
#1554
Conversation
binding_generator.py
Outdated
elif meta in type_conversion: | ||
return type_conversion[type_name] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the type_conversion
above and
static const char *argmeta[13] = { "none", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", "float", "double", "char16", "char32" };
does this mean that float
meta in the above array also gets converted to double
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's unfortunate. I think this elif
can be removed since you are never going to get int
or Nil
as metadata, and if you get float
then it means float
(not double
).
- Edit: Opened Avoid hardcoded type conversion for metadata #1555
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In retrospect we should probably have used the proper C++ types as meta from the start, but I guess this would break compat now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This looks good to me :-)
Ack, needs a rebase because I merged PR #1555 first |
a1e1418
to
9a89d22
Compare
Cherry-picked for 4.3 in PR #1569 |
char16_t
andchar32_t
godot#95840.