-
-
Notifications
You must be signed in to change notification settings - Fork 4k
Add a timer-powered DelayedCommand
for delayed ECS actions
#20155
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
base: main
Are you sure you want to change the base?
Conversation
Having written up a problem description, I'm reasonably confident that the generic is the least fragile and complex option. And also the most likely to work! It is extremely annoying that boxed commands were broken by error handling (or maybe they never worked), but that's not this PR's problem to solve TBH. |
impl Command for DelayedCommand { | ||
/// Spawns a new entity with the [`DelayedCommand`] as a component. | ||
fn apply(self, world: &mut World) { | ||
world.spawn(self); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels weird to me. Why not have users spawn/insert it like other components? We don't impl Command for Observer
, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's an interesting API idea 🤔
We'll also want to support delayed commands in both |
impl Command for DelayedCommand { | ||
/// Spawns a new entity with the [`DelayedCommand`] as a component. | ||
fn apply(self, world: &mut World) { | ||
world.spawn(self); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a perf cost to this; maybe we could have a pre-existing entities that gets re-used?
(and eventually it would be on a resource when resource-as-components lands)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm alright with the performance cost here. Better inspectability + cancellation + a simpler implementation is worth it IMO.
I don't expect this to be super hot, and users can comfortably add their own abstraction if it is.
) { | ||
let delta = time.delta(); | ||
for (entity, mut delayed_command) in delayed_commands.iter_mut() { | ||
delayed_command.delay -= delta; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my similar implementation it does not tick down but has the delay as a point in the future (initialized as: timer.elapsed() + duration
). So instead of ticking down it just compares
let current_time = time.elapsed();
for (entity, delayed_command) in &delayed_commands {
if delayed_command.delay <= current_time {
commands.entity(entity).queue(EvaluateDelayedCommand);
}
}
upside, you can have the query as none mut and have no math operations. Not sure there are any downsides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's super clever. Good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you make it an immutable component, you could even index it using a priority queue so that you only need to check the time of the next delayed command! Although if we're expecting these commands to frequently be despawned or have their timers changed then you might have to remove them from the queue early.
In case it helps: you can convert an fn command_to_boxed_fn(command: impl Command) -> Box<dyn FnOnce(&mut World) + Send + 'static> {
Box::new(|world| command.apply(world))
} And
I think they never worked, and the issue is that The blanket |
Objective
TimedCommands
SystemParam
for easier delayed operations #15129Solution
Create a
DelayedCommand
command, which wraps another command, moves it into the world as an entity, and then ticks down until it's time to evaluate, despawning itself.An equivalent
DelayedEntityCommand
is also provided to allow for easier usage with entity commands.TODO
Commands::queue
requiresHandleError
HandleError
is not dyn compatible, so we cannot simply add it to the box via a helper traitFnOnce
closures, because then you can't use this for commands that store dataHandleError
dyn compatible?queue_delayed
API for usabilityEntityCommand
variant