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] - Improve debugging tools for change detection #4160

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ pub trait DetectChanges {
///
/// **Note**: This operation is irreversible.
fn set_changed(&mut self);

/// Returns the change tick recording the previous time this component (or resource) was changed.
///
/// Note that components and resources are also marked as changed upon insertion.
///
/// 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;
}

macro_rules! change_detection_impl {
Expand All @@ -67,6 +76,11 @@ macro_rules! change_detection_impl {
.component_ticks
.set_changed(self.ticks.change_tick);
}

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

impl<$($generics),* $(: $traits)?> Deref for $name<$($generics),*> {
Expand Down
28 changes: 25 additions & 3 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,11 +1164,33 @@ impl<'w, 's> SystemParamFetch<'w, 's> for BundlesState {
}
}

/// The [`SystemParamState`] of [`SystemChangeTick`].
/// A [`SystemParam`] that reads the previous and current change ticks of the system.
///
/// A system's change ticks are updated each time it runs:
/// - `last_change_tick` copies the previous value of `change_tick`
/// - `change_tick` copies the current value of [`World::read_change_tick`]
Comment on lines +1170 to +1171
Copy link
Member

Choose a reason for hiding this comment

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

As you're making those fields private, it may be a little confusing to read in docs.rs where they will be hidden

Copy link
Member Author

Choose a reason for hiding this comment

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

So, my thought was that users should be able to piece together what's going on because we expose methods of the same name.

///
/// Component change ticks that are more recent than `last_change_tick` will be detected by the system.
/// Those can be read by calling [`last_changed`](crate::change_detection::DetectChanges::last_changed)
/// on a [`Mut<T>`](crate::change_detection::Mut) or [`ResMut<T>`](crate::change_detection::ResMut).
#[derive(Debug)]
pub struct SystemChangeTick {
pub last_change_tick: u32,
pub change_tick: u32,
last_change_tick: u32,
change_tick: u32,
}

impl SystemChangeTick {
/// Returns the current [`World`] change tick seen by the system.
#[inline]
pub fn change_tick(&self) -> u32 {
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
self.change_tick
}

/// Returns the [`World`] change tick seen by the system the previous time it ran.
#[inline]
pub fn last_change_tick(&self) -> u32 {
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
self.last_change_tick
}
}

// SAFE: Only reads internal system state
Expand Down