unsafeify World::entities_mut (#4093)

# Objective
make bevy ecs a lil bit less unsound

## Solution
make unsound API unsafe so that there is an unsafe block to blame:

```rust
use bevy_ecs::prelude::*;

#[derive(Debug, Component)]
struct Foo(u8);

fn main() {
    let mut world = World::new();
    let e1 = world.spawn().id();
    let e2 = world.spawn().insert(Foo(2)).id();
    world.entities_mut().meta[0] = world.entities_mut().meta[1].clone();
    let foo = world.entity(e1).get::<Foo>().unwrap();
    // whoo i love having components i dont have
    dbg!(foo);
}
```

This is not _strictly_ speaking UB, however: 
- `Query::get_multiple` cannot work if this is allowed
- bevy_ecs is a pile of unsafe code whose soundness generally depends on the world being in a "correct" state with "no funny business" so it seems best to disallow this
- it is trivial to get bevy to panic inside of functions with safety invariants that have been violated (the entity location is not valid)
- it seems to violate what the safety invariant on `Entities::flush` is trying to ensure
This commit is contained in:
Boxy 2022-03-30 23:52:45 +00:00
parent 2b35dbabfd
commit 637a149910
3 changed files with 14 additions and 4 deletions

View File

@ -199,7 +199,7 @@ impl<'a> core::iter::ExactSizeIterator for ReserveEntitiesIterator<'a> {}
#[derive(Debug, Default)]
pub struct Entities {
pub meta: Vec<EntityMeta>,
pub(crate) meta: Vec<EntityMeta>,
/// The `pending` and `free_cursor` fields describe three sets of Entity IDs
/// that have been freed or are in the process of being allocated:
@ -544,6 +544,12 @@ impl Entities {
}
}
/// Accessor for getting the length of the vec in `self.meta`
#[inline]
pub fn meta_len(&self) -> usize {
self.meta.len()
}
#[inline]
pub fn len(&self) -> u32 {
self.len

View File

@ -135,8 +135,12 @@ impl World {
}
/// Retrieves this world's [Entities] collection mutably
///
/// # Safety
/// Mutable reference must not be used to put the [`Entities`] data
/// in an invalid state for this [`World`]
#[inline]
pub fn entities_mut(&mut self) -> &mut Entities {
pub unsafe fn entities_mut(&mut self) -> &mut Entities {
&mut self.entities
}

View File

@ -189,7 +189,7 @@ impl Plugin for RenderPlugin {
// reserve all existing app entities for use in render_app
// they can only be spawned using `get_or_spawn()`
let meta_len = app_world.entities().meta.len();
let meta_len = app_world.entities().meta_len();
render_app
.world
.entities()
@ -198,7 +198,7 @@ impl Plugin for RenderPlugin {
// flushing as "invalid" ensures that app world entities aren't added as "empty archetype" entities by default
// these entities cannot be accessed without spawning directly onto them
// this _only_ works as expected because clear_entities() is called at the end of every frame.
render_app.world.entities_mut().flush_as_invalid();
unsafe { render_app.world.entities_mut() }.flush_as_invalid();
}
{