Improve Timer ergonomics using const generics with defaults

[tim-blackbird]

What problem does this solve or what need does it fill?

Wouldn't it be cool if we could set the initial values for duration and repeating on Timer at compile-time?

What solution would you like?

Change the definition of Timer from:

pub struct Timer { ... }

To:

pub struct Timer<const S: f32 = 0, const R: bool = false> { ... }

impl<const S: f32, const R: bool> Default for Timer<S, R> {
    fn default() -> Self {
        // verbose to prevent recursion on`Default::default()`
        Self {
            stopwatch: Default::default(),
            duration: Duration::from_secs_f32(S),
            repeating: R,
            finished: false,
            times_finished: 0
        }
    }
}

So we can do this:

fn local_timer(
    time: Res<Time>,
    mut timer: Local<Timer<13.5>>,
    mut repeating_timer: Local<Timer<5., true>>
) {
    if timer.tick(time.delta()).just_finished() {
        info!("One off timer just finished!");
    }

    if repeating_timer.tick(time.delta()).just_finished() {
        info!("Repeating timer just finished!");
    }
}

or this, a modified version of the timers example (with comments removed):

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .init_resource::<Countdown>()
        .add_system(countdown)
        .run();
}

#[derive(Default)]
pub struct Countdown {
    pub percent_trigger: Timer<4., true>,
    pub main_timer: Timer<20.>,
}

fn countdown(time: Res<Time>, mut countdown: ResMut<Countdown>) {
    countdown.main_timer.tick(time.delta());

    if countdown.percent_trigger.tick(time.delta()).just_finished() {

        if !countdown.main_timer.finished() || countdown.main_timer.just_finished() {
            info!(
                "Timer is {:0.0}% complete!",
                countdown.main_timer.percent() * 100.0
            );
        }
    }
}

Blocked

By rust-lang/rust#95174 as we can only use integral types (e.g: i32, bool) in const generics for now.

We could use

pub struct Timer<const M: u64 = 0, const R: bool = false> { ... }

// and then
duration: Duration::from_millis(M);

But...

// It's just not the same :c
Timer<2_500>

// I mean you could...
Timer<{(2.5 * 1000.) as u64}>
// Oh no

Breakage

This change compiled on all of bevy, and was tested on the existing timers example.

I don't think this breaks anything other than custom impl's for Timer.

P.S

I don't think this is doable yet, but it sure is neat.

[alice-i-cecile]

Cool! I don't know that you always want these to be const though: in a lot of gameplay applications (think attack speed) you're going to want to change things like the reset period dynamically, or on a per-entity basis.

[tim-blackbird]

You're right Alice. I do think the usefulness here is limited to quick prototyping or super simple cases like game-jam's. Certainly games are meant to be dynamic. But since the implementation is so simple and breakage is minimal it might be something fun to consider.

Oh and only the initial value comes from compile time. It can certainly be changed at run-time.

[alice-i-cecile]

Oh and only the initial value come from compile time. It can certainly be changed at run-time.

Ah I see! Okay cool, I'm open to the idea then!

[cart]

I think I'm opposed to having "two ways" to define timers (static and non-static), especially given that they exist next to each other on the same impl. Additionally, this adds "type complexity" to something that doesn't really need it, and it encourages building systems that "hard code" timer duration, which feels (generally) wrong to me.

[tim-blackbird]

Those are fair points @cart.
Especially with regards to encouraging hard-coding behavior. That should indeed be discouraged.
I do like this for quickly prototyping something, but I see that that may not be worth the cost.

I'll leave it up to you to whether close this issue.

Also. I suppose what I'd really like is a succinct way to set an initial value for any Local<T> not just Timer.
A new-type wrapper with manual Default impl seems like the best way currently. Maybe that's good enough?

What I laid out in this issue could also be implemented as a wrapper. Maybe is should make a crate for it and have it exist outside of Bevy.

[cart]

Also. I suppose what I'd really like is a succinct way to set an initial value for any Local not just Timer.

This used to be possible, but we deprecated in favor of closure systems. That would work for this case.

I think I'll close this out :)

[musjj]

The link to the closure system seems to be broken (it just links to this thread). Does anyone have any examples of how you'd achieve this in the current version?