Closed kokounet closed 2 years ago
This is an interesting proposal. I think the Timer
and FixedTimeStep
APIs could both use some love to be a bit less full of boilerplate, multiple similar approaches and boilerplateful my_timer.0
methods, which I think fits in well with making sure we can support this use case properly.
To make sure I'm understanding you correctly, the core issues are:
We might be able to get away with 1 through some specialty methods. The latter seems like it might be a better fit for a thin wrapper around Timer, either in the individual game / crate or within Bevy itself. I'm a bit worried about redundancy if we put it in Bevy, but making it a thin wrapper would reduce the maintenance burden substantially.
On FixedTimeStep
, I think that allowing systems and system sets to work with run_criteria (via the proposed system set and explicit system ordering features) would go a long way to eliminating most of the common boilerplate found with Timers
, and make FixedTimeStep
itself a more natural feature to use.
To get rid of the timer.0
ugliness (and confusion for beginners), I think the issue is mostly in sloppy examples. Really, these custom timer structs should be deriving Deref (or From, if you want to be explicit about things) to convert them seamlessly into timers.
struct HelloTimer(Timer);
// These Deref impls are optional sugar to make using these timers more pleasant.
// Rather than having to use `my_hello_timer.0` every time we want to use one of these objects
// we can just call `my_hello_timer` directly
impl Deref for HelloTimer{
type Target = Timer;
fn deref(&self) -> &Self::Target {
&self.0
}
}
impl DerefMut for HelloTimer{
fn deref_mut(&mut self) -> &mut Timer {
&mut self.0
}
}
Not thrilled with the amount of space that takes up; maybe a simple derive trait or proc macro is called for?
The Timer
API is also concerned with this PR #1112, which also propose more sugar for timers, and the feature could be used for the FixedTimestep
thingy. I'm not very familiar with the FixedTimestep
struct though...
One timestep offset between the timer being created and the first time that it can be finished, which can cause avoidable lag in animation and input.
Yes, that is correct, there are other usecases like auto-fire for weapons and stuff like that (cooldowns for spells).
Some nicer sugar for cooldowns, with inverted booleans for code clarity.
Yeah, this is right, although it's a bit more subtle than simply inverted booleans because when a cooldown is ticking it's not available, and the same goes for the Timer
. Although looking similar, Cooldowns and Timers are close but not really for the same use-cases. The first allow doing things before the ticking start, while the latter allow doing things once finished (tracking lifetimes for example).
I'm a bit worried about redundancy if we put it in Bevy [...].
My take on that is that Timers
, Cooldowns
, and just a simple Stopwatch
are very frequent features in games, and it's a small amount of code completely decoupled from the rest of the engine.
Plus, the Stopwatch
could be the base to construct both Timer
and Cooldown
. This would factor out the ticking / starting / resetting mechanism.
On
FixedTimeStep
, I think that allowing systems and system sets to work with run_criteria (via the proposed system set and explicit system ordering features) [...].
I'm not familiar with those features, but I think it will be clearer once the Timer
is sorted out.
To get rid of the
timer.0
ugliness (and confusion for beginners).
I don't know if that's part of my issue, but I agree that the NewType pattern for this is not very adequate. Maybe enforce that the timer is used with a type like Timer<T>
to hint that this feature is designed for specialization as it removes the boilerplate? And if one really must use a simple timer then making an alias for Timer<()>
? But maybe that's worth making a whole new issue 😅
Thanks for the explanations; I think I'm on board with making Cooldowns a standard bit of kit.
I really like the Timer<T>
approach to specialization: that makes the specialization dramatically more clear and cuts down on boilerplate.
I think this calls for a new issue (or PR) to overhaul the Timer-related API at this point :) I'll try to work on this in the next few hours, so don't rush to file a new issue for that.
I tried Timer<T>
, just pushed if you want to take a look: https://github.com/mockersf/bevy/commit/36f95796b164ebfa45e6b6e319e05d069e7762dd
there are two issues:
Oh woaw that's quite a lot of change I wasn't expecting as much!
deriving traits do not work that well when there is a generic type param, that can mostly be worked around
Yes, that is indeed a hassle. Are there still problems with the work around you propose ?
it becomes impossible to register all Timer
type registry here
Can you elaborate a bit more on why this is a problem? User defined timers will be components, resources, or locals in any cases, just like other user defined structs?
Regarding the initial Cooldown
matter I will (tomorrow) make a minimal example of an implementation here, so you can use the code if you find it useful (contrary to what the name suggest, it is not intended to directly fit in bevy as I feel I don't have enough experience with the bevy codebase).
We should probably close this issue since it was merged to bevy 0.5
What problem does this solve or what need does it fill?
A Cooldown is like a timer except it's
available
when constructed and areset
make it available again.The
Timer
struct currently can't be used to make a "cooldown" behavior because of the following minor differences between the two:Timer::finished() == false
Cooldown::available() == true
Timer::finished() == false
Cooldown::available() == true
N.A.
Cooldown::available()
goes fromtrue
tofalse
and the cooldown start ticking (only for non looping Cooldowns).I found the need for this behavior when implementing my sprite sheet animation system for my game. I need that a change of animation (from Idle to Attack for example) occur directly and not on the next animation frame in order to increase responsiveness (I'm sure there are other needs for this particular behavior).
Describe the solution would you like?
Because of the similarity of behavior of both things, my idea would be to have them share part of the code. But it could also be a new struct with a very similar API.
A cooldown could be made with a timer by doing the following:
However, implementing it this way is fragile because it doesn't support looping cooldown, and is a bit messy to use.
Describe the alternative(s) you've considered?
The more I think of it, the more I think that having a different struct for this job is more appropriate for the reason that it's not a complicated behavior and also that the ticking mechanism, although similar, is quite different. Maybe the ticking, pausing, elapsed times could be factored out in a
Chronometer
struct that both those two things could use?Additional context
An example of what a
Cooldown
struct could look like:I'm sorry if this is a bit messy, It's my first issue. I love your work on bevy, I think it's really great! I would be happy to help integrate this feature in bevy if you find it belongs there!