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

thread 'main' panicked at 'Entity does not exist' #1743

Closed
ndarilek opened this issue Mar 24, 2021 · 18 comments
Closed

thread 'main' panicked at 'Entity does not exist' #1743

ndarilek opened this issue Mar 24, 2021 · 18 comments
Labels
A-ECS Entities, components, systems, and events P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@ndarilek
Copy link
Contributor

Bevy version

47004df)

Operating system & version

Windows 10

What you did

I have a system that adds/removes unit struct components via Commands. Sometimes my game runs fine for several minutes before crashing. Others, it crashes quickly with this panic:

panic: thread 'main' panicked at 'Entity does not exist'

The full log of my run, with Bevy set to debug mode and the system ambiguity checker added, can be found here. I can't pin down an easy reproduction even in my own game--it just seems like things crash after a while and I don't know why. If I'm tracing the panic back correctly to the system in question, it isn't immediately obvious to me that it's accessing despawned entities.

I'm seeing lots of this in the log:

�[2mMar 24 10:46:27.784�[0m �[34mDEBUG�[0m bevy_ecs::system::commands: Failed to despawn non-existent entity 271v1

I'm not sure where this is happening, but this is logging on debug. I'm also not sure how to trace back from this failing command invocation, to whatever system originated it. Obviously I want to do something about these failed despawns--I just don't know how.

What you expected to happen

Not panicking would be nice. as would some mechanism for tracing back from the panic to the specific command that caused it. Maybe some sort of no-op remove (I.e. "remove this component, but don't panic if the entity doesn't exist") would be nice too, though maybe I'm hitting a soundness bug somewhere that shouldn't be papered over by no-op commands. :)

What actually happened

Panics at random times. It's always when interacting with a specific subsystem of my game, but never in response to a specific action type or at a transition point.

Additional information

Happy to provide any, just let me know how to turn logging up to 11. :)

@alice-i-cecile alice-i-cecile added P-Crash A sudden unexpected crash A-ECS Entities, components, systems, and events labels Mar 24, 2021
@alice-i-cecile
Copy link
Member

This discussion hints at a similar bug, introduced in #1471. I expect @cart will be merging a fix shortly, but until then does the behaviour go away if you revert to before that PR was merged?

@ndarilek
Copy link
Contributor Author

ndarilek commented Mar 24, 2021 via email

@alice-i-cecile
Copy link
Member

This does not appear to be the issue in #1743. Seems like they are attempting to insert a component into a non-existent entity. This should be an error as it has real implications for app state (unlike removing components from non-existent entities or despawning non-existent entities, which we have previously agreed should not be errors due to Command synchronization complexity).

From #1748 :) Perhaps add in a check to see if the entity exists before issuing the component insertion command? Failing that, you could write a custom command that checks if the entity exists before trying to insert into it.

Perhaps we should be batch processing commands and always running component insertion before entity deletion, but that's a long way off.

@cart
Copy link
Member

cart commented Mar 25, 2021

Perhaps we should be batch processing commands and always running component insertion before entity deletion, but that's a long way off.

Maybe we need to do command commits in a way that respects system dependencies?

@cart
Copy link
Member

cart commented Mar 25, 2021

Ah wait i think we already do because we topologically sort the "parallel system" list.

@cart
Copy link
Member

cart commented Mar 25, 2021

I'm thinking a simple remove_system.system().after(AddSystemLabel) should resolve this

@alice-i-cecile
Copy link
Member

Maybe we need to do command commits in a way that respects system dependencies?

Yeah, talking with @BoxyUwU about this has convinced me that commands should be handled in the same order as the systems that generated them in order to keep the mental model clear.

@Ratysz
Copy link
Contributor

Ratysz commented Mar 25, 2021

Ah wait i think we already do because we topologically sort the "parallel system" list.

This is correct.

Yeah, talking with @BoxyUwU about this has convinced me that commands should be handled in the same order as the systems that generated them in order to keep the mental model clear.

Applying command buffers in the exact order of the systems that issued them is not possible unless we forbid parallelization: e.g., "order" doesn't make sense for systems A and B where A started before but ended after B. And timestamping and sorting all individual commands by default will most definitely have perf implications.

@mockersf
Copy link
Member

Obviously I want to do something about these failed despawns--I just don't know how.

From my understanding, the issue is that there's no way to trace back the failing command to which system added it, making it hard to debug system order

@ndarilek
Copy link
Contributor Author

ndarilek commented Mar 25, 2021 via email

@mockersf
Copy link
Member

mockersf commented Mar 25, 2021

#846 may be relevant here.

With the new ordering of systems, this would be even more useful

@cart
Copy link
Member

cart commented Mar 26, 2021

Im thinking this this issue isn't really something we can solve in the next day (and system ordering/restructuring logic is a workaround), so I'm removing this from the 0.5 milestone. We can add it back if thats unpopular or if I missed something important.

@cart cart removed this from the Bevy 0.5 milestone Mar 26, 2021
@alice-i-cecile
Copy link
Member

Agreed: I'm much less concerned now that we understand what's going on better.

@ndarilek
Copy link
Contributor Author

ndarilek commented Mar 26, 2021 via email

@edisno
Copy link

edisno commented Apr 3, 2021

This is biting me hard also, and I have not yet been successful in reordering systems to avoid the as of now dreaded "Entity does not exist" crash. As a temporary workaround could this somehow be made into a non-fatal error?

@ndarilek
Copy link
Contributor Author

Sorry for the noise, but this has bitten me twice in the last hour on two different systems. The weird thing is that it seems to happen when things are despawned, and with operations that could safely fail if the entity doesn't exist. I.e. if I'm tweaking something's name, and that thing is gone because a) the player destroyed it or b) the entire game was reset, then it's absolutely fine for that thing to fail in a way that doesn't toast the entire process.

Maybe the ambiguity-checker could help, but that seems like overkill for situations where I don't care if this fails. Could it just be made a non-panic, maybe log it at some high level and move on?

@alice-i-cecile
Copy link
Member

Related to #2581.

@alice-i-cecile alice-i-cecile added the S-Needs-Design This issue requires design work to think about how it would best be accomplished label Dec 12, 2021
@alice-i-cecile
Copy link
Member

Closing; there's more specific issues for remaining improvements.

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 P-Crash A sudden unexpected crash S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

6 participants