Do not panic on non-UI child of UI entity #9621
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Legitimately, bevy emits a WARN when encountering entities in UI trees without NodeBunlde components. Bevy pretty much always panics when such a thing happens, due to the update_clipping system. However, sometimes, it's perfectly legitimate to have a child without UI nodes in a UI tree. For example, as a "seed" entity that is consumed by a 3rd party plugin, which will later spawn a valid UI tree. The presence of the WARN hints that bevy does not intend to panic on such occasion (otherwise the warn! would be a panic!) so I assume panic is an unintended behavior, aka a bug. Alternatives ------------ Instead of early-returning on invalid entity in `update_clipping`, do not call it with invalid entity in its recursive call.
nicopap
added
C-Bug
ickshonpe
approved these changes
Aug 29, 2023
mockersf
approved these changes
Aug 29, 2023
mockersf
added
the
S-Ready-For-Final-Review
rdrpenguin04
pushed a commit
to rdrpenguin04/bevy
that referenced
this pull request
Jan 9, 2024
Legitimately, bevy emits a WARN when encountering entities in UI trees without NodeBunlde components. Bevy pretty much always panics when such a thing happens, due to the update_clipping system. However, sometimes, it's perfectly legitimate to have a child without UI nodes in a UI tree. For example, as a "seed" entity that is consumed by a 3rd party plugin, which will later spawn a valid UI tree. In loading scenarios, you are pretty much guaranteed to have incomplete children. The presence of the WARN hints that bevy does not intend to panic on such occasion (otherwise the warn! would be a panic!) so I assume panic is an unintended behavior, aka a bug. ## Solution Early-return instead of panicking. I did only test that it indeed fixed the panic, not checked for UI inconsistencies. Though on a logical level, it can only have changed code that would otherwise panic. ## Alternatives Instead of early-returning on invalid entity in `update_clipping`, do not call it with invalid entity in its recursive call. --- ## Changelog - Do not panic on non-UI child of UI entity
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Legitimately, bevy emits a WARN when encountering entities in UI trees without NodeBunlde components.
Bevy pretty much always panics when such a thing happens, due to the update_clipping system.
However, sometimes, it's perfectly legitimate to have a child without UI nodes in a UI tree. For example, as a "seed" entity that is consumed by a 3rd party plugin, which will later spawn a valid UI tree. In loading scenarios, you are pretty much guaranteed to have incomplete children.
The presence of the WARN hints that bevy does not intend to panic on such occasion (otherwise the warn! would be a panic!) so I assume panic is an unintended behavior, aka a bug.
Solution
Early-return instead of panicking.
I did only test that it indeed fixed the panic, not checked for UI inconsistencies. Though on a logical level, it can only have changed code that would otherwise panic.
Alternatives
Instead of early-returning on invalid entity in
update_clipping, do not call it with invalid entity in its recursive call.Changelog