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

Use u64 for change ticks #6327

Closed
wants to merge 6 commits into from
Closed

Use u64 for change ticks #6327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
25d325f
Use u64 for change ticks
zakarumych Oct 21, 2022
d0f8911
Reverse comparison
zakarumych Oct 21, 2022
18602bc
Remove obsolete test
zakarumych Oct 21, 2022
a166289
Remove unnecessary field
zakarumych Oct 21, 2022
881947f
Fix SystemMeta::last_change_tick initialization
zakarumych Oct 21, 2022
700f037
Cleanup imports
zakarumych Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bevy_ecs/macros/src/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream {
unsafe fn init_fetch<'__w>(
_world: &'__w #path::world::World,
state: &Self::State,
_last_change_tick: u32,
_change_tick: u32
_last_change_tick: u64,
_change_tick: u64
) -> <Self as #path::query::WorldQueryGats<'__w>>::Fetch {
#fetch_struct_name {
#(#field_idents:
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream {
state: &'s mut Self,
system_meta: &SystemMeta,
world: &'w World,
change_tick: u32,
change_tick: u64,
) -> Self::Item {
ParamSet {
param_states: &mut state.0,
Expand Down Expand Up @@ -385,7 +385,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
state: &'s mut Self,
system_meta: &#path::system::SystemMeta,
world: &'w #path::world::World,
change_tick: u32,
change_tick: u64,
) -> Self::Item {
#struct_name {
#(#fields: <<#field_types as #path::system::SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_meta, world, change_tick),)*
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ impl BundleInfo {
components: &mut Components,
storages: &'a mut Storages,
archetype_id: ArchetypeId,
change_tick: u32,
change_tick: u64,
) -> BundleInserter<'a, 'b> {
let new_archetype_id =
self.add_bundle_to_archetype(archetypes, storages, components, archetype_id);
Expand Down Expand Up @@ -326,7 +326,7 @@ impl BundleInfo {
archetypes: &'a mut Archetypes,
components: &mut Components,
storages: &'a mut Storages,
change_tick: u32,
change_tick: u64,
) -> BundleSpawner<'a, 'b> {
let new_archetype_id =
self.add_bundle_to_archetype(archetypes, storages, components, ArchetypeId::EMPTY);
Expand Down Expand Up @@ -357,7 +357,7 @@ impl BundleInfo {
add_bundle: &AddBundle,
entity: Entity,
table_row: usize,
change_tick: u32,
change_tick: u64,
bundle: T,
) {
// NOTE: get_components calls this closure on each component in "bundle order".
Expand Down Expand Up @@ -482,7 +482,7 @@ pub(crate) struct BundleInserter<'a, 'b> {
sparse_sets: &'a mut SparseSets,
result: InsertBundleResult<'a>,
archetypes_ptr: *mut Archetype,
change_tick: u32,
change_tick: u64,
}

pub(crate) enum InsertBundleResult<'a> {
Expand Down Expand Up @@ -618,7 +618,7 @@ pub(crate) struct BundleSpawner<'a, 'b> {
bundle_info: &'b BundleInfo,
table: &'a mut Table,
sparse_sets: &'a mut SparseSets,
change_tick: u32,
change_tick: u64,
}

impl<'a, 'b> BundleSpawner<'a, 'b> {
Expand Down
137 changes: 15 additions & 122 deletions crates/bevy_ecs/src/change_detection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,6 @@ use crate::{component::ComponentTicks, ptr::PtrMut, system::Resource};
#[cfg(feature = "bevy_reflect")]
use std::ops::{Deref, DerefMut};

/// The (arbitrarily chosen) minimum number of world tick increments between `check_tick` scans.
///
/// Change ticks can only be scanned when systems aren't running. Thus, if the threshold is `N`,
/// the maximum is `2 * N - 1` (i.e. the world ticks `N - 1` times, then `N` times).
///
/// If no change is older than `u32::MAX - (2 * N - 1)` following a scan, none of their ages can
/// overflow and cause false positives.
// (518,400,000 = 1000 ticks per frame * 144 frames per second * 3600 seconds per hour)
pub const CHECK_TICK_THRESHOLD: u32 = 518_400_000;

/// The maximum change tick difference that won't overflow before the next `check_tick` scan.
///
/// Changes stop being detected once they become this old.
pub const MAX_CHANGE_AGE: u32 = u32::MAX - (2 * CHECK_TICK_THRESHOLD - 1);

/// Types that implement reliable change detection.
///
/// ## Example
Expand All @@ -31,7 +16,7 @@ pub const MAX_CHANGE_AGE: u32 = u32::MAX - (2 * CHECK_TICK_THRESHOLD - 1);
/// use bevy_ecs::prelude::*;
///
/// #[derive(Resource)]
/// struct MyResource(u32);
/// struct MyResource(u64);
///
/// fn my_system(mut resource: ResMut<MyResource>) {
/// if resource.is_changed() {
Expand Down Expand Up @@ -69,15 +54,15 @@ pub trait DetectChanges {
/// For comparison, the previous change tick of a system can be read using the
/// [`SystemChangeTick`](crate::system::SystemChangeTick)
/// [`SystemParam`](crate::system::SystemParam).
fn last_changed(&self) -> u32;
fn last_changed(&self) -> u64;

/// Manually sets the change tick recording the previous time this data was mutated.
///
/// # Warning
/// This is a complex and error-prone operation, primarily intended for use with rollback networking strategies.
/// If you merely want to flag this data as changed, use [`set_changed`](DetectChanges::set_changed) instead.
/// If you want to avoid triggering change detection, use [`bypass_change_detection`](DetectChanges::bypass_change_detection) instead.
fn set_last_changed(&mut self, last_change_tick: u32);
fn set_last_changed(&mut self, last_change_tick: u64);

/// Manually bypasses change detection, allowing you to mutate the underlying value without updating the change tick.
///
Expand All @@ -97,14 +82,14 @@ macro_rules! change_detection_impl {
fn is_added(&self) -> bool {
self.ticks
.component_ticks
.is_added(self.ticks.last_change_tick, self.ticks.change_tick)
.is_added(self.ticks.last_change_tick)
}

#[inline]
fn is_changed(&self) -> bool {
self.ticks
.component_ticks
.is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
.is_changed(self.ticks.last_change_tick)
}

#[inline]
Expand All @@ -115,12 +100,12 @@ macro_rules! change_detection_impl {
}

#[inline]
fn last_changed(&self) -> u32 {
fn last_changed(&self) -> u64 {
self.ticks.last_change_tick
}

#[inline]
fn set_last_changed(&mut self, last_change_tick: u32) {
fn set_last_changed(&mut self, last_change_tick: u64) {
self.ticks.last_change_tick = last_change_tick
}

Expand Down Expand Up @@ -226,8 +211,8 @@ macro_rules! impl_debug {

pub(crate) struct Ticks<'a> {
pub(crate) component_ticks: &'a mut ComponentTicks,
pub(crate) last_change_tick: u32,
pub(crate) change_tick: u32,
pub(crate) last_change_tick: u64,
pub(crate) change_tick: u64,
}

/// Unique mutable borrow of a [`Resource`].
Expand Down Expand Up @@ -333,14 +318,14 @@ impl<'a> DetectChanges for MutUntyped<'a> {
fn is_added(&self) -> bool {
self.ticks
.component_ticks
.is_added(self.ticks.last_change_tick, self.ticks.change_tick)
.is_added(self.ticks.last_change_tick)
}

#[inline]
fn is_changed(&self) -> bool {
self.ticks
.component_ticks
.is_changed(self.ticks.last_change_tick, self.ticks.change_tick)
.is_changed(self.ticks.last_change_tick)
}

#[inline]
Expand All @@ -351,12 +336,12 @@ impl<'a> DetectChanges for MutUntyped<'a> {
}

#[inline]
fn last_changed(&self) -> u32 {
fn last_changed(&self) -> u64 {
self.ticks.last_change_tick
}

#[inline]
fn set_last_changed(&mut self, last_change_tick: u32) {
fn set_last_changed(&mut self, last_change_tick: u64) {
self.ticks.last_change_tick = last_change_tick;
}

Expand All @@ -380,13 +365,8 @@ mod tests {

use crate::{
self as bevy_ecs,
change_detection::{
ComponentTicks, Mut, NonSendMut, ResMut, Ticks, CHECK_TICK_THRESHOLD, MAX_CHANGE_AGE,
},
change_detection::{ComponentTicks, Mut, NonSendMut, ResMut, Ticks},
component::Component,
query::ChangeTrackers,
system::{IntoSystem, Query, System},
world::World,
};

#[derive(Component)]
Expand All @@ -395,93 +375,6 @@ mod tests {
#[derive(Resource)]
struct R;

#[test]
fn change_expiration() {
fn change_detected(query: Query<ChangeTrackers<C>>) -> bool {
query.single().is_changed()
}

fn change_expired(query: Query<ChangeTrackers<C>>) -> bool {
query.single().is_changed()
}

let mut world = World::new();

// component added: 1, changed: 1
world.spawn(C);

let mut change_detected_system = IntoSystem::into_system(change_detected);
let mut change_expired_system = IntoSystem::into_system(change_expired);
change_detected_system.initialize(&mut world);
change_expired_system.initialize(&mut world);

// world: 1, system last ran: 0, component changed: 1
// The spawn will be detected since it happened after the system "last ran".
assert!(change_detected_system.run((), &mut world));

// world: 1 + MAX_CHANGE_AGE
let change_tick = world.change_tick.get_mut();
*change_tick = change_tick.wrapping_add(MAX_CHANGE_AGE);

// Both the system and component appeared `MAX_CHANGE_AGE` ticks ago.
// Since we clamp things to `MAX_CHANGE_AGE` for determinism,
// `ComponentTicks::is_changed` will now see `MAX_CHANGE_AGE > MAX_CHANGE_AGE`
// and return `false`.
assert!(!change_expired_system.run((), &mut world));
}

#[test]
fn change_tick_wraparound() {
fn change_detected(query: Query<ChangeTrackers<C>>) -> bool {
query.single().is_changed()
}

let mut world = World::new();
world.last_change_tick = u32::MAX;
*world.change_tick.get_mut() = 0;

// component added: 0, changed: 0
world.spawn(C);

// system last ran: u32::MAX
let mut change_detected_system = IntoSystem::into_system(change_detected);
change_detected_system.initialize(&mut world);

// Since the world is always ahead, as long as changes can't get older than `u32::MAX` (which we ensure),
// the wrapping difference will always be positive, so wraparound doesn't matter.
assert!(change_detected_system.run((), &mut world));
}

#[test]
fn change_tick_scan() {
let mut world = World::new();

// component added: 1, changed: 1
world.spawn(C);

// a bunch of stuff happens, the component is now older than `MAX_CHANGE_AGE`
*world.change_tick.get_mut() += MAX_CHANGE_AGE + CHECK_TICK_THRESHOLD;
let change_tick = world.change_tick();

let mut query = world.query::<ChangeTrackers<C>>();
for tracker in query.iter(&world) {
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
assert!(ticks_since_insert > MAX_CHANGE_AGE);
assert!(ticks_since_change > MAX_CHANGE_AGE);
}

// scan change ticks and clamp those at risk of overflow
world.check_change_ticks();

for tracker in query.iter(&world) {
let ticks_since_insert = change_tick.wrapping_sub(tracker.component_ticks.added);
let ticks_since_change = change_tick.wrapping_sub(tracker.component_ticks.changed);
assert!(ticks_since_insert == MAX_CHANGE_AGE);
assert!(ticks_since_change == MAX_CHANGE_AGE);
}
}

#[test]
fn mut_from_res_mut() {
let mut component_ticks = ComponentTicks {
Expand Down Expand Up @@ -561,6 +454,6 @@ mod tests {
*inner = 64;
assert!(inner.is_changed());
// Modifying one field of a component should flag a change for the entire component.
assert!(component_ticks.is_changed(last_change_tick, change_tick));
assert!(component_ticks.is_changed(last_change_tick));
}
}
Loading