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

Change detection doesn't work inside parallel queries #8656

Closed
Dauthdaert opened this issue May 23, 2023 · 3 comments
Closed

Change detection doesn't work inside parallel queries #8656

Dauthdaert opened this issue May 23, 2023 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Duplicate This issue or PR already exists

Comments

@Dauthdaert
Copy link

Dauthdaert commented May 23, 2023

Bevy version

0.10.0

What you did

I have a query using a Ref parameter and is_changed() to detect if one of two conditions (position changed or override flag) is true in order to avoid calculating FoV every frame. Since it's an expensive operation, I also want to use par_iter_mut in order to make the query parallel.
My problem is that, once I add par_iter_mut to my query, change detection seems to break and position.is_changed() is never true.

Example code

query
    .par_iter_mut() // If I remove this line, then change detection starts working again.
    .for_each_mut(|mut to_change, maybe_changed| {
        if maybe_changed.is_changed() || to_change.some_flag {
            to_change.expensive_change();
        }
   });

What went wrong

I would expect change detection to work in either case, but it seems to only work with sequential queries.

Additional information

In case it changes something, the specific query I'm having trouble with is also inside a ParamSet.
Also, this is a regression from bevy 0.9, where it worked fine.

@Dauthdaert Dauthdaert added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 23, 2023
@hymm
Copy link
Contributor

hymm commented May 23, 2023

I believe this is the same issue already fixed in #8029

@alice-i-cecile alice-i-cecile added S-Duplicate This issue or PR already exists A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels May 23, 2023
@alice-i-cecile
Copy link
Member

Agreed, this is a duplicate: fix will be coming soon in 0.11 :)

@Dauthdaert
Copy link
Author

Dauthdaert commented May 23, 2023

Oops, my mistake. Looked for an existing issue before posting, but didn't think to search for a PR unassociated to an issue.
Thanks.

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 S-Duplicate This issue or PR already exists
Projects
None yet
Development

No branches or pull requests

3 participants