EntityCommands should have methods to fail gracefully

[alice-i-cecile]

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

Commands failing due to an entity being despawned is one of the most common and frustrating sources of panics in Bevy at the moment.

What solution would you like?

1. Enable better backtrace behavior to enable debugging. In particular, we need to know the list of other commands that operated on this entity, and which systems they were issued in.
2. A tool to enable entity commands to fail gracefully (silently ignore / log / warn) rather than panicking.

What alternative(s) have you considered?

Try

For each of the existing methods on EntityCommands, unwrap this option. Create a try_X variant for each method.

This creates significant API bloat and doesn't capture the way that entity commands are chained: after the first operation has succeeded, the rest of the operations in the chain are infallible.

Verified entity flag

Store a verified_entity_exists Option flag on EntityCommands. Each of the existing methods on EntityCommands should attempt to set this flag by unwrapping it to Some(true) if it is None`, and be skipped if it is false.

Then, create a few methods on EntityCommands that set the value of the verification flag with varying behavior:

1. commands.entity(entity).silently_fail_if_missing().insert(Foo)
2. commands.entity(entity).log_if_missing().insert(Foo)
3. commands.entity(entity).warn_if_missing().insert(Foo)

This doesn't work though, because the check is occuring at command-issuing time.

Additional context

#2241 proposes a much more general approach to this, but has significant complexity and overhead.

#3757 aims for a similar goal, but doesn't actually create a reliable API: entities can be despawned after we check for their existence, resulting in the same panic.

@EmiOnGit expressed interest in this problem in #3840.

[alice-i-cecile]

The critical challenge here is that we need to check evaluation at the time of Command evaluation, not command issuing.

[mockersf]

• Enable better backtrace behavior to enable debugging. In particular, we need to know the list of other commands that operated on this entity, and which systems they were issued in.

I tried to solve this with:

• [Merged by Bors] - log spans on panic when trace is enabled #3848, which will give the source system of a command when it panics
• [Merged by Bors] - add more logs when despawning entities #3851, which will log in which system an entity was despawned

• A tool to enable entity commands to fail gracefully (silently ignore / log / warn) rather than panicking.

As said on discord, I dislike this option as I worry people would jump on ignoring failures instead of fixing their system ordering, potentially leaving their game in an unexpected state

[ndarilek]

I want to respectfully push back against this idea, or at least my perception of this idea, that those of us experiencing this just need to reorder our systems already. :)

There really aren't any good tools for doing this at scale, without introducing all sorts of brittle cross-plugin dependencies. So say I have a damage/health system that despawns any entities with < 0 damage. That means I...run every single system before this destroy system? Unless I'm misunderstanding, that's a lot of .before(DESTROY_LABEL).before(DESTROY_LABEL).before(...) all over my game, and now every plugin in my game has to know about this one very specific point in an unrelated plugin. That, when I as the game designer know that 90% of these current panics can fail without consequence, and the 10% that can't aren't worth adding lots of extra overhead to the other 90% because it's more academically correct. :) I'm sure someone's going to suggest how I can fix this by using events, tagging entities about to despawn, etc. but why? If I as the game designer know these systems can fail to do their thing, then please don't make me have to care about one more thing that doesn't seem strictly necessary. Writing a game is complicated enough without having to fight the engine, says the guy used to fighting engines. :)

To be clear, I'm fine with methods like .silently_fail_if_missing() and similar. Opting in is fine. But right now there's no opt either way, and these crashes are outpacing others for me by a long shot. I just tend to see a lot more understanding for the "your commands don't make sense!" perspective than I do the "Building a game out of emergent complexity is hard enough already!" perspective.

[cart]

Some other solutions that might help reduce these problems:

• (optional) entity refcounting (if you have a strong reference, the entity is guaranteed to still be alive)
• "hard system dependencies": force SystemA commands to be applied before running dependent SystemB (this is the biggest feature I want from the "stageless" scheduler we're designing now). This would make it possible for SystemB to directly respond to the effects from SystemA, rather than needing a second data channel to "anticipate" changes. Note that this is already possible by converting your systems to exclusive systems.

[mockersf]

• (optional) entity refcounting (if you have a strong reference, the entity is guaranteed to still be alive)

in the cases that panics now, there are two commands to apply on the same entity: a despawn, then a component insert. How would it work with refcounting? would it be the despawn that would panic?

[ChristopherBiscardi]

One graceful method (try_insert) was added in #9844