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

[Merged by Bors] - Add a change detection bypass and manual control over change ticks #5635

Closed

Conversation

alice-i-cecile
Copy link
Member

Objective

Changelog

  • added ChangeDetection::set_last_changed to manually mutate the last_change_ticks field"
  • the ChangeDetection trait now requires an Inner associated type, which contains the value being wrapped.
  • added ChangeDetection::bypass_change_detection, which hands out a raw &mut Inner

Migration Guide

Add the Inner associated type and new methods to any type that you've implemented DetectChanges for.

@alice-i-cecile alice-i-cecile force-pushed the change-detection-yolo branch from 9dca3d1 to f8b565a Compare August 9, 2022 18:14
@alice-i-cecile alice-i-cecile marked this pull request as ready for review August 9, 2022 18:14
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR labels Aug 9, 2022
@alice-i-cecile
Copy link
Member Author

CI failure appears to be due to the Deref / DerefMut impls getting broken somehow.

@alice-i-cecile
Copy link
Member Author

#5838 added similar functionality to systems themselves.

Copy link
Contributor

@jakobhellermann jakobhellermann left a comment

Choose a reason for hiding this comment

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

This is a useful feature for rare cases, and the docs correctly warn against potential footguns and suggests better alternatives for common cases.
Since this cannot be worked around in third-party crates I am very much in favor of adding these methods.

crates/bevy_ecs/src/change_detection.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/change_detection.rs Show resolved Hide resolved
alice-i-cecile and others added 2 commits September 8, 2022 19:59
Co-authored-by: Jakob Hellermann <jakob.hellermann@protonmail.com>
Co-authored-by: Jakob Hellermann <jakob.hellermann@protonmail.com>
@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 X-Controversial There is active debate or serious implications around merging this PR labels Sep 9, 2022
@alice-i-cecile
Copy link
Member Author

Removing the controversial label as this flavor of escape hatch was approved in #5838.

@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-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Sep 9, 2022
@alice-i-cecile
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Sep 9, 2022
…5635)

# Objective

- Our existing change detection API is not flexible enough for advanced users: particularly those attempting to do rollback networking.
- This is an important use case, and with adequate warnings we can make mucking about with change ticks scary enough that users generally won't do it.
- Fixes #5633.
- Closes #2363.

## Changelog

- added `ChangeDetection::set_last_changed` to manually mutate the `last_change_ticks` field"
- the `ChangeDetection` trait now requires an `Inner` associated type, which contains the value being wrapped.
- added `ChangeDetection::bypass_change_detection`, which hands out a raw `&mut Inner`

## Migration Guide

Add the `Inner` associated type and new methods to any type that you've implemented `DetectChanges` for.
@bors bors bot changed the title Add a change detection bypass and manual control over change ticks [Merged by Bors] - Add a change detection bypass and manual control over change ticks Sep 9, 2022
@bors bors bot closed this Sep 9, 2022
nicopap pushed a commit to nicopap/bevy that referenced this pull request Sep 12, 2022
…evyengine#5635)

# Objective

- Our existing change detection API is not flexible enough for advanced users: particularly those attempting to do rollback networking.
- This is an important use case, and with adequate warnings we can make mucking about with change ticks scary enough that users generally won't do it.
- Fixes bevyengine#5633.
- Closes bevyengine#2363.

## Changelog

- added `ChangeDetection::set_last_changed` to manually mutate the `last_change_ticks` field"
- the `ChangeDetection` trait now requires an `Inner` associated type, which contains the value being wrapped.
- added `ChangeDetection::bypass_change_detection`, which hands out a raw `&mut Inner`

## Migration Guide

Add the `Inner` associated type and new methods to any type that you've implemented `DetectChanges` for.
@ickk ickk added the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Oct 27, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
…evyengine#5635)

# Objective

- Our existing change detection API is not flexible enough for advanced users: particularly those attempting to do rollback networking.
- This is an important use case, and with adequate warnings we can make mucking about with change ticks scary enough that users generally won't do it.
- Fixes bevyengine#5633.
- Closes bevyengine#2363.

## Changelog

- added `ChangeDetection::set_last_changed` to manually mutate the `last_change_ticks` field"
- the `ChangeDetection` trait now requires an `Inner` associated type, which contains the value being wrapped.
- added `ChangeDetection::bypass_change_detection`, which hands out a raw `&mut Inner`

## Migration Guide

Add the `Inner` associated type and new methods to any type that you've implemented `DetectChanges` for.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…evyengine#5635)

# Objective

- Our existing change detection API is not flexible enough for advanced users: particularly those attempting to do rollback networking.
- This is an important use case, and with adequate warnings we can make mucking about with change ticks scary enough that users generally won't do it.
- Fixes bevyengine#5633.
- Closes bevyengine#2363.

## Changelog

- added `ChangeDetection::set_last_changed` to manually mutate the `last_change_ticks` field"
- the `ChangeDetection` trait now requires an `Inner` associated type, which contains the value being wrapped.
- added `ChangeDetection::bypass_change_detection`, which hands out a raw `&mut Inner`

## Migration Guide

Add the `Inner` associated type and new methods to any type that you've implemented `DetectChanges` for.
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-Usability A targeted quality-of-life change that makes Bevy easier to use M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

Add an API to manually set change ticks for advanced users
4 participants