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.

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

Bubbling observers traversal should use query data #15385

Merged
merged 11 commits into from
Sep 23, 2024
Merged

Bubbling observers traversal should use query data #15385

merged 11 commits into from
Sep 23, 2024

Conversation

BenjaminBrienen
Copy link
Contributor

@BenjaminBrienen BenjaminBrienen commented Sep 23, 2024 •
edited
Loading

Objective

Fixes #14331

Solution

  • Make Traversal a subtrait of ReadOnlyQueryData
  • Update implementations and usages

Testing

  • Updated unit tests

Migration Guide

Update implementations of Traversal.

@alice-i-cecile alice-i-cecile added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes A-ECS Entities, components, systems, and events A-Picking Pointing at and selecting objects of all sorts labels Sep 23, 2024
@alice-i-cecile
Copy link
Member

When writing the migration guide, frame it relative to 0.14 (no traversal at all), not the previous state of main.

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Sep 23, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Sep 23, 2024
alice-i-cecile
alice-i-cecile reviewed Sep 23, 2024
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Fundamentals look good; let us know if you need help :)

@BenjaminBrienen BenjaminBrienen marked this pull request as ready for review September 23, 2024 16:05
@NthTensor NthTensor self-requested a review September 23, 2024 16:36
NthTensor
NthTensor approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@NthTensor NthTensor left a comment

Choose a reason for hiding this comment

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

Nice! Very happy with this. Happy to see you remembered to update the docs about () as well.

Thanks!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 23, 2024
@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-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Sep 23, 2024
Merged via the queue into bevyengine:main with commit 27bea6a Sep 23, 2024
27 checks passed
@BenjaminBrienen BenjaminBrienen deleted the Bubbling-observers-Traversal-should-use-QueryData branch September 23, 2024 19:01
@BenjaminBrienen BenjaminBrienen self-assigned this Nov 10, 2024
@BenjaminBrienen BenjaminBrienen removed the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ItsDoot ItsDoot ItsDoot left review comments

@NthTensor NthTensor NthTensor approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees

@BenjaminBrienen BenjaminBrienen

Labels
A-ECS Entities, components, systems, and events A-Picking Pointing at and selecting objects of all sorts C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes
Projects
None yet
Milestone
0.15
Development

Successfully merging this pull request may close these issues.

Bubbling observers Traversal should use QueryData
4 participants
@BenjaminBrienen @alice-i-cecile @ItsDoot @NthTensor