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

GDExtension: Remove redundant method bind hash check #82191

Merged

Conversation

dsnopek
Copy link
Contributor

@dsnopek dsnopek commented Sep 23, 2023

This fixes a bug in PR #81521, which is causing GDExtensions built for Godot 4.1 to fail to load.

That PR maps some old method hashes to new method hashes in order to get the correct method bind. Unfortunately, it runs afoul of a check towards the end of the function:

	if (mb->get_hash() != p_hash) {
		ERR_PRINT("Hash mismatch for method '" + classname + "." + methodname + "'.");
		return nullptr;
	}

This will fail because the hash on the method bind we found doesn't match the hash that was originally requested (because we mapped it to a new hash).

However, this check is completely unnecessary, because ClassDB::get_method_with_compatibility() won't return the method bind unless the hash matched the one being searched for. I think this check may be left over from before ClassDB::get_method_with_compatibility() was added.

So, this PR completely removes the redundant check.

Sorry for missing this when working on PR #81521! This was a failure to do sufficient testing on my part :-/

@dsnopek dsnopek added this to the 4.2 milestone Sep 23, 2023
@dsnopek dsnopek requested a review from a team as a code owner September 23, 2023 13:57
@dsnopek dsnopek assigned akien-mga and unassigned akien-mga Sep 23, 2023
@dsnopek dsnopek requested a review from akien-mga September 23, 2023 13:57
Copy link
Member

@adamscott adamscott left a comment

Choose a reason for hiding this comment

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

The removal seems logical. Thanks for the quick PR to fix this!

@adamscott
Copy link
Member

Works number one, now my project infinite loops 😅 as intended, but that's not the concern of that PR.

@akien-mga akien-mga merged commit d8c3115 into godotengine:master Sep 24, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@dsnopek dsnopek deleted the gdextension-redundant-hash-check branch July 22, 2024 15:28
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.

3 participants