Skip to content

Commit

Permalink
Added the ability to get or set the last change tick of a system. (be…
Browse files Browse the repository at this point in the history
…vyengine#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<dyn System<...>>`. 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.
  • Loading branch information
StarArawn authored and james7132 committed Oct 28, 2022
1 parent 07249b0 commit 6c2a0ed
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
8 changes: 8 additions & 0 deletions crates/bevy_ecs/src/system/function_system.rs
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
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);
}

/// 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
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 {
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
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 {
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

0 comments on commit 6c2a0ed

Please sign in to comment.