Skip to content

System::check_change_tick and similar methods take CheckChangeTicks #19600

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
Merged
merged 19 commits into from
Jun 13, 2025
Merged

System::check_change_tick and similar methods take CheckChangeTicks #19600

merged 19 commits into from
Jun 13, 2025

Conversation

urben1680
Copy link
Contributor

@urben1680 urben1680 commented Jun 12, 2025 •
edited
Loading

Follow-up of #19274.

Make the check_change_tick methods, of which some are now public, take CheckChangeTicks to make it obvious where this tick comes from, see other PR.

This also affects the System trait, hence the many changed files.

urben1680
urben1680 commented Jun 12, 2025
View reviewed changes
@alice-i-cecile alice-i-cecile requested a review from chescock June 13, 2025 00:27
alice-i-cecile
alice-i-cecile requested changes Jun 13, 2025
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.

I agree with this change (type-safety is really valuable here), but the migration guide really needs to clearly state where you can get the right value for this type. That information should be mirrored on the CheckChangeTicks docs.

@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jun 13, 2025
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 13, 2025
@urben1680
Copy link
Contributor Author

urben1680 commented Jun 13, 2025 •
edited
Loading

Do you mean the information should be mirrored from the CheckChangeTicks docs? As these already contain where to get the value.

@urben1680 urben1680 requested a review from alice-i-cecile June 13, 2025 16:17
chescock
chescock approved these changes Jun 13, 2025
View reviewed changes
@urben1680 @chescock
Update crates/bevy_ecs/src/component.rs
e0d6d24 
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jun 13, 2025
@urben1680
Copy link
Contributor Author

Made World::check_change_ticks return it if the observers were triggered.

chescock
chescock approved these changes Jun 13, 2025
View reviewed changes
urben1680 and others added 3 commits June 13, 2025 19:53
@urben1680 @chescock
Update crates/bevy_ecs/src/observer/mod.rs
4d0a014 
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
@urben1680 @chescock
Update crates/bevy_ecs/src/world/deferred_world.rs
68d522b 
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
@urben1680
Merge branch 'main' into system-check-tick_takes_CheckChangeTicks
cb036e6
@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 Jun 13, 2025
urben1680
urben1680 commented Jun 13, 2025
View reviewed changes
@alice-i-cecile alice-i-cecile enabled auto-merge June 13, 2025 19:12
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 13, 2025
Merged via the queue into bevyengine:main with commit a292ac5 Jun 13, 2025
32 checks passed
hukasu pushed a commit to hukasu/bevy that referenced this pull request Jun 15, 2025
@urben1680 @chescock @hukasu
System::check_change_tick and similar methods take `CheckChangeTick…
a71bdd8 
…s` (bevyengine#19600)

Follow-up of bevyengine#19274.

Make the `check_change_tick` methods, of which some are now public, take
`CheckChangeTicks` to make it obvious where this tick comes from, see
other PR.

This also affects the `System` trait, hence the many changed files.

---------

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@chescock chescock chescock approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
0.17
Development

Successfully merging this pull request may close these issues.
3 participants
@urben1680 @alice-i-cecile @chescock