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

Allow free() to be used as Callable #87294

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

vnen
Copy link
Member

@vnen vnen commented Jan 17, 2024

This method is registered in a special way so ClassDB doesn't naturally know about its existence. Here it is hardcoded if any other option fail to check if it is about the "free()" method and, if so, say it exists and return a Callable.

Fix #86706

@vnen vnen added this to the 4.3 milestone Jan 17, 2024
@vnen vnen requested a review from a team as a code owner January 17, 2024 14:31
@akien-mga akien-mga requested a review from dalexeev January 17, 2024 14:48
@akien-mga
Copy link
Member

It seems to make the GDScript test suite crash:

https://github.com/godotengine/godot/actions/runs/7557248007/job/20576060316?pr=87294

@vnen
Copy link
Member Author

vnen commented Jan 17, 2024

Yeah, the problem is some things still tries to get the MethodBind, which does not exist for free(). I'll change back the has_method to avoid this issue and add a more localized solution in GDScript.

@vnen vnen force-pushed the allow-free-callable branch from 3fc4afd to d0c7480 Compare January 17, 2024 15:16
@vnen vnen requested a review from a team as a code owner January 17, 2024 15:16
Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

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

Looks good to me. Probably worth adding a test.

var node := Node.new()
var callable: Callable = node.free
callable.call()
print(node)

node = Node.new()
callable = node["free"]
callable.call()
print(node)

@akien-mga akien-mga changed the title Allow "free()" to be used as Callable Allow free() to be used as Callable Jan 18, 2024
This method is registered in a special way so ClassDB doesn't naturally
know about its existence. Here it is hardcoded if any other option fail
to check if it is about the `free()` method and, if so, say it exists
and return a Callable.
@vnen vnen force-pushed the allow-free-callable branch from d0c7480 to b4e08eb Compare January 18, 2024 12:34
@vnen
Copy link
Member Author

vnen commented Jan 18, 2024

Added a test.

@akien-mga akien-mga merged commit baf87e2 into godotengine:master Jan 18, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@vnen vnen deleted the allow-free-callable branch January 18, 2024 15:50
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.

Can't use free() as Callable
3 participants