Update deferred calls to use Callables #86301
Conversation
|
Ah brilliant, I wanted to do this for so long :) You have an extra empty line, it seems, which fails static checks. |
| callable_mp((Control *)this, &Control::update_minimum_size).call_deferred(); | ||
| callable_mp((Control *)get_line_edit(), &Control::update_minimum_size).call_deferred(); |
There was a problem hiding this comment.
This is already called automatically when NOTIFICATION_THEME_CHANGED is received, the only difference is that here it's deferred for both nodes. I wonder if it could be removed.
KoBeWi
force-pushed
the
deferred_cleanup
branch
2 times, most recently
from
45f9a15
to
768c0e5
Compare
KoBeWi
force-pushed
the
deferred_cleanup
branch
2 times, most recently
from
520dcd8
to
d61bb4e
Compare
| @@ -37,7 +37,7 @@ | |||
| #include "core/object/script_language.h" | |||
|
|
|||
| void Callable::call_deferredp(const Variant **p_arguments, int p_argcount) const { | |||
| MessageQueue::get_singleton()->push_callablep(*this, p_arguments, p_argcount); | |||
| MessageQueue::get_singleton()->push_callablep(*this, p_arguments, p_argcount, true); | |||
There was a problem hiding this comment.
This makes the deferred calls print error when they fail, instead of doing so silently.
Using callable_mp() can be very error prone, so this helps finding wrong calls. I just wonder if it has unforeseen side effects.
This comment was marked as resolved.
This comment was marked as resolved.
KoBeWi
force-pushed
the
deferred_cleanup
branch
from
d61bb4e
to
f571815
Compare
|
I triple-checked everything, but still missed some cases. Thanks for looking into it. |
KoBeWi
force-pushed
the
deferred_cleanup
branch
from
dabbd82
to
2005281
Compare
There was a problem hiding this comment.
Didn't review in depth, but makes sense to me overall. Could be good to merge after 4.3-dev2 is released, giving us some time to spot potential regressions before dev3.
btw this might fix some bugs, as there were some methods called that weren't bound, resulting in silent fail.
This could actually introduce bugs, if the silent failures are now code that's not supposed to be working, and it working would introduce unwanted changes. So these might be worth flagging for relevant maintainers to look at more closely.
Well, one was in RichTextLabel, I don't remember the other one. |
I see one with |
KoBeWi
force-pushed
the
deferred_cleanup
branch
from
2005281
to
0e8f90f
Compare
|
Thanks! |
Somewhat follow-up to #67730
This PR changes
call_deferred()calls to use Callables, removing many unnecessary method binds as a result. I also changed some usages of MessageQueue to use Callables too.There is still some usage of old
call_deferred()left, namely when a method is vararg or private. I also left some MessageQueue calls in SceneTree, as I'm not sure about performance requirements in there.EDIT:
btw this might fix some bugs, as there were some methods called that weren't bound, resulting in silent fail.