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

Use MethodInfo::get_compatibility_hash() to generate the hash for MethodBind::get_hash() and other GDExtension hash clean up #101449

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Jan 11, 2025

In PR #100674, MethodInfo::get_compatibility_hash() was added so that virtual methods could have compatibility hashes too.

The main goal of this PR is to change MethodBind::get_hash() to use MethodInfo::get_compatibility_hash() so that we have only one, consistent method for generating the hashes.

However, doing that turned up a handful of small bugs which this PR also fixes:

  • There were 3 functions that had more default arguments than arguments - this PR removes those and adds entries to GDExtensionSpecialCompatHashes for them
  • In order to try and prevent this from happening again, I've added a check in ClassDB::bind_method() that'll print out an error if there's more default arguments than arguments
  • GetTypeInfo<GDExtensionConstPtr<T>>::VARIANT_TYPE and GetTypeInfo<GDExtensionPtr<T>>::VARIANT_TYPE is changed from Variant::NIL to the correct value of Variant::INT

These fixes have to be made for the MethodBind::get_hash() change to work, because, while the two methods generate the same exact hashes in the "correct case", they generate different hashes in the "incorrect cases" of the bugs above.

…MethodBind::get_hash()` and other GDExtension hash clean up
@akien-mga akien-mga modified the milestones: 4.x, 4.4 Jan 12, 2025
@akien-mga akien-mga merged commit 23e05b7 into godotengine:master Jan 12, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants