Skip to content

Introduce CheckChangeTicks event that is triggered by World::check_change_ticks #19274

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

urben1680
Copy link
Contributor

@urben1680 urben1680 commented May 18, 2025

Objective

In the past I had custom data structures containing Ticks. I learned that these need to be regularly checked to clamp them. But there was no way to hook into that logic so I abandoned storing ticks since then.

Another motivation to open this up some more is to be more able to do a correct implementation of System::check_ticks.

Solution

Add CheckChangeTicks and trigger it in World::check_change_ticks.
Make Tick::check_tick public.

This event makes it possible to store ticks in components or resources and have them checked.

I also made Schedules::check_change_ticks public so users can store schedules in custom resources/components for whatever reasons.

Testing

The logic boils down to a single World::trigger call and I don't think this needs more tests.

Alternatives

Making this obsolete like with #15683.


Showcase

From the added docs:

use bevy_ecs::prelude::*;
use bevy_ecs::component::CheckChangeTicks;

#[derive(Resource)]
struct CustomSchedule(Schedule);

let mut world = World::new();
world.add_observer(|tick: Trigger<CheckChangeTicks>, mut schedule: ResMut<CustomSchedule>| {
    schedule.0.check_change_ticks(tick.get());
});

@janhohenheim janhohenheim added A-ECS Entities, components, systems, and events A-Time Involves time keeping and reporting D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 18, 2025
@urben1680
Copy link
Contributor Author

An idea I had as well was to make all check_change_ticks methods to take CheckChangeTicks instead of Tick for a stricter context and to make it self-explaining where this value is taken from.

This would however touch numerous files, a good few of them because it changes the System trait method too. What do others think?


impl CheckChangeTicks {
/// Get the `Tick` that can be used as the parameter of [`Tick::check_tick`].
pub fn get(self) -> Tick {
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, I would have expected this to take &self, but I guess this is actually cheaper since it's Copy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, never use &self for tiny Copy types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events A-Time Involves time keeping and reporting D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants