Commit graph

94 commits

Author SHA1 Message Date
JMS55
708535536b Document EntityCommands/EntityMut insert() (#6270)
Fixes #6258.
2022-10-17 14:38:58 +00:00
mike
a0f1468108 Add iter_entities to World #6228 (#6242)
# Objective

- Add a way to iterate over all entities from &World

## Solution

- Added a function `iter_entities` on World which returns an iterator of `Entity` derived from the entities in the `World`'s `archetypes`

---

## Changelog

- Added a function `iter_entities` on World, allowing iterating over all entities in contexts where you only have read-only access to the World.
2022-10-17 13:47:00 +00:00
Kewei Huang
2a6b544a0a Document EntityMut::remove() (#6168)
# Objective

- Fixes #5990

## Solution

- Add docs for `EntityMut::remove()` explaining its return value.
2022-10-05 12:21:09 +00:00
Dawid Piotrowski
0bf7f3153d Allow access to non-send resource through World::resource_scope (#6113)
# Objective

Relaxes the trait bound for `World::resource_scope` to allow non-send resources. Fixes #6037.

## Solution

No big changes in code had to be made. Added a check so that the non-send resources won't be accessed from a different thread.

---

## Changelog
 - `World::resource_scope` accepts non-send resources now
 - `World::resource_scope` verifies non-send access if the resource is non-send
 - Two new tests are added, one for valid use of `World::resource_scope` with a non-send resource, and one for invalid use (calling it from a different thread, resulting in panic)

Co-authored-by: Dawid Piotrowski <41804418+Pietrek14@users.noreply.github.com>
2022-09-28 17:53:58 +00:00
Carter Anderson
01aedc8431 Spawn now takes a Bundle (#6054)
# Objective

Now that we can consolidate Bundles and Components under a single insert (thanks to #2975 and #6039), almost 100% of world spawns now look like `world.spawn().insert((Some, Tuple, Here))`. Spawning an entity without any components is an extremely uncommon pattern, so it makes sense to give spawn the "first class" ergonomic api. This consolidated api should be made consistent across all spawn apis (such as World and Commands).

## Solution

All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input:

```rust
// before:
commands
  .spawn()
  .insert((A, B, C));
world
  .spawn()
  .insert((A, B, C);

// after
commands.spawn((A, B, C));
world.spawn((A, B, C));
```

All existing instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api. A new `spawn_empty` has been added, replacing the old `spawn` api.  

By allowing `world.spawn(some_bundle)` to replace `world.spawn().insert(some_bundle)`, this opened the door to removing the initial entity allocation in the "empty" archetype / table done in `spawn()` (and subsequent move to the actual archetype in `.insert(some_bundle)`).

This improves spawn performance by over 10%:
![image](https://user-images.githubusercontent.com/2694663/191627587-4ab2f949-4ccd-4231-80eb-80dd4d9ad6b9.png)

To take this measurement, I added a new `world_spawn` benchmark.

Unfortunately, optimizing `Commands::spawn` is slightly less trivial, as Commands expose the Entity id of spawned entities prior to actually spawning. Doing the optimization would (naively) require assurances that the `spawn(some_bundle)` command is applied before all other commands involving the entity (which would not necessarily be true, if memory serves). Optimizing `Commands::spawn` this way does feel possible, but it will require careful thought (and maybe some additional checks), which deserves its own PR. For now, it has the same performance characteristics of the current `Commands::spawn_bundle` on main.

**Note that 99% of this PR is simple renames and refactors. The only code that needs careful scrutiny is the new `World::spawn()` impl, which is relatively straightforward, but it has some new unsafe code (which re-uses battle tested BundlerSpawner code path).** 

---

## Changelog

- All `spawn` apis (`World::spawn`, `Commands:;spawn`, `ChildBuilder::spawn`, and `WorldChildBuilder::spawn`) now accept a bundle as input
- All instances of `spawn_bundle` have been deprecated in favor of the new `spawn` api
- World and Commands now have `spawn_empty()`, which is equivalent to the old `spawn()` behavior.  

## Migration Guide

```rust
// Old (0.8):
commands
  .spawn()
  .insert_bundle((A, B, C));
// New (0.9)
commands.spawn((A, B, C));

// Old (0.8):
commands.spawn_bundle((A, B, C));
// New (0.9)
commands.spawn((A, B, C));

// Old (0.8):
let entity = commands.spawn().id();
// New (0.9)
let entity = commands.spawn_empty().id();

// Old (0.8)
let entity = world.spawn().id();
// New (0.9)
let entity = world.spawn_empty();
```
2022-09-23 19:55:54 +00:00
Carter Anderson
cd15f0f5be Accept Bundles for insert and remove. Deprecate insert/remove_bundle (#6039)
# Objective

Take advantage of the "impl Bundle for Component" changes in #2975 / add the follow up changes discussed there.

## Solution

- Change `insert` and `remove` to accept a Bundle instead of a Component (for both Commands and World)
- Deprecate `insert_bundle`, `remove_bundle`, and `remove_bundle_intersection`
- Add `remove_intersection`

---

## Changelog

- Change `insert` and `remove` now accept a Bundle instead of a Component (for both Commands and World)
- `insert_bundle` and `remove_bundle` are deprecated
 

## Migration Guide

Replace `insert_bundle` with `insert`:
```rust
// Old (0.8)
commands.spawn().insert_bundle(SomeBundle::default());
// New (0.9)
commands.spawn().insert(SomeBundle::default());
```

Replace `remove_bundle` with `remove`:
```rust
// Old (0.8)
commands.entity(some_entity).remove_bundle::<SomeBundle>();
// New (0.9)
commands.entity(some_entity).remove::<SomeBundle>();
```

Replace `remove_bundle_intersection` with `remove_intersection`:
```rust
// Old (0.8)
world.entity_mut(some_entity).remove_bundle_intersection::<SomeBundle>();
// New (0.9)
world.entity_mut(some_entity).remove_intersection::<SomeBundle>();
```

Consider consolidating as many operations as possible to improve ergonomics and cut down on archetype moves:
```rust
// Old (0.8)
commands.spawn()
  .insert_bundle(SomeBundle::default())
  .insert(SomeComponent);

// New (0.9) - Option 1
commands.spawn().insert((
  SomeBundle::default(),
  SomeComponent,
))

// New (0.9) - Option 2
commands.spawn_bundle((
  SomeBundle::default(),
  SomeComponent,
))
```

## Next Steps

Consider changing `spawn` to accept a bundle and deprecate `spawn_bundle`.
2022-09-21 21:47:53 +00:00
Daniel McNab
1a2aedd165 Implement Bundle for Component. Use Bundle tuples for insertion (#2975)
@BoxyUwU this is your fault. 

Also cart didn't arrive in time to tell us not to do this.

# Objective

- Fix #2974

## Solution

- The first commit just does the actual change
- Follow up commits do steps to prove that this method works to unify as required, but this does not remove `insert_bundle`.

## Changelog

### Changed
Nested bundles now collapse automatically, and every `Component` now implements `Bundle`.
This means that you can combine bundles and components arbitrarily, for example:
```rust
// before:
.insert(A).insert_bundle(MyBBundle{..})
// after:
.insert_bundle((A, MyBBundle {..}))
```

Note that there will be a follow up PR that removes the current `insert` impl and renames `insert_bundle` to `insert`.

### Removed
The `bundle` attribute in `derive(Bundle)`.

## Migration guide

In `derive(Bundle)`, the `bundle` attribute has been removed. Nested bundles are not collapsed automatically. You should remove `#[bundle]` attributes.

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-09-20 20:17:08 +00:00
targrub
d0e294c86b Query filter types must be ReadOnlyWorldQuery (#6008)
# Objective

Fixes Issue #6005.

## Solution

Replaced WorldQuery with ReadOnlyWorldQuery on F generic in Query filters and QueryState to restrict its trait bound.

## Migration Guide

Query filter (`F`) generics are now bound by `ReadOnlyWorldQuery`, rather than `WorldQuery`. If for some reason you were requesting `Query<&A, &mut B>`, please use `Query<&A, With<B>>` instead.
2022-09-18 23:52:01 +00:00
Boxy
404b4fc0eb lifetime related cleanup in entity_ref.rs (#5611)
# Objective

EntityMut::world takes &mut self instead of &self I don't see any reason for this.
EntityRef is overly restrictive with fn world and could return &'w World

---

## Changelog

- EntityRef now implements Copy and Clone
- EntityRef::world is now fn(&self) -> &'w World instead of fn(&mut self) -> &World
- EntityMut::world is now fn(&self) -> &World instead of fn(&mut self) -> &World
2022-09-12 04:34:52 +00:00
Moulberry
927441d048 Add From<EntityMut> for EntityRef (fixes #5459) (#5461)
Add From<EntityMut> for EntityRef (fixes #5459)
2022-09-03 18:06:42 +00:00
Boxy
df31b7d762 Remove insert_resource_with_id (#5608)
# Objective

remove `insert_resource_with_id` because `insert_resource_by_id` exists and does almost exactly the same thing

blocked on #5587 because otherwise we will leak a resource when it's inserted 

## Solution

remove the function and also add a safety invariant of to `insert_resource_by_id` that the id be valid for the world.

I didn't see any discussion in #4447 about this safety invariant being left off in favor of a panic so I'm curious if there was one or if it just seemed nicer to have less safety invariants for callers to uphold 😅 

---

## Changelog

- safety invariant added to `insert_resource_by_id` requiring the id to be valid for world

## Migration Guide

- audit any calls to `insert_resource_by_id` making sure that the id is valid for the world

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-08-30 20:32:15 +00:00
Nathan Ward
00323b3048 Better error message for World::resource_scope (#5727)
# Objective

- Fixes #5365 
- The `assert!()` when the resource from `World::resource_scope` is inserted into the world is not descriptive.

## Solution

- Add more context to the assert inside of `World::resource_scope` when the `FnOnce` param inserts the resource.
2022-08-18 18:53:08 +00:00
Jakob Hellermann
5595733035 drop old value in insert_resource_by_id if exists (#5587)
# Objective

While trying out the lint `unsafe_op_in_unsafe_fn` I noticed that `insert_resource_by_id` didn't drop the old value if it already existed, and reimplemented `Column::replace` manually for no apparent reason.

## Solution

- use `Column::replace` and add a test expecting the correct drop count

---

## Changelog

- `World::insert_resource_by_id` will now correctly drop the old resource value, if one already existed
2022-08-09 18:05:43 +00:00
Nicola Papale
397b6df023 Add into_world_mut to EntityMut (#5586)
# Objective

Provide a safe API to access an `EntityMut`'s `World`.

## Solution

* Add `EntityMut::into_world_mut` for safe access to the entity's world.

---

## Changelog

* Add `EntityMut::into_world_mut` for safe access to the entity's world.
2022-08-08 22:59:18 +00:00
ira
992681b59b Make Resource trait opt-in, requiring #[derive(Resource)] V2 (#5577)
*This PR description is an edited copy of #5007, written by @alice-i-cecile.*
# Objective
Follow-up to https://github.com/bevyengine/bevy/pull/2254. The `Resource` trait currently has a blanket implementation for all types that meet its bounds.

While ergonomic, this results in several drawbacks:

* it is possible to make confusing, silent mistakes such as inserting a function pointer (Foo) rather than a value (Foo::Bar) as a resource
* it is challenging to discover if a type is intended to be used as a resource
* we cannot later add customization options (see the [RFC](https://github.com/bevyengine/rfcs/blob/main/rfcs/27-derive-component.md) for the equivalent choice for Component).
* dependencies can use the same Rust type as a resource in invisibly conflicting ways
* raw Rust types used as resources cannot preserve privacy appropriately, as anyone able to access that type can read and write to internal values
* we cannot capture a definitive list of possible resources to display to users in an editor
## Notes to reviewers
 * Review this commit-by-commit; there's effectively no back-tracking and there's a lot of churn in some of these commits.
   *ira: My commits are not as well organized :')*
 * I've relaxed the bound on Local to Send + Sync + 'static: I don't think these concerns apply there, so this can keep things simple. Storing e.g. a u32 in a Local is fine, because there's a variable name attached explaining what it does.
 * I think this is a bad place for the Resource trait to live, but I've left it in place to make reviewing easier. IMO that's best tackled with https://github.com/bevyengine/bevy/issues/4981.

## Changelog
`Resource` is no longer automatically implemented for all matching types. Instead, use the new `#[derive(Resource)]` macro.

## Migration Guide
Add `#[derive(Resource)]` to all types you are using as a resource.

If you are using a third party type as a resource, wrap it in a tuple struct to bypass orphan rules. Consider deriving `Deref` and `DerefMut` to improve ergonomics.

`ClearColor` no longer implements `Component`. Using `ClearColor` as a component in 0.8 did nothing.
Use the `ClearColorConfig` in the `Camera3d` and `Camera2d` components instead.


Co-authored-by: Alice <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: devil-ira <justthecooldude@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2022-08-08 21:36:35 +00:00
Rob Parrett
cfee0e882e Fix various typos (#5417)
## Objective

- Fix some typos

## Solution

- Fix em. 
- My favorite was `maxizimed`
2022-07-21 20:46:54 +00:00
François
0fa499a7d0 rename send_default_event to send_event_default on world (#5383)
after #5355, three methods were added on world:
* `send_event`
* `send_event_batch`
* `send_default_event`

rename `send_default_event` to `send_event_default` for better discoverability
2022-07-19 22:28:05 +00:00
Aevyrie
282f8ed0b9 Add helpers to send Events from World (#5355)
# Objective

- With access to `World`, it's not obvious how to send an event.
- This is especially useful if you are writing a `Command` that needs to send an `Event`.
- `Events` are a first-class construct in bevy, even though they are just `Resources` under the hood. Their methods should be discoverable.

## Solution

- Provide a simple helpers to send events through `Res<Events<T>>`.
---

## Changelog

> `send_event`, `send_default_event`, and `send_event_batch` methods added to `World`.
2022-07-19 20:54:03 +00:00
Obdzen
cf200f09dd Fix typo in Word::get_by_id docs (#5246)
I believe this should read `immutable` not `mutable`



Co-authored-by: Obdzen <108854527+Obdzen@users.noreply.github.com>
2022-07-07 15:25:17 +00:00
Jakob Hellermann
d38a8dfdd7 add more SAFETY comments and lint for missing ones in bevy_ecs (#4835)
# Objective

`SAFETY` comments are meant to be placed before `unsafe` blocks and should contain the reasoning of why in this case the usage of unsafe is okay. This is useful when reading the code because it makes it clear which assumptions are required for safety, and makes it easier to spot possible unsoundness holes. It also forces the code writer to think of something to write and maybe look at the safety contracts of any called unsafe methods again to double-check their correct usage.

There's a clippy lint called `undocumented_unsafe_blocks` which warns when using a block without such a comment. 

## Solution

- since clippy expects `SAFETY` instead of `SAFE`, rename those
- add `SAFETY` comments in more places
- for the last remaining 3 places, add an `#[allow()]` and `// TODO` since I wasn't comfortable enough with the code to justify their safety
- add ` #![warn(clippy::undocumented_unsafe_blocks)]` to `bevy_ecs`


### Note for reviewers

The first commit only renames `SAFETY` to `SAFE` so it doesn't need a thorough review.
cb042a416e..55cef2d6fa is the diff for all other changes.

### Safety comments where I'm not too familiar with the code

774012ece5/crates/bevy_ecs/src/entity/mod.rs (L540-L546)

774012ece5/crates/bevy_ecs/src/world/entity_ref.rs (L249-L252)

### Locations left undocumented with a `TODO` comment

5dde944a30/crates/bevy_ecs/src/schedule/executor_parallel.rs (L196-L199)

5dde944a30/crates/bevy_ecs/src/world/entity_ref.rs (L287-L289)

5dde944a30/crates/bevy_ecs/src/world/entity_ref.rs (L413-L415)

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
2022-07-04 14:44:24 +00:00
Boxy
a1a07945d6 fix some memory leaks detected by miri (#4959)
The first leak:
```rust
    #[test]
    fn blob_vec_drop_empty_capacity() {
        let item_layout = Layout:🆕:<Foo>();
        let drop = drop_ptr::<Foo>;
        let _ = unsafe { BlobVec::new(item_layout, Some(drop), 0) };
    }
```
this is because we allocate the swap scratch in blobvec regardless of what the capacity is, but we only deallocate if capacity is > 0

The second leak:
```rust
    #[test]
    fn panic_while_overwriting_component() {
        let helper = DropTestHelper::new();

        let res = panic::catch_unwind(|| {
            let mut world = World::new();
            world
                .spawn()
                .insert(helper.make_component(true, 0))
                .insert(helper.make_component(false, 1));

            println!("Done inserting! Dropping world...");
        });

        let drop_log = helper.finish(res);

        assert_eq!(
            &*drop_log,
            [
                DropLogItem::Create(0),
                DropLogItem::Create(1),
                DropLogItem::Drop(0),
            ]
        );
    }
```
this is caused by us not running the drop impl on the to-be-inserted component if the drop impl of the overwritten component panics

---

managed to figure out where the leaks were by using this 10/10 command
```
cargo --quiet test --lib -- --list | sed 's/: test$//' | MIRIFLAGS="-Zmiri-disable-isolation" xargs -n1 cargo miri test --lib -- --exact
```
which runs every test one by one rather than all at once which let miri actually tell me which test had the leak 🙃
2022-07-01 21:54:28 +00:00
harudagondi
b3fa4790b7 Add ability to inspect entity's components (#5136)
# Objective

- Provide a way to see the components of an entity.
- Fixes #1467

## Solution

- Add `World::inspect_entity`. It accepts an `Entity` and returns a vector of `&ComponentInfo` that the entity has.
- Add `EntityCommands::log_components`. It logs the component names of the entity. (info level)

---

## Changelog

### Added
- Ability to inspect components of an entity through `World::inspect_entity` or `EntityCommands::log_components`
2022-06-30 15:23:09 +00:00
Mike
510ce5e832 fix resource not found error message (#5128)
There are some outdated error messages for when a resource is not found. It references `add_resource` and `add_non_send_resource` which were renamed to `insert_resource` and `insert_non_send_resource`.
2022-06-29 02:29:51 +00:00
Garett Cooper
fa56a5cd51 Add component_id function to World and Components (#5066)
# Objective

- Simplify the process of obtaining a `ComponentId` instance corresponding to a `Component`.
- Resolves #5060.

## Solution

- Add a `component_id::<T: Component>(&self)` function to both `World` and `Components` to retrieve the `ComponentId` associated with `T` from a immutable reference.

---

## Changelog

- Added `World::component_id::<C>()` and `Components::component_id::<C>()` to retrieve a `Component`'s corresponding `ComponentId` if it exists.
2022-06-25 20:41:54 +00:00
James Liu
c988264180 Mark mutable APIs under ECS storage as pub(crate) (#5065)
# Objective
Closes #1557. Partially addresses #3362.

Cleanup the public facing API for storage types. Most of these APIs are difficult to use safely when directly interfacing with these types, and is also currently impossible to interact with in normal ECS use as there is no `World::storages_mut`. The majority of these types should be easy enough to read, and perhaps mutate the contents, but never structurally altered without the same checks in the rest of bevy_ecs code. This both cleans up the public facing types and helps use unused code detection to remove a few of the APIs we're not using internally. 

## Solution

 - Mark all APIs that take `&mut T` under `bevy_ecs::storage` as `pub(crate)` or `pub(super)`
 - Cleanup after it all.

Entire type visibility changes:

 - `BlobVec` is `pub(super)`, only storage code should be directly interacting with it.
 - `SparseArray` is now `pub(crate)` for the entire type. It's an implementation detail for `Table` and `(Component)SparseSet`.
 - `TableMoveResult` is now `pub(crate)

---

## Changelog
TODO

## Migration Guide
Dear God, I hope not.
2022-06-21 20:35:26 +00:00
Alice Cecile
dc950a4d2f Fix broken WorldCell test (#5009)
# Objective

Fixes #5008. Aliasing references is allowed under Rust if and only if they are immutable.

This logic applies to `WorldCell` as well.
2022-06-14 16:14:33 +00:00
James Liu
f2b545049c Implement FusedIterator for eligible Iterator types (#4942)
# Objective
Most of our `Iterator` impls satisfy the requirements of `std::iter::FusedIterator`, which has internal specialization that optimizes `Interator::fuse`. The std lib iterator combinators do have a few that rely on `fuse`, so this could optimize those use cases. I don't think we're using any of them in the engine itself, but beyond a light increase in compile time, it doesn't hurt to implement the trait.

## Solution
Implement the trait for all eligible iterators in first party crates. Also add a missing `ExactSizeIterator` on an iterator that could use it.
2022-06-09 03:19:31 +00:00
Jakob Hellermann
60584139de untyped APIs for components and resources (#4447)
# Objective

Even if bevy itself does not provide any builtin scripting or modding APIs, it should have the foundations for building them yourself.
For that it should be enough to have APIs that are not tied to the actual rust types with generics, but rather accept `ComponentId`s and `bevy_ptr` ptrs.

## Solution

Add the following APIs to bevy
```rust
fn EntityRef::get_by_id(ComponentId) -> Option<Ptr<'w>>;
fn EntityMut::get_by_id(ComponentId) -> Option<Ptr<'_>>;
fn EntityMut::get_mut_by_id(ComponentId) -> Option<MutUntyped<'_>>;

fn World::get_resource_by_id(ComponentId) -> Option<Ptr<'_>>;
fn World::get_resource_mut_by_id(ComponentId) -> Option<MutUntyped<'_>>;

// Safety: `value` must point to a valid value of the component
unsafe fn World::insert_resource_by_id(ComponentId, value: OwningPtr);

fn ComponentDescriptor::new_with_layout(..) -> Self;
fn World::init_component_with_descriptor(ComponentDescriptor) -> ComponentId;
```

~~This PR would definitely benefit from #3001 (lifetime'd pointers) to make sure that the lifetimes of the pointers are valid and the my-move pointer in `insert_resource_by_id` could be an `OwningPtr`, but that can be adapter later if/when #3001 is merged.~~

### Not in this PR
- inserting components on entities (this is very tied to types with bundles and the `BundleInserter`)
- an untyped version of a query (needs good API design, has a large implementation complexity, can be done in a third-party crate)

Co-authored-by: Jakob Hellermann <hellermann@sipgate.de>
2022-05-30 15:32:47 +00:00
SarthakSingh31
dbd856de71 Nightly clippy fixes (#3491)
Fixes the following nightly clippy lints:
- ~~[map_flatten](https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten)~~ (Fixed on main)
- ~~[needless_borrow](https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow)~~ (Fixed on main)
- [return_self_not_must_use](https://rust-lang.github.io/rust-clippy/master/index.html#return_self_not_must_use) (Added in 1.59.0)
- ~~[unnecessary_lazy_evaluations](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations)~~ (Fixed on main)
- [extra_unused_lifetimes](https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes) outside of macros
- [let_unit_value](https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value)
2022-05-17 04:38:03 +00:00
Jakob Hellermann
1e322d9f76 bevy_ptr standalone crate (#4653)
# Objective

The pointer types introduced in #3001 are useful not just in `bevy_ecs`, but also in crates like `bevy_reflect` (#4475) or even outside of bevy.

## Solution

Extract `Ptr<'a>`, `PtrMut<'a>`, `OwnedPtr<'a>`, `ThinSlicePtr<'a, T>` and `UnsafeCellDeref` from `bevy_ecs::ptr` into `bevy_ptr`.

**Note:** `bevy_ecs` still reexports the `bevy_ptr` as `bevy_ecs::ptr` so that crates like `bevy_transform` can use the `Bundle` derive without needing to depend on `bevy_ptr` themselves.
2022-05-04 19:16:10 +00:00
TheRawMeatball
5ca78b1e27 Add get_change_ticks method to EntityRef and EntityMut (#2539)
Direct access to the change ticks is useful for integrating the reliable change detection with external stuff.
2022-05-02 18:44:58 +00:00
TheRawMeatball
73c78c3667 Use lifetimed, type erased pointers in bevy_ecs (#3001)
# Objective

`bevy_ecs` has large amounts of unsafe code which is hard to get right and makes it difficult to audit for soundness.

## Solution

Introduce lifetimed, type-erased pointers: `Ptr<'a>` `PtrMut<'a>` `OwningPtr<'a>'` and `ThinSlicePtr<'a, T>` which are newtypes around a raw pointer with a lifetime and conceptually representing strong invariants about the pointee and validity of the pointer.

The process of converting bevy_ecs to use these has already caught multiple cases of unsound behavior.

## Changelog

TL;DR for release notes: `bevy_ecs` now uses lifetimed, type-erased pointers internally, significantly improving safety and legibility without sacrificing performance. This should have approximately no end user impact, unless you were meddling with the (unfortunately public) internals of `bevy_ecs`.

- `Fetch`, `FilterFetch` and `ReadOnlyFetch` trait no longer have a `'state` lifetime
    - this was unneeded
- `ReadOnly/Fetch` associated types on `WorldQuery` are now on a new `WorldQueryGats<'world>` trait
    - was required to work around lack of Generic Associated Types (we wish to express `type Fetch<'a>: Fetch<'a>`)
- `derive(WorldQuery)` no longer requires `'w` lifetime on struct
    - this was unneeded, and improves the end user experience
- `EntityMut::get_unchecked_mut` returns `&'_ mut T` not `&'w mut T`
    - allows easier use of unsafe API with less footguns, and can be worked around via lifetime transmutery as a user
- `Bundle::from_components` now takes a `ctx` parameter to pass to the `FnMut` closure
    - required because closure return types can't borrow from captures
- `Fetch::init` takes `&'world World`, `Fetch::set_archetype` takes `&'world Archetype` and `&'world Tables`, `Fetch::set_table` takes `&'world Table`
    - allows types implementing `Fetch` to store borrows into world
- `WorldQuery` trait now has a `shrink` fn to shorten the lifetime in `Fetch::<'a>::Item`
    - this works around lack of subtyping of assoc types, rust doesnt allow you to turn `<T as Fetch<'static>>::Item'` into `<T as Fetch<'a>>::Item'`
    - `QueryCombinationsIter` requires this
- Most types implementing `Fetch` now have a lifetime `'w`
    - allows the fetches to store borrows of world data instead of using raw pointers

## Migration guide

- `EntityMut::get_unchecked_mut` returns a more restricted lifetime, there is no general way to migrate this as it depends on your code
- `Bundle::from_components` implementations must pass the `ctx` arg to `func`
- `Bundle::from_components` callers have to use a fn arg instead of closure captures for borrowing from world
- Remove lifetime args on `derive(WorldQuery)` structs as it is nonsensical
- `<Q as WorldQuery>::ReadOnly/Fetch` should be changed to either `RO/QueryFetch<'world>` or `<Q as WorldQueryGats<'world>>::ReadOnly/Fetch`
- `<F as Fetch<'w, 's>>` should be changed to `<F as Fetch<'w>>`
- Change the fn sigs of `Fetch::init/set_archetype/set_table` to match respective trait fn sigs
- Implement the required `fn shrink` on any `WorldQuery` implementations
- Move assoc types `Fetch` and `ReadOnlyFetch` on `WorldQuery` impls to `WorldQueryGats` impls
- Pass an appropriate `'world` lifetime to whatever fetch struct you are for some reason using

### Type inference regression

in some cases rustc may give spurrious errors when attempting to infer the `F` parameter on a query/querystate this can be fixed by manually specifying the type, i.e. `QueryState:🆕:<_, ()>(world)`. The error is rather confusing:

```rust=
error[E0271]: type mismatch resolving `<() as Fetch<'_>>::Item == bool`
    --> crates/bevy_pbr/src/render/light.rs:1413:30
     |
1413 |             main_view_query: QueryState::new(world),
     |                              ^^^^^^^^^^^^^^^ expected `bool`, found `()`
     |
     = note: required because of the requirements on the impl of `for<'x> FilterFetch<'x>` for `<() as WorldQueryGats<'x>>::Fetch`
note: required by a bound in `bevy_ecs::query::QueryState::<Q, F>::new`
    --> crates/bevy_ecs/src/query/state.rs:49:32
     |
49   |     for<'x> QueryFetch<'x, F>: FilterFetch<'x>,
     |                                ^^^^^^^^^^^^^^^ required by this bound in `bevy_ecs::query::QueryState::<Q, F>::new`
```

---

Made with help from @BoxyUwU and @alice-i-cecile 

Co-authored-by: Boxy <supbscripter@gmail.com>
2022-04-27 23:44:06 +00:00
Aevyrie
4aa56050b6 Add infallible resource getters for WorldCell (#4104)
# Objective

- Eliminate all `worldcell.get_resource().unwrap()` cases.
- Provide helpful messages on panic.

## Solution

- Adds infallible resource getters to `WorldCell`, mirroring `World`.
2022-04-25 23:19:13 +00:00
Alice Cecile
4bcb310008 Basic EntityRef and EntityMut docs (#3388)
# Objective

- `EntityRef` and `EntityMut` are surpisingly important public types when working directly with the `World`.
- They're undocumented.

## Solution

- Just add docs!
2022-04-25 14:32:57 +00:00
TheRawMeatball
0fdb45ce90 Remove EntityMut::get_unchecked (#4547)
The only way to soundly use this API is already encapsulated within `EntityMut::get`, so this api is removed.

# Migration guide

Replace calls to `EntityMut::get_unchecked` with calls to `EntityMut::get`.
2022-04-22 20:06:41 +00:00
François
8630b194dc add more logs when despawning entities (#3851)
# Objective

- Provide more information when despawning an entity

## Solution

- Add a debug log when despawning an entity
- Add spans to the recursive ways of despawning an entity

```sh
RUST_LOG=debug cargo run --example panic --features trace
# RUST_LOG=debug needed to show debug logs from bevy_ecs
# --features trace needed to have the extra spans
...

DEBUG bevy_app:frame:stage{name=Update}:system_commands{name="panic::despawn_parent"}:command{name="DespawnRecursive" entity=0v0}: bevy_ecs::world: Despawning entity 1v0
DEBUG bevy_app:frame:stage{name=Update}:system_commands{name="panic::despawn_parent"}:command{name="DespawnRecursive" entity=0v0}: bevy_ecs::world: Despawning entity 0v0
```
2022-04-13 23:35:28 +00:00
Boxy
dba7790012 REMOVE unsound lifetime annotations on EntityMut (#4096)
Fixes #3408
#3001 also solves this but I dont see it getting merged any time soon so...
# Objective
make bevy ecs a lil bit less unsound

## Solution
make `EntityMut::get_component_mut` return borrows from self instead of `'w`
2022-04-04 21:33:33 +00:00
Daniel McNab
aca7fc1854 Remove outdated perf comments (#4374)
# Objective

- The perf comments, added (by me) in https://github.com/bevyengine/bevy/pull/1349, became outdated once the initialisation call started to take an exclusive reference, (presumably in https://github.com/bevyengine/bevy/pull/1525).
- They have been naïvely transferred along ever since

## Solution

- Remove them
2022-03-31 20:43:00 +00:00
Boxy
637a149910 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
2022-03-30 23:52:45 +00:00
Boxy
050d2b7f00 yeet World::components_mut >:( (#4092)
# Objective
make bevy ecs a lil bit less unsound
## Solution
yeet unsound API `World::components_mut`:
```rust
use bevy_ecs::prelude::*;

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

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

fn main() {
    let mut world = World::new();
    let e = world.spawn().insert(Foo(0)).id();
    *world.components_mut() = Default::default();
    let bar = world.entity_mut(e).remove::<Bar>().unwrap();
    // oopsies reading memory copied from outside allocation
    dbg!(bar);
}
```
2022-03-21 23:43:08 +00:00
Alice Cecile
557ab9897a Make get_resource (and friends) infallible (#4047)
# Objective

- In the large majority of cases, users were calling `.unwrap()` immediately after `.get_resource`.
- Attempting to add more helpful error messages here resulted in endless manual boilerplate (see #3899 and the linked PRs).

## Solution

- Add an infallible variant named `.resource` and so on.
- Use these infallible variants over `.get_resource().unwrap()` across the code base.

## Notes

I did not provide equivalent methods on `WorldCell`, in favor of removing it entirely in #3939.

## Migration Guide

Infallible variants of `.get_resource` have been added that implicitly panic, rather than needing to be unwrapped.

Replace `world.get_resource::<Foo>().unwrap()` with `world.resource::<Foo>()`.

## Impact

- `.unwrap` search results before: 1084
- `.unwrap` search results after: 942
- internal `unwrap_or_else` calls added: 4
- trivial unwrap calls removed from tests and code: 146
- uses of the new `try_get_resource` API: 11
- percentage of the time the unwrapping API was used internally: 93%
2022-02-27 22:37:18 +00:00
Alice Cecile
330160cf14 SystemState usage docs (#3783)
# Objective

- `SystemStates` rock for dealing with exclusive world access, but are hard to figure out how to use.
- Fixes #3341.

## Solution

- Clearly document how to use `SystemState`, and why they're useful as an end-user.
2022-02-15 21:53:52 +00:00
danieleades
d8974e7c3d small and mostly pointless refactoring (#2934)
What is says on the tin.

This has got more to do with making `clippy` slightly more *quiet* than it does with changing anything that might greatly impact readability or performance.

that said, deriving `Default` for a couple of structs is a nice easy win
2022-02-13 22:33:55 +00:00
Alice Cecile
bdbf626341 Implement init_resource for Commands and World (#3079)
# Objective

- Fixes #3078
- Fixes #1397

## Solution

- Implement Commands::init_resource.
- Also implement for World, for consistency and to simplify internal structure.
- While we're here, clean up some of the docs for Command and World resource modification.
2022-02-08 23:04:19 +00:00
TheRawMeatball
142e7f3c50 Backport soundness fix (#3685)
#3001 discovered a soundness bug in World::resource_scope, this PR backports the fix with a smaller PR to patch out the bug sooner.

Fixes #3147
2022-02-04 03:21:31 +00:00
bjorn3
af22cc1dc3 Use ManuallyDrop instead of forget in insert_resource_with_id (#2947)
# Objective

Calling forget would invalidate the data pointer before it is used.

## Solution

Use `ManuallyDrop` to prevent the value from being dropped without moving it.
2022-02-03 22:34:31 +00:00
KDecay
506642744c docs: Fix private doc links and enable CI test (#3743)
# Objective

Fixes #3566

## Solution

- [x] Fix broken links in private docs.
- [x] Add the `--document-private-items` flag to the CI.

## Note

The following was said by @killercup in #3566:

> I don't have time to confirm this but I assume that linking to private items throws an error/warning when just running cargo doc, and --document-private-item might actually hide that warning. So to test this, you'd have to run it twice.

I tested this and this is thankfully not the case. If you are linking to a private item you will get a warning no matter if you run `cargo doc` or `cargo doc --document-private-items`.

### Example

I added `struct Test;` to `bevy_core/src/name.rs` and linked to it inside of a doc comment using ``[`Test`]``. After that I ran `cargo doc -p bevy_core --document-private-items` using `RUSTDOCFLAGS="-D warnings"` and got the following output (note the last sentence):

```rust
error: public documentation for `Name` links to private item `Test`
  --> crates/bevy_core/src/name.rs:11:82
   |
11 | /// Component used to identify an entity. Stores a hash for faster comparisons [`Test`]
   |                                                                                  ^^^^ this item is private
   |
   = note: `-D rustdoc::private-intra-doc-links` implied by `-D warnings`
   = note: this link resolves only because you passed `--document-private-items`, but will break without
```
2022-02-02 21:47:29 +00:00
Alice Cecile
f5039a476d Mark .id() methods which return an Entity as must_use (#3750)
# Objective

- Calling .id() has no purpose unless you use the Entity returned
- This is an easy source of confusion for beginners.
- This is easily missed during refactors.

## Solution

- Mark the appropriate methods as #[must_use]
2022-01-23 14:24:37 +00:00
François
17bb812d5d Ignore clippy 1.58 (#3667)
- Work around #3666 until a proper fix is done
- Also update duplicate dependencies list
2022-01-14 18:21:22 +00:00
Niklas Eicker
fbab01a40d Add missing closing ticks for inline examples and some cleanup (#3573)
# Objective

- clean up documentation and inline examples

## Solution

- add missing closing "```"
- remove stray "```"
- remove whitespace in inline examples
- unify inline examples (remove some `rust` labels)
2022-01-07 09:25:12 +00:00