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

[3.x] Bind Semaphore.try_wait() #60502

Merged
merged 1 commit into from
Apr 25, 2022
Merged

Conversation

timothyqiu
Copy link
Member

Also updates the documentation of wait() and post(), as they always return OK now.

@timothyqiu timothyqiu added this to the 3.5 milestone Apr 25, 2022
@timothyqiu timothyqiu requested review from a team as code owners April 25, 2022 05:18
@timothyqiu timothyqiu changed the title Bind Semaphore.try_wait() [3.x] Bind Semaphore.try_wait() Apr 25, 2022
@@ -13,13 +13,19 @@
<method name="post">
<return type="int" enum="Error" />
<description>
Lowers the [Semaphore], allowing one more thread in. Returns [constant OK] on success, [constant ERR_BUSY] otherwise.
Lowers the [Semaphore], allowing one more thread in. Always return [constant OK] for backwards compatibility.
Copy link
Member

Choose a reason for hiding this comment

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

For this and wait, since it mentions backwards compatibility, it raises the question of what's the "new" way to use this API where these return values are no longer relevant.

Copy link
Member Author

Choose a reason for hiding this comment

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

The implementation of these two methods changed, so they always succeed. There is no need to check for return value now.

master changed their return type to void.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe the wording could be changed to something like, "This method internals' can't possibly fail, but an error code is returned for back compat, which will always be OK".

@akien-mga akien-mga merged commit 4c47e40 into godotengine:3.x Apr 25, 2022
@akien-mga
Copy link
Member

Thanks!

@timothyqiu timothyqiu deleted the sem-api branch April 25, 2022 13:32
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