Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
add try_insert to entity commands (bevyengine#9844)
# Objective - I spoke with some users in the ECS channel of bevy discord today and they suggested that I implement a fallible form of .insert for components. - In my opinion, it would be nice to have a fallible .insert like .try_insert (or to just make insert be fallible!) because it was causing a lot of panics in my game. In my game, I am spawning terrain chunks and despawning them in the Update loop. However, this was causing bevy_xpbd to panic because it was trying to .insert some physics components on my chunks and a race condition meant that its check to see if the entity exists would pass but then the next execution step it would not exist and would do an .insert and then panic. This means that there is no way to avoid a panic with conditionals. Luckily, bevy_xpbd does not care about inserting these components if the entity is being deleted and so if there were a .try_insert, like this PR provides it could use that instead in order to NOT panic. ( My interim solution for my own game has been to run the entity despawn events in the Last schedule but really this is just a hack and I should not be expected to manage the scheduling of despawns like this - it should just be easy and simple. IF it just so happened that bevy_xpbd ran .inserts in the Last schedule also, this would be an untenable soln overall ) ## Solution - Describe the solution used to achieve the objective above. Add a new command named TryInsert (entitycommands.try_insert) which functions exactly like .insert except if the entity does not exist it will not panic. Instead, it will log to info. This way, crates that are attaching components in ways which they do not mind that the entity no longer exists can just use try_insert instead of insert. --- ## Changelog ## Additional Thoughts In my opinion, NOT panicing should really be the default and having an .insert that does panic should be the odd edgecase but removing the panic! from .insert seems a bit above my paygrade -- although i would love to see it. My other thought is it would be good for .insert to return an Option AND not panic but it seems it uses an event bus right now so that seems to be impossible w the current architecture.
- Loading branch information