From 9e34c748c6736546537fb5039f4c3de58cc25da3 Mon Sep 17 00:00:00 2001 From: John Date: Wed, 31 Aug 2022 01:53:15 +0000 Subject: [PATCH] Added the ability to get or set the last change tick of a system. (#5838) # Objective I'm build a UI system for bevy. In this UI system there is a concept of a system per UI entity. I had an issue where change detection wasn't working how I would expect and it's because when a function system is ran the `last_change_tick` is updated with the latest tick(from world). In my particular case I want to "wait" to update the `last_change_tick` until after my system runs for each entity. ## Solution Initially I thought bypassing the change detection all together would be a good fix, but on talking to some users in discord a simpler fix is to just expose `last_change_tick` to the end users. This is achieved by adding the following to the `System` trait: ```rust /// Allows users to get the system's last change tick. fn get_last_change_tick(&self) -> u32; /// Allows users to set the system's last change tick. fn set_last_change_tick(&mut self, last_change_tick: u32); ``` This causes a bit of weirdness with two implementors of `System`. `FixedTimestep` and `ChainSystem` both implement system and thus it's required that some sort of implementation be given for the new functions. I solved this by outputting a warning and not doing anything for these systems. I think it's important to understand why I can't add the new functions only to the function system and not to the `System` trait. In my code I store the systems generically as `Box>`. I do this because I have differing parameters that are being passed in depending on the UI widget's system. As far as I can tell there isn't a way to take a system trait and cast it into a specific type without knowing what those parameters are. In my own code this ends up looking something like: ```rust // Runs per entity. let old_tick = widget_system.get_last_change_tick(); should_update_children = widget_system.run((widget_tree.clone(), entity.0), world); widget_system.set_last_change_tick(old_tick); // later on after all the entities have been processed: for system in context.systems.values_mut() { system.set_last_change_tick(world.read_change_tick()); } ``` ## Changelog - Added `get_last_change_tick` and `set_last_change_tick` to `System`'s. --- crates/bevy_ecs/src/system/function_system.rs | 8 ++++++++ crates/bevy_ecs/src/system/system.rs | 9 +++++++++ crates/bevy_ecs/src/system/system_chaining.rs | 9 +++++++++ crates/bevy_time/src/fixed_timestep.rs | 8 ++++++++ 4 files changed, 34 insertions(+) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 8ca47e0f4c311..7344e4463288b 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -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); diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 6a7139b25b35d..7c48df6910bfb 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -31,6 +31,7 @@ pub trait System: Send + Sync + 'static { fn archetype_component_access(&self) -> &Access; /// 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. @@ -59,6 +60,14 @@ pub trait System: Send + Sync + 'static { fn default_labels(&self) -> Vec { 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); } /// A convenience type alias for a boxed [`System`] trait object. diff --git a/crates/bevy_ecs/src/system/system_chaining.rs b/crates/bevy_ecs/src/system/system_chaining.rs index 0a338cf0f4863..a4bc85805102e 100644 --- a/crates/bevy_ecs/src/system/system_chaining.rs +++ b/crates/bevy_ecs/src/system/system_chaining.rs @@ -106,6 +106,15 @@ impl> 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 { + 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`] diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index 8f1c44065c0af..22767033a9dff 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -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 { + 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)]