Skip to content

Remove manual apply_deferred in bevy_ui #14768

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

Merged

Conversation

alice-i-cecile
Copy link
Member

Objective

This apply_deferred doesn't seem to have any effect, pointlessly restricts parallelism and is responsible for a large number of system order ambiguities. Spotted as part of #7386.

Solution

Remove it.

This is the only manual apply_deferred in the code base currently.

Testing

I've checked various UI examples and split_screen, and couldn't discern any difference.

This looks like a remnant of a (a, apply_deferred, b).chain() pattern where b got removed, leaving us with a weird vestige.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 15, 2024
@alice-i-cecile alice-i-cecile changed the title Remove manual apply_deferred in bevy_ui Remove manual apply_deferred in bevy_ui Aug 15, 2024
Copy link
Member

@janhohenheim janhohenheim left a comment

Choose a reason for hiding this comment

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

Yeah, definitely seems vestigial.
Also: wow, that tiny change fixed 24 ambiguities?!

@janhohenheim janhohenheim added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 15, 2024
@alice-i-cecile
Copy link
Member Author

Yeah, exclusive world access is heavily conflicting lol.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 15, 2024
Merged via the queue into bevyengine:main with commit 5243fe6 Aug 15, 2024
33 checks passed
@UkoeHB
Copy link
Contributor

UkoeHB commented Aug 15, 2024

What happens if someone disables automatic apply_deferred?

@alice-i-cecile
Copy link
Member Author

That can only be done locally as far as I'm aware. If there's a global setting (now or later), it will break a great deal more than this (which didn't seem to have any effect anyways).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants