Skip to content
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

It would be nice if Bevy had a commands.entity_option() that returned a Option<EntityCommands<'w, 's, 'a>> #3757

Closed
jm-ra opened this issue Jan 24, 2022 · 5 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@jm-ra
Copy link

jm-ra commented Jan 24, 2022

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

Sometimes commands.entity(entity_id) panics because the entity is already unspawned.

What solution would you like?

commands.entity_option(...) that returns a Option<EntityCommands<'w, 's, 'a>> ... this would be a workaround for commands.entity(...) that can panic.

What alternative(s) have you considered?

panic::catch_unwind doesn't work.

Additional context

Getting the world with exclusive_system to use get_entity doesn't work in my case.

@jm-ra jm-ra added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 24, 2022
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 24, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 24, 2022

This is an interesting idea. It's closely related to #2004, but not a duplicate.

In terms of naming, I'd probably lean towards commands.maybe_entity(entity) myself.

I'm not sure that this gives quite the behavior you want though: the various commands work on EntityCommands, and you can't know at the time of issuing them. Instead, you'd want to store an Option<Entity> in EntityCommands itself, and then fail more gracefully in insert, remove, despawn and so on.

@DJMcNab
Copy link
Member

DJMcNab commented Jan 24, 2022

This is error handling for the easy case where the given entity doesn't actually exist.

Fwiw, I think try_entity is the terminology I'd propose.

That would just be:

pub fn try_entity(&mut self, entity: Entity)->Option<EntityCommands> {
self.entities.contains(entity).then(|| self.entity(entity))
}

@HackerFoo
Copy link
Contributor

It seems like the convention is to use "get" in the name of methods that optionally return things from a collection, especially when there is a non-optional lookup that panics.

@jmonasterio
Copy link

jmonasterio commented Jan 24, 2022

Seeing as how in World, it is called World::get_entity(), maybe that is the convention we should follow? I probably should not have proposed a name... just pointing out the need for a way to do this.

@alice-i-cecile
Copy link
Member

Closing this out in favor of #3845, as discussed in #3840.

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 C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants