-
-
Notifications
You must be signed in to change notification settings - Fork 21.3k
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
Let thread func have optional parameter #38078
Conversation
There was a problem hiding this 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, but this should be tested on the latest master
and 3.x
branches before it can be merged (with the multi-threading modernization in place).
core/bind/core_bind.cpp
Outdated
@@ -2098,10 +2098,11 @@ void _Thread::_start_func(void *ud) { | |||
memdelete(tud); | |||
Callable::CallError ce; | |||
const Variant *arg[1] = { &t->userdata }; | |||
int argc = arg[0]->get_type() != 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use Variant::NIL
, not 0
.
And I'd make the cast from bool
to int
explicit:
int argc = arg[0]->get_type() != 0; | |
int argc = (int)(arg[0]->get_type() != Variant::NIL); |
Sorry for the late review. This would need a rebase for the The fix looks OK to me but I'll let @RandomShaper or @reduz validate as they know this code better. FYI, the email used to author your commit (see https://patch-diff.githubusercontent.com/raw/godotengine/godot/pull/38078.patch) is not affiliated to your GitHub account. It's not a problem for Git, but if you want to claim ownership of that commit, you can add this email as secondary email in your GH settings. |
Force pushed a rebase that solves merge conflict and my above comment. |
Thanks! And congrats for your first merged Godot contribution 🎉 |
Cherry-picked for 3.4. |
Fixes #38042