Skip to content
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

Give compile-time error if registering a class without its own _bind_methods() function #1448

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented Apr 24, 2024

It's already a compile-time error if you make a class without a _bind_methods() that descends from a native class (like Node, Control, etc).

But as pointed out on issue #1430, if you make a class that descends from another extension class, there won't be a compile-time error because the parent class's _bind_methods() will get picked up, and some things won't work correctly (ex virtual method registration, like _ready(), as mentioned in the issue).

This PR attempts to make it a compile-time error if the class doesn't have its own _bind_methods(), ignoring any _bind_methods() inherited from its parent class.

This seems to work in my testing on GCC!

Fixes #1430

@dsnopek dsnopek added bug This has been identified as a bug cherrypick:4.1 cherrypick:4.2 labels Apr 24, 2024
@dsnopek dsnopek requested a review from a team as a code owner April 24, 2024 19:31
@dsnopek dsnopek added this to the 4.x milestone Apr 24, 2024
@pupil1337
Copy link
Contributor

I tested it, work good.

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dsnopek
Copy link
Collaborator Author

dsnopek commented Apr 26, 2024

Thanks for the testing and review!~

@dsnopek dsnopek merged commit 1d829f2 into godotengine:master Apr 26, 2024
12 checks passed
@dsnopek
Copy link
Collaborator Author

dsnopek commented May 17, 2024

Cherry-picked for 4.2 in PR #1465

@dsnopek
Copy link
Collaborator Author

dsnopek commented May 17, 2024

Cherry-picked for 4.1 in PR #1466

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not declare _bind_methods() may be cause not call "_ready()"
4 participants