Skip to content

Fix #19219 by moving observer triggers out of resource_scope #19221

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
merged 3 commits into from
May 30, 2025

Conversation

mbrea-c
Copy link
Contributor

@mbrea-c mbrea-c commented May 15, 2025

Objective

Fixes #19219

Solution

Instead of calling world.commands().trigger and world.commands().trigger_targets whenever each scene is spawned, save the instance_id and optional parent entity to perform all such calls at the end. This prevents the potential flush of the world command queue that can happen if add_child is called from causing the crash.

Testing

  • Did you test these changes? If so, how?
  • Are there any parts that need more testing?
    • Pending to run cargo test at the root to test that all examples still build; I will update the PR when that's done
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    • Run bevy as usual
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    • N/a (tested on Linux/wayland but it shouldn't be relevant)

@mbrea-c mbrea-c marked this pull request as ready for review May 15, 2025 21:43
@mbrea-c
Copy link
Contributor Author

mbrea-c commented May 15, 2025

No idea why CI is failing here; the errors coming up are in bevy_render and don't look related to this.

cargo run -p ci -- lints is also succeeding locally on my computer.

Am I missing something or is CI being flaky?

@greeble-dev
Copy link
Contributor

The CI issue should now be resolved: #19222.

@greeble-dev greeble-dev added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 16, 2025
@alice-i-cecile alice-i-cecile added this to the 0.16.1 milestone May 21, 2025
@alice-i-cecile alice-i-cecile 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 May 30, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 30, 2025
Merged via the queue into bevyengine:main with commit 3902804 May 30, 2025
34 checks passed
mockersf pushed a commit that referenced this pull request May 30, 2025
# Objective

Fixes #19219 

## Solution

Instead of calling `world.commands().trigger` and
`world.commands().trigger_targets` whenever each scene is spawned, save
the `instance_id` and optional parent entity to perform all such calls
at the end. This prevents the potential flush of the world command queue
that can happen if `add_child` is called from causing the crash.

## Testing

- Did you test these changes? If so, how?
- Verified that I can no longer reproduce the bug with the instructions
at #19219.
  - Ran `bevy_scene` tests
- Visually verified that the following examples still run as expected
`many_foxes`, `scene` . (should I test any more?)
- Are there any parts that need more testing?
- Pending to run `cargo test` at the root to test that all examples
still build; I will update the PR when that's done
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run bevy as usual
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - N/a (tested on Linux/wayland but it shouldn't be relevant)

---
mockersf pushed a commit that referenced this pull request May 30, 2025
# Objective

Fixes #19219 

## Solution

Instead of calling `world.commands().trigger` and
`world.commands().trigger_targets` whenever each scene is spawned, save
the `instance_id` and optional parent entity to perform all such calls
at the end. This prevents the potential flush of the world command queue
that can happen if `add_child` is called from causing the crash.

## Testing

- Did you test these changes? If so, how?
- Verified that I can no longer reproduce the bug with the instructions
at #19219.
  - Ran `bevy_scene` tests
- Visually verified that the following examples still run as expected
`many_foxes`, `scene` . (should I test any more?)
- Are there any parts that need more testing?
- Pending to run `cargo test` at the root to test that all examples
still build; I will update the PR when that's done
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run bevy as usual
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - N/a (tested on Linux/wayland but it shouldn't be relevant)

---
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-Bug An unexpected or incorrect behavior D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes 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.

Crash when observing SceneInstanceReady trigger
4 participants