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

GDScript: Do not reset return value in release build #62317

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

Black-Cat
Copy link
Contributor

Fixes #59239

Reset return value on error only in debug builds. Right now there is no mechanism in release build to know if there was error or not.

@Black-Cat Black-Cat requested a review from a team as a code owner June 22, 2022 17:07
@akien-mga akien-mga added this to the 4.0 milestone Jun 22, 2022
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Makes sense to me that the return value should actually be retrieved.

The comment makes it sounds like it was only intended in case of error/failure while this seems to be used also in non-failing cases, so the comment might need to be changed or maybe this is not in the right place.

CC @vnen

Edit: I misread, this makes the retvalue only be assigned in debug which is consistent with what the comment says. Seems sensible.

@akien-mga akien-mga changed the title Do not reset return value in release build GDScript: Do not reset return value in release build Jun 23, 2022
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Updated my previous review, this looks correct to me.

Copy link
Member

@vnen vnen 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. This was introduced in #58244 but as said here on release there's nothing checking for errors (which is intended). I didn't realize this was out of the DEBUG_ENABLED block.

@akien-mga akien-mga merged commit e3fb066 into godotengine:master Jun 23, 2022
@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.

Function return values are always zero/null/false in 4.0 release export
3 participants