-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Fix NavigationRegion2D transform update #85258
Conversation
Fixes NavigationRegion2D transform update.
@@ -165,6 +165,7 @@ void NavigationRegion2D::_notification(int p_what) { | |||
|
|||
case NOTIFICATION_INTERNAL_PHYSICS_PROCESS: { | |||
set_physics_process_internal(false); | |||
_region_update_transform(); |
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.
The way internal physics process is used here as a one shot seems like a convoluted way to do a deferred call. I assume call deferred wouldn't be enough for this method in NOTOFICATION_TRANSFORM_CHANGED
?
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.
If I remember correctly the reason this was added as a physics process one shot is because the server does the sync on the physics tick directly after this.
If it is added to the msg queue it would need a bool to stop the transform notification from spamming the msg queue like crazy which was one of the original problems. E.g. animationplayers that moved regions would trigger 1000+ transform update calls at high frame rate for every navigation server sync due to the lower physics tick rate.
I tested the fix locally, and I can confirm it Fixes #85242 |
Thanks! |
Cherry-picked for 4.2.1. |
Fixes NavigationRegion2D transform update.
Fixes #85242
That line slipped on a refactor.