Skip to content

Add a test to verify that a ComputedState being remove properly despawn DespawnOnExit entities #22280

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.

Sign up for GitHub

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

Jump to bottom
Open
hukasu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add a test to verify that a ComputedState being remove properly despawn DespawnOnExit entities #22280

hukasu wants to merge 1 commit into from
+72 −0

Conversation

@hukasu
Copy link
Contributor

@hukasu hukasu commented Dec 26, 2025

Objective

Complement #20320 and have a proof for #22260

Solution

Create a test that shows that DespawnOnExit properly despawns entities on having a ComputedStates being removed (return None on ComputedStates::compute)

Testing

New test and cargo run -p ci
@hukasu
Copy link
Contributor Author

hukasu commented Dec 26, 2025

#20320 already has a test, maybe this one is not distinct enough to warrant being added
kfc35
kfc35 approved these changes Dec 26, 2025
View reviewed changes
Copy link
Contributor

@kfc35 kfc35 left a comment

Choose a reason for hiding this comment

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

I think the test is distinct enough personally!
@hukasu hukasu added A-ECS Entities, components, systems, and events C-Testing A change that impacts how we test Bevy or how users test their apps D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 26, 2025
it-me-joda
it-me-joda approved these changes Dec 31, 2025
View reviewed changes
Copy link
Contributor

@it-me-joda it-me-joda left a comment

Choose a reason for hiding this comment

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

Great test!
@it-me-joda
Copy link
Contributor

it-me-joda commented Dec 31, 2025

#20320 already has a test, maybe this one is not distinct enough to warrant being added

I think it its distinct enough and if I was implementing a similar feature I would write both cases but I like to have more tests than the average person usually.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Testing A change that impacts how we test Bevy or how users test their apps D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward

4 participants

@hukasu @it-me-joda @kfc35 @Firesz25