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] Physics Interpolation 2D - fix light and light occluder resetting #89577

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

lawnjelly
Copy link
Member

It turns out NOTIFICATION_TRANSFORM_CHANGED is deferred for these nodes, which can mean the transform is not set in the VisualServer until after the reset has been sent, even if the transform is set before the reset in script. This prevented the reset from acting correctly.

Here we explicitly set the transform prior to each reset, to ensure the VisualServer is up to date.

Fixes bug mentioned here:
#88424 (comment)

Notes

  • I hadn't noticed this during development of 2D interpolation. There are already similar techniques in place for canvas items and CPUParticles to deal with this deferred notification.
  • This is probably the simplest way to fix this.
  • It would be possible to do more complex (bug prone) code to ensure the transform was only sent once (after this PR it will be sent twice on a reset tick, once during the reset, and once redundantly during the NOTIFICATION_TRANSFORM_CHANGED), but resetting should be a rare occurrence and this will have minimum impact.

It turns out `NOTIFICATION_TRANSFORM_CHANGED` is deferred for these nodes, which can mean the transform is not set in the `VisualServer` until after the reset has been sent, even if the transform is set before the reset in script. This prevented the reset from acting correctly.

Here we explicitly set the transform prior to each reset, to ensure the `VisualServer` is up to date.
@lawnjelly lawnjelly added this to the 3.6 milestone Mar 16, 2024
@lawnjelly lawnjelly requested a review from a team as a code owner March 16, 2024 16:14
Copy link
Member

@rburing rburing 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.

@lawnjelly lawnjelly mentioned this pull request Mar 16, 2024
4 tasks
@lawnjelly lawnjelly merged commit 64457ba into godotengine:3.x Mar 16, 2024
13 checks passed
@lawnjelly lawnjelly deleted the fti2d_fix_light_resets branch March 16, 2024 17:13
@lawnjelly
Copy link
Member Author

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.

2 participants