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] - Added the ability to get or set the last change tick of a system. #5838

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
8 changes: 8 additions & 0 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,14 @@ where
out
}

fn get_last_change_tick(&self) -> u32 {
self.system_meta.last_change_tick
}

fn set_last_change_tick(&mut self, last_change_tick: u32) {
self.system_meta.last_change_tick = last_change_tick;
}

#[inline]
fn apply_buffers(&mut self, world: &mut World) {
let param_state = self.param_state.as_mut().expect(Self::PARAM_MESSAGE);
Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub trait System: Send + Sync + 'static {
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId>;
/// Returns true if the system is [`Send`].
fn is_send(&self) -> bool;

/// Runs the system with the given input in the world. Unlike [`System::run`], this function
/// takes a shared reference to [`World`] and may therefore break Rust's aliasing rules, making
/// it unsafe to call.
Expand Down Expand Up @@ -59,6 +60,14 @@ pub trait System: Send + Sync + 'static {
fn default_labels(&self) -> Vec<SystemLabelId> {
Vec::new()
}
/// Gets the system's last change tick
fn get_last_change_tick(&self) -> u32;
/// Sets the system's last change tick
/// # Warning
/// This is a complex and error-prone operation, that can have unexpected consequences on any system relying on this code.
/// However, it can be an essential escape hatch when, for example,
/// you are trying to synchronize representations using change detection and need to avoid infinite recursion.
fn set_last_change_tick(&mut self, last_change_tick: u32);
Copy link
Member

Choose a reason for hiding this comment

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

As mentioned in #5633, I think this deserves some level of warning around the effects this will have on change detection / that setting it manually could break things if you aren't careful.

}

/// A convenience type alias for a boxed [`System`] trait object.
Expand Down
9 changes: 9 additions & 0 deletions crates/bevy_ecs/src/system/system_chaining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,15 @@ impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for ChainSystem
self.system_a.check_change_tick(change_tick);
self.system_b.check_change_tick(change_tick);
}

fn get_last_change_tick(&self) -> u32 {
Copy link
Member

Choose a reason for hiding this comment

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

I would argue that if we can't implement these functions for every System, they aren't System functions.
That being said, it seems like chained systems should just assume a shared change tick? At the very least, setting last_change_tick for both seems correct.

self.system_a.get_last_change_tick()
}

fn set_last_change_tick(&mut self, last_change_tick: u32) {
self.system_a.set_last_change_tick(last_change_tick);
self.system_b.set_last_change_tick(last_change_tick);
}
}

/// An extension trait providing the [`IntoChainSystem::chain`] method for convenient [`System`]
Expand Down
8 changes: 8 additions & 0 deletions crates/bevy_time/src/fixed_timestep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,14 @@ impl System for FixedTimestep {
fn check_change_tick(&mut self, change_tick: u32) {
self.internal_system.check_change_tick(change_tick);
}

fn get_last_change_tick(&self) -> u32 {
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Why can't we just pipe through the internal system's last_change_tick?

self.internal_system.get_last_change_tick()
}

fn set_last_change_tick(&mut self, last_change_tick: u32) {
self.internal_system.set_last_change_tick(last_change_tick);
}
}

#[cfg(test)]
Expand Down