-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Bind missing default value for RichTextLabel
methods.
#79053
Bind missing default value for RichTextLabel
methods.
#79053
Conversation
d958404
to
dd9e2b7
Compare
4ef0c4e
to
7fdaa1c
Compare
CC @RedworkDE - We should think about how we want to handle compatibility now that 4.1 is released. Edit: I wouldn't block this PR either way, if we add the 4.1-stable checks in a follow-up PR we can move the expected errors added here to the 4.1 file. |
7fdaa1c
to
8d54d7e
Compare
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.
This needs GDExtension compat methods a-la #76577, or this will break compat between 4.1 and 4.2 (we skipped that for 4.1 because of the more fundamental breaks) and then the validation error will go away as well.
Adding default values should not break anything, call arguments won't change. If it does, it probably should be fixed on godot-cpp side instead. |
The default values are part of the method hash tho, so a build looking for the old hash will no longer find these methods, which is what the compat methods then fix. While it can be argued that the method hash should not depend on the default values, that is an entirely different issue and changing that is way beyond the scope of this PR (it would need to be done here in core tho) |
I think we should convert this pr to draft, and wait a pr to implement generate method hash without default values. @RedworkDE , @bruvzg , @akien-mga what do you think? |
I would just add the compat methods here, as changing how the hash is computed comes with a whole bunch of complications, which adding the compat methods is pretty straight forward and quickly unblocks this PR. |
8d54d7e
to
eb34579
Compare
eb34579
to
4029a05
Compare
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.
Seems fine to me
Thanks! |
Bind missing default value for
RichTextLabel
methods:void push_font()
void set_table_column_expand()
I found that
void push_paragraph()
's default value of argumentp_direction
in c++ isControl::TEXT_DIRECTION_INHERITED
, but useTextServer::DIRECTION_AUTO
to bind.I'm not sure this need to be fixed or not.