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

Add an API to manually set change ticks for advanced users #5633

Closed
alice-i-cecile opened this issue Aug 9, 2022 · 2 comments
Closed

Add an API to manually set change ticks for advanced users #5633

alice-i-cecile opened this issue Aug 9, 2022 · 2 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

This API is essential to ensure the reliability of change detection with rollback networking.

[6:42 AM] Joy: and another thing is I haven't fully figured out how to not break change detection with the whole rollback thing
[6:42 AM] Joy: simulation component and system change ticks need to be reset like everything else that gets rolled back, so I need us to add a method that lets users manually set change ticks

From @maniwani.

What solution would you like?

Add a set_last_changed method to DetectChanges. This should come with warnings in the docs about the need to carefully think through exactly what you're doing.

What alternative(s) have you considered?

Never use change detection in games with rollback networking?

Use a different networking strategy?

Additional context

If we're adding this, we should similarly add a change-detection bypass to avoid users cobbling together an approach to do this by manually setting the change ticks.

This was previously done in #2363, but IMO should be unconditionally enabled instead.

@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 S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Aug 9, 2022
@maniwani
Copy link
Contributor

maniwani commented Aug 9, 2022

For context, any plugins (not just multiplayer) that want to cleanly reset some/all of the data in a World to an earlier state and pretend like nothing happened have to reset system and component change tick values too, or else they'll be different and the second run of your systems might see completely different changes.

Also, this is not something blocking me today, I'm not even that far lol. Another alternative would be attaching your own versions of system and component ticks as Local params and components respectively (e.g. MyCustomTicks<T>), but that's a little clunky and it'd be nice to not have to duplicate anything.

@alice-i-cecile
Copy link
Member Author

Right; we'll need the ability to muck with system change ticks too. IMO that should be a seperate PR, and be done after the bulk of stageless.

@bors bors bot closed this as completed in 54e32ee Sep 9, 2022
nicopap pushed a commit to nicopap/bevy that referenced this issue 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.
james7132 pushed a commit to james7132/bevy that referenced this issue 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 issue 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 S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
2 participants