Skip to content

Correct type for char16 and char32 meta#1554

Merged
dsnopek merged 1 commit into
godotengine:masterfrom
raulsntos:char-metadata
Aug 23, 2024
Merged

Correct type for char16 and char32 meta#1554
dsnopek merged 1 commit into
godotengine:masterfrom
raulsntos:char-metadata

Conversation

@raulsntos

Copy link
Copy Markdown
Member

@raulsntos raulsntos added enhancement This is an enhancement on the current functionality topic:gdextension This relates to the new Godot 4 extension implementation labels Aug 20, 2024
@raulsntos raulsntos added this to the 4.x milestone Aug 20, 2024
@raulsntos raulsntos requested a review from a team as a code owner August 20, 2024 02:53
Comment thread binding_generator.py Outdated
Comment on lines 2724 to 2725
elif meta in type_conversion:
return type_conversion[type_name]

Copy link
Copy Markdown
Member

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?

@raulsntos raulsntos Aug 20, 2024

Copy link
Copy Markdown
Member Author

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).

@akien-mga akien-mga left a comment

Copy link
Copy Markdown
Member

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.

@dsnopek dsnopek left a comment

Copy link
Copy Markdown
Collaborator

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 :-)

@dsnopek

dsnopek commented Aug 22, 2024

Copy link
Copy Markdown
Collaborator

Ack, needs a rebase because I merged PR #1555 first

@dsnopek dsnopek merged commit 19c83a8 into godotengine:master Aug 23, 2024
@raulsntos raulsntos deleted the char-metadata branch August 23, 2024 00:41
@dsnopek

dsnopek commented Sep 3, 2024

Copy link
Copy Markdown
Collaborator

Cherry-picked for 4.3 in PR #1569

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This is an enhancement on the current functionality topic:gdextension This relates to the new Godot 4 extension implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants