adding an method to retrieve entity optionally

[EmiOnGit]

Objective

• optionally retrieve an entity from the world if it exists
• Fixes It would be nice if Bevy had a commands.entity_option() that returned a Option<EntityCommands<'w, 's, 'a>> #3757

[alice-i-cecile]

Suggested change

	/// In case of a nonexisting entity, a [`None`] is
	/// If the entity does not exist at the time this command is evaluated, [`None`] is

[alice-i-cecile]

Suggested change

	///

[alice-i-cecile]

This comment is unclear / seems wrong; based on the code, it seems like this will be evaluated as part of the command processing automatically. That is in fact much more useful; it allows users to write commands that fail safely if the chosen entity does not exist.

EDIT: no, this is wrong.

[alice-i-cecile]

Hey, thanks! This is really cool stuff for a first contribution. IMO this should have an additional doc test / example, demonstrating how these entity commands are robust to the target entity not existing (due to being despawned).

You can do this easily using the following basic pattern:

 let mut app = App::new();
 // Add various systems
 app.add_plugins(MinimalPlugins).add_system(maybe_insert_component);
 
 // You can manually remove entities using app.world.despawn(entity)
 
 // Then, you can run the app to verify that everything works
 app.update();

[mockersf]

I think it would still be possible to get a panic due to a non existing entity:

1. System A despawns the entity
2. System B query for the entity
3. Commands are executed, get_entity from system B fails as the entity doesn't exist anymore

It should be mentioned in the doc that there are still cases where the command will panic with a missing entity.

[EmiOnGit]

First of, thank you for your kind response.
I can confirm @mockersf. Therefore I'd rather not have this method merged as most of the users would simply assume its safeness.
I found @alice-i-cecile suggestion in the pinned issue appealing but somewhat of a big step for my first contribution without asking some veterans first.
(to rewrite EntityCommands to hold an Option<Entity> and fail gracefully in the corresponding methods).
I currently see no way for Commands to know if an Entity was deleted in the same cycle so it would result in the same error then my previous try

[alice-i-cecile]

I currently see no way for Commands to know if an Entity was deleted in the same cycle so it would result in the same error then my previous try

Yep, my understanding is that this is effectively impossible.

I can confirm @mockersf. Therefore I'd rather not have this method merged as most of the users would simply assume its safeness.

Agreed.

I found @alice-i-cecile suggestion in the pinned issue appealing but somewhat of a big step for my first contribution without asking some veterans first.

Understandable <3 If you're looking for a bit of a challenge, this would be deeply appreciated (make a new PR so it's easier to review). Feel free to reach out to me (and the rest of the ECS dev team); this is important, well-isolated work and mentoring new contributors is an important use of energy.