Commands double (or more) despawn and NoSuchEntity Error.

[kakoeimon]

Right now if we despawn an Entity with Commands and by user error we despawn this Entity more than once we get the error NoSuchEntity and this error is produced by Commands internally. There is no way to cach the error by the side of the user cause despawn in Commands returns it's Self.

This error is very hard to track down. It requires to check all the calls of despawn in a project, etc.

But even doing so there are times where you want to despawn all the entities with a specific Compoment and this will lead to NoSuchEntity errors, the only sollution I was able to come up was to despawn those entities in a thread_local_system.

I believe that silencing the error internally will be a huge gain in productivity. Maybe a call to dbg! to inform the user that more than one calls of despawn to an entity where called it will be ideal too.
I do not know the overhead of all this, but a solution to this problem have to be found.

[mrk-its]

+1 for it - dealing with double despawn panics was hardest thing while developing my own game.

[TheNeikos]

But even doing so there are times where you want to despawn all the entities with a specific Compoment and this will lead to NoSuchEntity errors

Do you have some example code that creates this issue?

[kakoeimon]

Just despawn an entity two times.

[TheNeikos]

Just despawn an entity two times.

If you despawn something twice using an entity reference, that's a bug right? After all, keeping a stale entity reference is useless.

[kakoeimon]

You may need several systems to despawn an entity for several reasons.

[cart]

Yeah at this point I think allowing double despawns feels like the right call. The alternative is synchronization across systems that might free an entity, which imo isn't worth it.

This is a tough call. I cant think of any significant problems from double despawns, whereas synchronization is a significant problem.

Flecs (another popular ecs) uses this approach with success.

[TheNeikos]

Would an option be to return a Result? I think that having potential logic bugs be surfaced outweighs the need to not declare mutability. This way as a user I have a choice what path to take.

[kakoeimon]

Command's methods return Command so it can be nested.

[Bauxitedev]

Seconding this. Right now, I have an entity that holds a 2d array of booleans, encapsulated in a Grid component. I want to draw it as a grid to the screen, so I create a sprite for every cell in the grid. However, the sprites need to be updated if the grid changes. So I use the Changed<Grid> query to detect this. This system basically triggers two things:

1. It deleted all previously spawned sprites.
2. It creates a new sprite for every cell in the grid.

The main problem is 1. Since it uses the Commands interface to despawn the sprites, the actual deletion appears to happen a few frames afterwards. If the grid is changed again in the next frame, then the sprites will be despawned twice, since the previously despawned sprites haven't been fully deleted yet. This results in a panic.

It is quite annoying to keep track of which entities have been despawned. I think Bevy itself should be able to do this, and ignore any double despawns.

As a quick hack I thought I'd just wrap it with panic::catch_unwind but that doesn't work, since Commands is not safe to transfer across panic unwind boundaries...

UPDATE: Just tried keeping track of despawned entities by adding a Despawned component to all entities I've despawned, and checking if an entity doesn't have Despawned before calling despawn on it. This doesn't work either, since the Despawned component is added with the same delay as despawning in the first place.

[CleanCut]

Yeah at this point I think allowing double despawns feels like the right call.

I gave it a shot in #649