Commit graph

1537 commits

Author SHA1 Message Date
Nuutti Kotivuori
76d610d465
Flush commands after every mutation in WorldEntityMut (#16219)
# Objective

- Currently adding observers spawns an entity which implicitly flushes
the command queue, which can cause undefined behaviour if the
`WorldEntityMut` is used after this
- The reason `WorldEntityMut` attempted to (unsuccessfully) avoid
flushing commands until finished was that such commands may move or
despawn the entity being referenced, invalidating the cached location.
- With the introduction of hooks and observers, this isn't sensible
anymore as running the commands generated by hooks immediately is
required to maintain correct ordering of operations and to not expose
the world in an inconsistent state
- Objective is to make command flushing deterministic and fix the
related issues
- Fixes #16212
- Fixes #14621 
- Fixes #16034

## Solution

- Allow `WorldEntityMut` to exist even when it refers to a despawned
entity by allowing `EntityLocation` to be marked invalid
- Add checks to all methods to panic if trying to access a despawned
entity
- Flush command queue after every operation that might trigger hooks or
observers
- Update entity location always after flushing command queue

## Testing

- Added test cases for currently broken behaviour
- Added test cases that flushes happen in all operations
- Added test cases to ensure hooks and commands are run exactly in
correct order when nested

---

Todo:

- [x] Write migration guide
- [x] Add tests that using `EntityWorldMut` on a despawned entity panics
- [x] Add tests that commands are flushed after every operation that is
supposed to flush them
- [x] Add tests that hooks, observers and their spawned commands are run
in the correct order when nested

---

## Migration Guide

Previously `EntityWorldMut` triggered command queue flushes in
unpredictable places, which could interfere with hooks and observers.
Now the command queue is flushed always immediately after any call in
`EntityWorldMut` that spawns or despawns an entity, or adds, removes or
replaces a component. This means hooks and observers will run their
commands in the correct order.

As a side effect, there is a possibility that a hook or observer could
despawn the entity that is being referred to by `EntityWorldMut`. This
could already currently happen if an observer was added while keeping an
`EntityWorldMut` referece and would cause unsound behaviour. If the
entity has been despawned, calling any methods which require the entity
location will panic. This matches the behaviour that `Commands` will
panic if called on an already despawned entity. In the extremely rare
case where taking a new `EntityWorldMut` reference or otherwise
restructuring the code so that this case does not happen is not
possible, there's a new `is_despawned` method that can be used to check
if the referred entity has been despawned.
2024-12-05 20:30:12 +00:00
Christian Hughes
f87b9fe20c
Turn apply_deferred into a ZST System (#16642)
# Objective

- Required by #16622 due to differing implementations of `System` by
`FunctionSystem` and `ExclusiveFunctionSystem`.
- Optimize the memory usage of instances of `apply_deferred` in system
schedules.

## Solution

By changing `apply_deferred` from being an ordinary system that ends up
as an `ExclusiveFunctionSystem`, and instead into a ZST struct that
implements `System` manually, we save ~320 bytes per instance of
`apply_deferred` in any schedule.

## Testing

- All current tests pass.

---

## Migration Guide

- If you were previously calling the special `apply_deferred` system via
`apply_deferred(world)`, don't.
2024-12-05 18:14:05 +00:00
vil'mo
67bd2b00e1
Expose SystemMeta's access field as part of public API (#16625)
# Objective

Outside of the `bevy_ecs` crate it's hard to implement `SystemParam`
trait on params that require access to the `World`, because `init_state`
expects user to extend access in `SystemMeta` and access-related fields
of `SystemMeta` are private.

## Solution

Expose those fields as a functions
2024-12-05 18:10:58 +00:00
Zachary Harrold
a35811d088
Add Immutable Component Support (#16372)
# Objective

- Fixes #16208

## Solution

- Added an associated type to `Component`, `Mutability`, which flags
whether a component is mutable, or immutable. If `Mutability= Mutable`,
the component is mutable. If `Mutability= Immutable`, the component is
immutable.
- Updated `derive_component` to default to mutable unless an
`#[component(immutable)]` attribute is added.
- Updated `ReflectComponent` to check if a component is mutable and, if
not, panic when attempting to mutate.

## Testing

- CI
- `immutable_components` example.

---

## Showcase

Users can now mark a component as `#[component(immutable)]` to prevent
safe mutation of a component while it is attached to an entity:

```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
    // ...
}
```

This prevents creating an exclusive reference to the component while it
is attached to an entity. This is particularly powerful when combined
with component hooks, as you can now fully track a component's value,
ensuring whatever invariants you desire are upheld. Before this would be
done my making a component private, and manually creating a `QueryData`
implementation which only permitted read access.

<details>
  <summary>Using immutable components as an index</summary>
  
```rust
/// This is an example of a component like [`Name`](bevy::prelude::Name), but immutable.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Component)]
#[component(
    immutable,
    on_insert = on_insert_name,
    on_replace = on_replace_name,
)]
pub struct Name(pub &'static str);

/// This index allows for O(1) lookups of an [`Entity`] by its [`Name`].
#[derive(Resource, Default)]
struct NameIndex {
    name_to_entity: HashMap<Name, Entity>,
}

impl NameIndex {
    fn get_entity(&self, name: &'static str) -> Option<Entity> {
        self.name_to_entity.get(&Name(name)).copied()
    }
}

fn on_insert_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
    let Some(&name) = world.entity(entity).get::<Name>() else {
        unreachable!()
    };
    let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
        return;
    };

    index.name_to_entity.insert(name, entity);
}

fn on_replace_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
    let Some(&name) = world.entity(entity).get::<Name>() else {
        unreachable!()
    };
    let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
        return;
    };

    index.name_to_entity.remove(&name);
}

// Setup our name index
world.init_resource::<NameIndex>();

// Spawn some entities!
let alyssa = world.spawn(Name("Alyssa")).id();
let javier = world.spawn(Name("Javier")).id();

// Check our index
let index = world.resource::<NameIndex>();

assert_eq!(index.get_entity("Alyssa"), Some(alyssa));
assert_eq!(index.get_entity("Javier"), Some(javier));

// Changing the name of an entity is also fully capture by our index
world.entity_mut(javier).insert(Name("Steven"));

// Javier changed their name to Steven
let steven = javier;

// Check our index
let index = world.resource::<NameIndex>();

assert_eq!(index.get_entity("Javier"), None);
assert_eq!(index.get_entity("Steven"), Some(steven));
```
  
</details>

Additionally, users can use `Component<Mutability = ...>` in trait
bounds to enforce that a component _is_ mutable or _is_ immutable. When
using `Component` as a trait bound without specifying `Mutability`, any
component is applicable. However, methods which only work on mutable or
immutable components are unavailable, since the compiler must be
pessimistic about the type.

## Migration Guide

- When implementing `Component` manually, you must now provide a type
for `Mutability`. The type `Mutable` provides equivalent behaviour to
earlier versions of `Component`:
```rust
impl Component for Foo {
    type Mutability = Mutable;
    // ...
}
```
- When working with generic components, you may need to specify that
your generic parameter implements `Component<Mutability = Mutable>`
rather than `Component` if you require mutable access to said component.
- The entity entry API has had to have some changes made to minimise
friction when working with immutable components. Methods which
previously returned a `Mut<T>` will now typically return an
`OccupiedEntry<T>` instead, requiring you to add an `into_mut()` to get
the `Mut<T>` item again.

## Draft Release Notes

Components can now be made immutable while stored within the ECS.

Components are the fundamental unit of data within an ECS, and Bevy
provides a number of ways to work with them that align with Rust's rules
around ownership and borrowing. One part of this is hooks, which allow
for defining custom behavior at key points in a component's lifecycle,
such as addition and removal. However, there is currently no way to
respond to _mutation_ of a component using hooks. The reasons for this
are quite technical, but to summarize, their addition poses a
significant challenge to Bevy's core promises around performance.
Without mutation hooks, it's relatively trivial to modify a component in
such a way that breaks invariants it intends to uphold. For example, you
can use `core::mem::swap` to swap the components of two entities,
bypassing the insertion and removal hooks.

This means the only way to react to this modification is via change
detection in a system, which then begs the question of what happens
_between_ that alteration and the next run of that system?
Alternatively, you could make your component private to prevent
mutation, but now you need to provide commands and a custom `QueryData`
implementation to allow users to interact with your component at all.

Immutable components solve this problem by preventing the creation of an
exclusive reference to the component entirely. Without an exclusive
reference, the only way to modify an immutable component is via removal
or replacement, which is fully captured by component hooks. To make a
component immutable, simply add `#[component(immutable)]`:

```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
    // ...
}
```

When implementing `Component` manually, there is an associated type
`Mutability` which controls this behavior:

```rust
impl Component for Foo {
    type Mutability = Mutable;
    // ...
}
```

Note that this means when working with generic components, you may need
to specify that a component is mutable to gain access to certain
methods:

```rust
// Before
fn bar<C: Component>() {
    // ...
}

// After
fn bar<C: Component<Mutability = Mutable>>() {
    // ...
}
```

With this new tool, creating index components, or caching data on an
entity should be more user friendly, allowing libraries to provide APIs
relying on components and hooks to uphold their invariants.

## Notes

- ~~I've done my best to implement this feature, but I'm not happy with
how reflection has turned out. If any reflection SMEs know a way to
improve this situation I'd greatly appreciate it.~~ There is an
outstanding issue around the fallibility of mutable methods on
`ReflectComponent`, but the DX is largely unchanged from `main` now.
- I've attempted to prevent all safe mutable access to a component that
does not implement `Component<Mutability = Mutable>`, but there may
still be some methods I have missed. Please indicate so and I will
address them, as they are bugs.
- Unsafe is an escape hatch I am _not_ attempting to prevent. Whatever
you do with unsafe is between you and your compiler.
- I am marking this PR as ready, but I suspect it will undergo fairly
major revisions based on SME feedback.
- I've marked this PR as _Uncontroversial_ based on the feature, not the
implementation.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Nuutti Kotivuori <naked@iki.fi>
2024-12-05 14:27:48 +00:00
Zachary Harrold
c9fa975977
Remove petgraph from bevy_ecs (#15519)
# Objective

- Contributes to #15460

## Solution

- Removed `petgraph` as a dependency from the `bevy_ecs` crate.
- Replaced `TarjanScc` and `GraphMap` with specialised in-tree
alternatives.

## Testing

- Ran CI locally.
- Added new unit tests to check ordering invariants.
- Confirmed `petgraph` is no longer present in `cargo tree -p bevy_ecs`

## Migration Guide

The `Dag::graph` method no longer returns a `petgraph` `DiGraph` and
instead returns the new `DiGraph` type within `bevy_ecs`. Edge and node
iteration methods are provided so conversion to the `petgraph` type
should be trivial if required.

## Notes

- `indexmap` was already in the dependency graph for `bevy_ecs`, so its
inclusion here makes no difference to compilation time for Bevy.
- The implementation for `Graph` is heavily inspired from the `petgraph`
original, with specialisations added to simplify and improve the type.
- `petgraph` does have public plans for `no_std` support, however there
is no timeframe on if or when that functionality will be available.
Moving to an in-house solution in the interim allows Bevy to continue
developing its `no_std` offerings and further explore alternate graphing
options.

---------

Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: vero <11307157+atlv24@users.noreply.github.com>
2024-12-03 20:01:55 +00:00
SpecificProtagonist
410f3c478a
Use disqualified for B0001 (#16623)
# Objective

Fix #16553
2024-12-03 19:51:50 +00:00
SpecificProtagonist
d92fc1e456
Move required components doc to type doc (#16575)
# Objective

Make documentation of a component's required components more visible by
moving it to the type's docs

## Solution

Change `#[require]` from a derive macro helper to an attribute macro.

Disadvantages:
- this silences any unused code warnings on the component, as it is used
by the macro!
- need to import `require` if not using the ecs prelude (I have not
included this in the migration guilde as Rust tooling already suggests
the fix)

---

## Showcase
![Documentation of
Camera](https://github.com/user-attachments/assets/3329511b-747a-4c8d-a43e-57f7c9c71a3c)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
2024-12-03 19:45:20 +00:00
SpecificProtagonist
1a6b94c5e8
Remove flush_and_reserve_invalid_assuming_no_entities (#16460)
# Objective

`flush_and_reserve_invalid_assuming_no_entities` was made for the old
rendering world (which was reset every frame) and is usused since the
0.15 retained rendering world, but wasn't removed yet. It is pub, but is
undocumented apart from the safety comment.

## Solution

Remove `flush_and_reserve_invalid_assuming_no_entities` and the safety
invariants this method required for `EntityMeta`, `EntityLocation`,
`TableId` and `TableRow`. This reduces the amount of unsafe code &
safety invariants and makes #16047 easier.

## Alternatives
- Document `flush_and_reserve_invalid_assuming_no_entities` and keep it
unchanged
- Document `flush_and_reserve_invalid_assuming_no_entities` and change
it to be based on `EntityMeta::INVALID`


## Migration Guide
- exchange `Entities::flush_and_reserve_invalid_assuming_no_entities`
for `reserve` and `flush_as_invalid` and notify us if that's
insufficient

---------

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
2024-12-03 19:42:22 +00:00
Benjamin Brienen
afd0f1322d
Move all_tuples to a new crate (#16161)
# Objective

Fixes #15941

## Solution

Created https://crates.io/crates/variadics_please and moved the code
there; updating references

`bevy_utils/macros` is deleted.

## Testing

cargo check

## Migration Guide

Use `variadics_please::{all_tuples, all_tuples_with_size}` instead of
`bevy::utils::{all_tuples, all_tuples_with_size}`.
2024-12-03 17:41:09 +00:00
eugineerd
2e267bba5a
Entity cloning (#16132)
## Objective

Fixes #1515 

This PR implements a flexible entity cloning system. The primary use
case for it is to clone dynamically-generated entities.

Example:
```rs
#[derive(Component, Clone)]
pub struct Projectile;

#[derive(Component, Clone)]
pub struct Damage {
    value: f32,
}

fn player_input(
    mut commands: Commands,
    projectiles: Query<Entity, With<Projectile>>,
    input: Res<ButtonInput<KeyCode>>,
) {
    // Fire a projectile
    if input.just_pressed(KeyCode::KeyF) {
        commands.spawn((Projectile, Damage { value: 10.0 }));
    }

    // Triplicate all active projectiles
    if input.just_pressed(KeyCode::KeyT) {
        for projectile in projectiles.iter() {
            // To triplicate a projectile we need to create 2 more clones
            for _ in 0..2{
                commands.clone_entity(projectile)
            }
        }
    }
}
```

## Solution

### Commands
Add a `clone_entity` command to create a clone of an entity with all
components that can be cloned. Components that can't be cloned will be
ignored.
```rs
commands.clone_entity(entity)
```
If there is a need to configure the cloning process (like set to clone
recursively), there is a second command:
```rs
commands.clone_entity_with(entity, |builder| {
    builder.recursive(true)
});
```
Both of these commands return `EntityCommands` of the cloned entity, so
the copy can be modified afterwards.

### Builder
All these commands use `EntityCloneBuilder` internally. If there is a
need to clone an entity using `World` instead, it is also possible:
```rs
let entity = world.spawn(Component).id();
let entity_clone = world.spawn_empty().id();
EntityCloneBuilder::new(&mut world).clone_entity(entity, entity_clone);
```

Builder has methods to `allow` or `deny` certain components during
cloning if required and can be extended by implementing traits on it.
This PR includes two `EntityCloneBuilder` extensions:
`CloneEntityWithObserversExt` to configure adding cloned entity to
observers of the original entity, and `CloneEntityRecursiveExt` to
configure cloning an entity recursively.

### Clone implementations
By default, all components that implement either `Clone` or `Reflect`
will be cloned (with `Clone`-based implementation preferred in case
component implements both).

This can be overriden on a per-component basis:
```rs
impl Component for SomeComponent {
    const STORAGE_TYPE: StorageType = StorageType::Table;

    fn get_component_clone_handler() -> ComponentCloneHandler {
        // Don't clone this component
        ComponentCloneHandler::Ignore
    }
}
```

### `ComponentCloneHandlers`
Clone implementation specified in `get_component_clone_handler` will get
registered in `ComponentCloneHandlers` (stored in
`bevy_ecs::component::Components`) at component registration time.

The clone handler implementation provided by a component can be
overriden after registration like so:
```rs
let component_id = world.components().component_id::<Component>().unwrap()
world.get_component_clone_handlers_mut()
     .set_component_handler(component_id, ComponentCloneHandler::Custom(component_clone_custom))
```
The default clone handler for all components that do not explicitly
define one (or don't derive `Component`) is
`component_clone_via_reflect` if `bevy_reflect` feature is enabled, and
`component_clone_ignore` (noop) otherwise.
Default handler can be overriden using
`ComponentCloneHandlers::set_default_handler`

### Handlers
Component clone handlers can be used to modify component cloning
behavior. The general signature for a handler that can be used in
`ComponentCloneHandler::Custom` is as follows:
```rs
pub fn component_clone_custom(
    world: &mut DeferredWorld,
    entity_cloner: &EntityCloner,
) {
    // implementation
}
```
The `EntityCloner` implementation (used internally by
`EntityCloneBuilder`) assumes that after calling this custom handler,
the `target` entity has the desired version of the component from the
`source` entity.

### Builder handler overrides
Besides component-defined and world-overriden handlers,
`EntityCloneBuilder` also has a way to override handlers locally. It is
mainly used to allow configuration methods like `recursive` and
`add_observers`.
```rs
// From observer clone handler implementation
impl CloneEntityWithObserversExt for EntityCloneBuilder<'_> {
    fn add_observers(&mut self, add_observers: bool) -> &mut Self {
        if add_observers {
            self.override_component_clone_handler::<ObservedBy>(ComponentCloneHandler::Custom(
                component_clone_observed_by,
            ))
        } else {
            self.remove_component_clone_handler_override::<ObservedBy>()
        }
    }
}
```

## Testing
Includes some basic functionality tests and doctests.

Performance-wise this feature is the same as calling `clone` followed by
`insert` for every entity component. There is also some inherent
overhead due to every component clone handler having to access component
data through `World`, but this can be reduced without breaking current
public API in a later PR.
2024-12-03 17:38:10 +00:00
Vic
aa600ae95e
implement the full set of sorts on QueryManyIter (#13443)
# Objective

~Blocked on #13417~

Motivation is the same as in #13417. If users can sort `QueryIter`, to
only makes sense to also allow them to use this functionality on
`QueryManyIter`.

## Solution

Also implement the sorts on `QueryManyIter`. 

The implementation of the sorts themselves are mostly the same as with
`QueryIter` in #13417.
They differ in that they re-use the `entity_iter` passed to the
`iter_many`, and internally call `iter_many_unchecked_manual` on the
lens `QueryState` with it.
These methods also return a different struct, `QuerySortedManyIter`,
because there is no longer a guarantee of unique entities.
`QuerySortedManyIter` implements the various `Iterator` traits for
read-only iteration, as `QueryManyIter` does + `DoubleEndedIterator`.
For mutable iteration, there is both a `fetch_next` and a
`fetch_next_back` method. However, they only become available after the
user calls `collect_inner` on `QuerySortedManyIter` first. This collects
the inner `entity_iter` (this is the sorted one, **not** the original
the user passed) to drop all query lens items to avoid aliasing.
When TAITs are available this `collect_inner` could be hidden away,
until then it is unfortunately not possible to elide this without either
regressing read-only iteration, or introducing a whole new type, mostly
being a copy of `QuerySortedIter`.

As a follow-up we could add a `entities_all_unique` method to check
whether the entity list consists of only unique entities, and then
return a `QuerySortedIter` from it (under opaque impl Trait if need be),
*allowing mutable `Iterator` trait iteration* over what was originally
an `iter_many` call.
Such a method can also be added to `QueryManyIter`, albeit needing a
separate, new return type.

## Testing

I've switched the third example/doc test under `sort` out for one that
shows the collect_inner/fetch_next_back functionality, otherwise the
examples are the same as in #13417, adjusted to use `iter_many` instead
of `iter`.

The `query-iter-many-sorts` test checks for equivalence to the
underlying sorts.
The test after shows that these sorts *do not* panic after
`fetch`/`fetch_next` calls.

## Changelog

Added `sort`, `sort_unstable`, `sort_by`, `sort_unstable_by`,
`sort_by_key`, `sort_by_cached_key` to `QueryManyIter`.
Added `QuerySortedManyIter`.
2024-12-03 17:02:37 +00:00
Zachary Harrold
c02696b609
Add Commands::run_schedule (#16537)
# Objective

- Fixes #16495

## Solution

- Added `Commands::run_schedule`, which internally calls
`World::try_run_schedule`, logging any issues.

## Testing

- Added doctest
- Ran CI

## Showcase

Instead of writing:

```rust
commands.queue(|world: &mut World| world.run_schedule(FooSchedule));
```

You can now write:

```rust
commands.run_schedule(FooSchedule);
```
2024-12-02 22:16:58 +00:00
vil'mo
2a1064ec5e
Getting QueryState from immutable World reference (#16434)
# Objective

There is currently no way of getting `QueryState` from `&World`, so it
is hard to, for example, iterate over all entities with a component,
only having `&World`.

## Solution

Add `try_new` function to `QueryState` that internally uses
`WorldQuery`'s `get_state`.

## Testing

No testing
2024-12-01 20:09:29 +00:00
Christian Hughes
6fe4b1440c
Refactor FunctionSystem to use a single Option (#16514)
# Objective

Combine the `Option<_>` state in `FunctionSystem` into a single `Option`
to provide clarity and save space.

## Solution

Simplifies `FunctionSystem`'s layout by using a single
`Option<FunctionSystemState>` for state that must be initialized before
running, and saves a byte by removing the need to store an enum tag.
Additionally, calling `System::run` on an uninitialized `System` will
now give a more descriptive message prior to verifying the `WorldId`.

## Testing

Ran CI checks locally.
2024-12-01 20:09:22 +00:00
Rob Parrett
c425c32064
Fix a couple typos (#16573)
# Objective

Fix typo reported on
[discord](https://discord.com/channels/691052431525675048/743559241461399582/1312485153284558932).

## Solution

- Search for "will can."
- Fix typo
- Fix another related typo
2024-11-30 19:28:53 +00:00
Vic
eaa7dfedea
fix QueryIter::sort_unstable_by (#16565)
# Objective

`QueryIter::sort_unstable_by` is mistakenly using `slice::sort_by`.

## Solution

Use `slice::sort_unstable_by`.
2024-11-30 15:24:35 +00:00
Alice Cecile
75c92fb47b
Clarify inheritance behavior of required components (#16546)
Co-authored by: @BenjaminBrienen

# Objective

Fixes #16494. Closes #16539, which this replaces. Suggestions alone
weren't enough, so now we have a new PR!

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-11-28 21:09:26 +00:00
Joona Aalto
da68bfe94b
Fix Single doc links (#16493)
# Objective

In the [*Similar parameters* section of
`Query`](https://dev-docs.bevyengine.org/bevy/ecs/prelude/struct.Query.html#similar-parameters),
the doc link for `Single` actually links to `Query::single`, and
`Option<Single>` just links to `Option`. They should both link to
`Single`!

The first link is broken because there is a reference-style link defined
for `single`, but not for `Single`, and rustdoc treats the link as
case-insensitive for some reason.

## Solution

Fix the links!

## Testing

I built the docs locally with `cargo doc` and tested the links.
2024-11-24 18:44:00 +00:00
andriyDev
6741e01dfa
Fix adding a subtree of required components to an existing tree replacing shallower required component constructors (#16441)
# Objective

- Fixes #16406 even more. The previous implementation did not take into
account the depth of the requiree when setting the depth relative to the
required_by component.

## Solution

- Add the depth of the requiree!

## Testing

- Added a test.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-11-22 00:52:17 +00:00
Illus1on
a54d85bb2d
Correcting misspellings (#16443)
When I browsed the source code I found suspicious misspellings

# Objective

- Correcting misspelling

# Solution
- Change `doesnn't` to `doesn't`
2024-11-21 14:42:56 +00:00
Chris Russell
4362b52a01
Add a note to the on_unimplemented message for QueryData recommending &T and &mut T. (#16449)
# Objective

A new user is likely to try `Query<Component>` instead of
`Query<&Component>`. The error message should guide them to the right
solution.

## Solution

Add a note to the on_unimplemented message for `QueryData` recommending
`&T` and `&mut T`.

The full error message now looks like: 

```
error[E0277]: `A` is not valid to request as data in a `Query`
   --> crates\bevy_ecs\src\query\world_query.rs:260:18
    |
260 | fn system(query: Query<A>) {}
    |                  ^^^^^^^^ invalid `Query` data
    |
    = help: the trait `fetch::QueryData` is not implemented for `A`
    = note: if `A` is a component type, try using `&A` or `&mut A`
    = help: the following other types implement trait `fetch::QueryData`:
              &'__w mut T
              &Archetype
              &T
              ()
              (F,)
              (F0, F1)
              (F0, F1, F2)
              (F0, F1, F2, F3)
            and 41 others
note: required by a bound in `system::query::Query`
   --> crates\bevy_ecs\src\system\query.rs:362:37
    |
362 | pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> {
    |                                     ^^^^^^^^^ required by this bound in `Query`
```
2024-11-21 03:52:10 +00:00
Carter Anderson
deda3f2522
Fix detailed_trace! (#16452)
Alternative to #16450 

# Objective

detailed_trace! in its current form does not work  (and breaks CI)

## Solution

Fix detailed_trace by checking for the feature properly, adding it to
the correct crates, and removing it from the incorrect crates
2024-11-20 22:01:33 +00:00
andriyDev
4a6b686832
Fix runtime required components not registering correctly (#16436)
# Objective

- Fixes #16406 
- Fixes an issue where registering a "deeper" required component, then a
"shallower" required component, would result in the wrong required
constructor being used for the root component.

## Solution

- Make `register_required_components` add any "parent" of a component as
`required_by` to the new "child".
- Assign the depth of the `requiree` plus 1 as the depth of a new
runtime required component.

## Testing

- Added two new tests.
2024-11-19 16:31:00 +00:00
Joona Aalto
ded5ce27ae
Fix bubbling of runtime requirements for #[require(...)] attribute (#16410)
# Objective

Fixes #16406.

Currently, the `#[require(...)]` attribute internally registers
component requirements using `register_required_components_manual`. This
is done recursively in a way where every requirement in the "inheritance
tree" is added into a flat `RequiredComponents` hash map with component
constructors and inheritance depths stored.

However, this does not consider runtime requirements: if a plugins has
already registered `C` as required by `B`, and a component `A` requires
`B` through the macro attribute, spawning an entity with `A` won't add
`C`. The `required_by` hash set for `C` doesn't have `A`, and the
`RequiredComponents` of `A` don't have `C`.

Intuitively, I would've thought that the macro attribute's requirements
were always added *before* runtime requirements, and in that case I
believe this shouldn't have been an issue. But the macro requirements
are based on `Component::register_required_components`, which in a lot
of cases (I think) is only called *after* the first time a bundle with
the component is inserted. So if a runtime requirement is defined
*before* this (as is often the case, during `Plugin::build`), the macro
may not take it into account.

## Solution

Register requirements inherited from the `required` component in
`register_required_components_manual_unchecked`.

## Testing

I added a test, essentially the same as in #16406, and it now passes. I
also ran some of the tests in #16409, and they seem to work as expected.
All the existing tests for required components pass.
2024-11-17 13:51:39 +00:00
Christian Hughes
d3e9ecbb8c
Add missing exports in bevy_ecs (#16415)
# Objective

Seemed to have missed the export of `DynamicComponentFetch` from #15593.
`TryFromFilteredError` which is returned by `impl
TryFrom<FiliteredEntityMut/Ref> for EntityRef/Mut` also seemed to have
been missing.

## Solution

Export both of them.
2024-11-17 09:47:33 +00:00
Benjamin Brienen
8dfd076982
Fix MSRVs for standalone crates (#16333)
# Objective

MSRV in the standalone crates should be accurate

## Solution

Determine the msrv of each crate and set it

## Testing

Adding better msrv checks to the CI is a next-step.
2024-11-17 09:38:13 +00:00
Volodymyr Enhelhardt
db1915a1f0
Use the fully qualified name for Component in the require attribute (#16378)
# Objective

- Describe the objective or issue this PR addresses.
Use the fully qualified name for `Component` in the `require` attribute

- If you're fixing a specific issue, say "Fixes #X".
Fixes #16377

## Solution

- Describe the solution used to achieve the objective above.
Use the fully qualified name for `Component` in the `require` attribute,
i.e.,`<#ident as #bevy_ecs_path::component::Component>`

## Testing

- Did you test these changes? If so, how?
`cargo run -p ci -- lints`
`cargo run -p ci -- compile`
`cargo run -p ci -- test`
- Are there any parts that need more testing?
no
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
try to compile
```rust
#[derive(::bevy::ecs::component::Component, Default)]
pub struct A;

#[derive(::bevy::ecs::component::Component)]
#[require(A)]
pub struct B;
```
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Mac only
---


</details>

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

Co-authored-by: Volodymyr Enhelhardt <volodymyr.enhelhardt@ambr.net>
2024-11-13 20:37:50 +00:00
Rob Grindeland
a8c610a52d
Add unregister_system command (#16340)
# Objective

Fixes #16266 

## Solution

Added an `UnregisterSystem` command struct and
`Commands::unregister_system`. Also renamed `World::remove_system` and
`World::remove_system_cached` to `World::unregister_*`

## Testing

It's a fairly simple change, but I tested locally to ensure it actually
works.

---------

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
2024-11-12 22:49:29 +00:00
François Mockers
4225848b0a
undeprecate component_reads_and_writes (#16357)
# Objective

- Does not correct #16339 but takes it out of the 0.15 milestone

## Solution

- Make it future us problem instead of solving it now
2024-11-11 23:28:16 +00:00
Sou1gh0st
b83c0e106e
Add EntityMut::get_mut_by_id_unchecked (#16210)
# Objective

- Fixes: #15603 

## Solution

- Add an unsafe `get_mut_by_id_unchecked` to `EntityMut` that borrows
&self instead of &mut self, thereby allowing access to multiple
components simultaneously.

## Testing

- a unit test function `get_mut_by_id_unchecked` was added.

---------

Co-authored-by: Mike <mike.hsu@gmail.com>
2024-11-11 18:46:47 +00:00
Zachary Harrold
d143da338a
Fixed issue with derive_more Display Implementations (#16298)
# Objective

- Fixed issue where `thiserror` `#[error(...)]` attributes were
improperly converted to `derive_more` `#[display(...)]` equivalents in
certain cases with a tuple struct/enum variant.

## Solution

- Used `re/#\[display\(.*\{[0-9]+\}.*\)\]/` to find occurences of using
`{0}` where `{_0}` was intended (checked for other field indexes too)and
updated accordingly.

## Testing

- `cargo check`
- CI

## Notes

This was discovered by @dtolnay in [this
comment](https://github.com/bevyengine/bevy/pull/15772#discussion_r1833730555).
2024-11-08 20:29:52 +00:00
Hennadii Chernyshchyk
e53aaddf96
Make ComponentTicks field public (#16269)
# Objective

After #12929 we no longer have methods to get component or ticks for
previously obtained table column.
It's possible to use a lower level API by indexing the slice, but then
it won't be possible to construct `ComponentTicks`.

## Solution

Make `ComponentTicks` fields public. They don't hold any invariants and
you can't get a mutable reference to the struct in Bevy.

I also removed the getters since they are no longer needed.

## Testing

- I tested the compilation

---

## Migration Guide

- Instead of using `ComponentTicks::last_changed_tick` and
`ComponentTicks::added_tick` methods, access fields directly.
2024-11-06 22:21:04 +00:00
Chris Russell
a967c75e92
Enable EntityRef tests that now pass. (#16263)
# Objective

Re-enable some tests in `entity_ref.rs` that are marked as `#[ignore]`,
but that pass after #14561.

## Solution

Remove `#[ignore]` from those tests.
2024-11-06 16:10:55 +00:00
Derick M
8d24efe60c
Remove unused debug identifiers for ComponentHooks methods (#16228)
# Objective

- Cleans up unused debug identifiers for `ComponentHooks` methods:
`on_add`, `on_insert`, `on_replace`, and `on_remove`
 
## Solution

- Simplify the expect messages by removing the unused `{:?}`

## Testing

- Currently untested


[Context](https://discord.com/channels/691052431525675048/749335865876021248/1302988180592525362)
2024-11-04 15:41:07 +00:00
urben1680
1e47604506
Adding ScheduleGraph::contains_set (#16206)
# Objective

The schedule graph can easily confirm whether a set is contained or not.

This helps me in my personal project where I write an extension trait
for `Schedule` and I want to configure a specific set in its methods.
The set in question has a run condition though and I don't want to add
that condition to the same schedule as many times as the trait methods
are called. Since the non-pub set is unknown to the schedule until then,
a `contains_set` is sufficient.

It is probably trivial to add a method that returns an `Option<NodeId>`
as well but as I personally don't need it I did not add that. If it is
desired I can do so here though. It might be unneeded to have a
`contains_set` then because one could check `is_some` on the returned id
in that case.

An argument against that is that future changes may be easier if only a
`contains_set` needs to be ported.

## Solution

Added `ScheduleGraph::contains_set`.

## Testing

I put the below showcase code into a temporary unit test and it worked.
If wanted I add it as a test too but I did not see that other more
somewhat complicated methods have tests

---

## Showcase

```rs
#[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
struct MySchedule;

#[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
struct MySet;

let mut schedule = Schedule::new(MySchedule);
assert_eq!(schedule.graph().contains_set(MySet), false);
schedule.configure_sets(MySet);
assert_eq!(schedule.graph().contains_set(MySet), true);
```
2024-11-03 16:16:24 +00:00
MiniaczQ
5edc23db41
Fix fallible param notes (#16218)
I noticed one of the reflinks doesn't work correctly
2024-11-03 16:14:26 +00:00
Benjamin Brienen
f005a96dd4
ReflectBundle::remove improvement (#16139)
# Objective

Fixes #15676

## Solution

`remove` returns the removed item

Add `take`

## Testing

None yet

## Migration Guide

If you don't need the returned value from `remove`, discard it.
2024-10-28 22:29:05 +00:00
Tau Gärtli
a644ac73f7
More #[doc(fake_variadic)] goodness (#16108)
This PR adds `#[doc(fake_variadic)]` to that were previously not
supported by rustdoc.

Thanks to an [upstream
contribution](https://github.com/rust-lang/rust/pull/132115) by yours
truly, `#[doc(fake_variadic)]` is now supported on impls such as `impl
QueryData for AnyOf<(T, ...)>` 🎉
Requires the latest nightly compiler (2024-10-25) which is already
available on [docs.rs](https://docs.rs/about/builds).


![image](https://github.com/user-attachments/assets/68589c7e-f68f-44fb-9a7b-09d24ccf19c9)

![image](https://github.com/user-attachments/assets/f09d20d6-d89b-471b-9a81-4a72c8968178)

This means that the impl sections for `QueryData` and `QueryFilter` are
now nice and tidy 

---

I also added `fake_variadic` to some impls that use
`all_tuples_with_size`, however I'm not entirely happy because the docs
are slightly misleading now:


![image](https://github.com/user-attachments/assets/fac93d08-dc02-430f-9f34-c97456256c56)

Note that the docs say `IntoBindGroupLayoutEntryBuilderArray<1>` instead
of
`IntoBindGroupLayoutEntryBuilderArray<N>`.
2024-10-27 19:01:50 +00:00
Rob Parrett
30d84519a2
Use en-us locale for typos (#16037)
# Objective

Bevy seems to want to standardize on "American English" spellings. Not
sure if this is laid out anywhere in writing, but see also #15947.

While perusing the docs for `typos`, I noticed that it has a `locale`
config option and tried it out.

## Solution

Switch to `en-us` locale in the `typos` config and run `typos -w`

## Migration Guide

The following methods or fields have been renamed from `*dependants*` to
`*dependents*`.

- `ProcessorAssetInfo::dependants`
- `ProcessorAssetInfos::add_dependant`
- `ProcessorAssetInfos::non_existent_dependants`
- `AssetInfo::dependants_waiting_on_load`
- `AssetInfo::dependants_waiting_on_recursive_dep_load`
- `AssetInfos::loader_dependants`
- `AssetInfos::remove_dependants_and_labels`
2024-10-20 18:55:17 +00:00
SpecificProtagonist
3eec0f0a77
QueryEntityError: Use short name for components (#16032)
Use the new `disqualified` crate in `QueryEntityError` to make the error
message more readable.

---

## Showcase

Old:
QueryDoesNotMatch(0v1 with components my_game::main::foo::A,
my_game::main::foo::B, bevy_pbr::light::point_light::PointLight,
bevy_render::primitives::CubemapFrusta,
bevy_pbr::bundle::CubemapVisibleEntities,
bevy_transform::components::transform::Transform,
bevy_transform::components::global_transform::GlobalTransform,
bevy_render::view::visibility::Visibility,
bevy_render::view::visibility::InheritedVisibility,
bevy_render::view::visibility::ViewVisibility,
bevy_render::sync_world::SyncToRenderWorld)

New:
QueryDoesNotMatch(0v1 with components A, B, PointLight, CubemapFrusta,
CubemapVisibleEntities, Transform, GlobalTransform, Visibility,
InheritedVisibility, ViewVisibility, SyncToRenderWorld)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-20 17:56:43 +00:00
Peter Hayman
75096fbf97
fix: add reflect to SceneInstanceReady and other observers/events (#16018)
# Objective

Built-in observers & events should be `Reflect` so that components that
interact with them can be serialized in scenes. This is a similar pr to
#14259.
2024-10-20 13:51:41 +00:00
Rob Parrett
da5d2fccf5
Fix some duplicate words in docs/comments (#15980)
# Objective

Stumbled upon one of these, and set off in search of more, armed with my
trusty `\b(\w+)\s+\1\b`.

## Solution

Remove ~one~ one of them.
2024-10-20 01:03:27 +00:00
Carter Anderson
015f2c69ca
Merge Style properties into Node. Use ComputedNode for computed properties. (#15975)
# Objective

Continue improving the user experience of our UI Node API in the
direction specified by [Bevy's Next Generation Scene / UI
System](https://github.com/bevyengine/bevy/discussions/14437)

## Solution

As specified in the document above, merge `Style` fields into `Node`,
and move "computed Node fields" into `ComputedNode` (I chose this name
over something like `ComputedNodeLayout` because it currently contains
more than just layout info. If we want to break this up / rename these
concepts, lets do that in a separate PR). `Style` has been removed.

This accomplishes a number of goals:

## Ergonomics wins

Specifying both `Node` and `Style` is now no longer required for
non-default styles

Before:
```rust
commands.spawn((
    Node::default(),
    Style {
        width:  Val::Px(100.),
        ..default()
    },
));
```

After:

```rust
commands.spawn(Node {
    width:  Val::Px(100.),
    ..default()
});
```

## Conceptual clarity

`Style` was never a comprehensive "style sheet". It only defined "core"
style properties that all `Nodes` shared. Any "styled property" that
couldn't fit that mold had to be in a separate component. A "real" style
system would style properties _across_ components (`Node`, `Button`,
etc). We have plans to build a true style system (see the doc linked
above).

By moving the `Style` fields to `Node`, we fully embrace `Node` as the
driving concept and remove the "style system" confusion.

## Next Steps

* Consider identifying and splitting out "style properties that aren't
core to Node". This should not happen for Bevy 0.15.

---

## Migration Guide

Move any fields set on `Style` into `Node` and replace all `Style`
component usage with `Node`.

Before:
```rust
commands.spawn((
    Node::default(),
    Style {
        width:  Val::Px(100.),
        ..default()
    },
));
```

After:

```rust
commands.spawn(Node {
    width:  Val::Px(100.),
    ..default()
});
```

For any usage of the "computed node properties" that used to live on
`Node`, use `ComputedNode` instead:

Before:
```rust
fn system(nodes: Query<&Node>) {
    for node in &nodes {
        let computed_size = node.size();
    }
}
```

After:
```rust
fn system(computed_nodes: Query<&ComputedNode>) {
    for computed_node in &computed_nodes {
        let computed_size = computed_node.size();
    }
}
```
2024-10-18 22:25:33 +00:00
Christian Hughes
345f935b1a
Add Trigger::components, which lists the component targets that were triggered (#15811)
# Objective

- Closes #14774 

## Solution

Added:

```rust
impl<'w, E, B: Bundle> Trigger<'w, E, B> {
    pub fn components(&self) -> &[ComponentId];
}
```

I went with storing it in the trigger as a `SmallVec<[Component; 1]>`
because a singular target component will be the most common case, and it
remains the same size as `Vec<ComponentId>`.

## Testing

Added a test.
2024-10-15 02:17:03 +00:00
Clar Fon
e79bc7811d
Fix *most* clippy lints (#15906)
# Objective

Another clippy-lint fix: the goal is so that `ci lints` actually
displays the problems that a contributor caused, and not a bunch of
existing stuff in the repo. (when run on nightly)

## Solution

This fixes all but the `clippy::needless_lifetimes` lint, which will
result in substantially more fixes and be in other PR(s). I also
explicitly allow `non_local_definitions` since it is [not working
correctly, but will be
fixed](https://github.com/rust-lang/rust/issues/131643).

A few things were manually fixed: for example, some places had an
explicitly defined `div_ceil` function that was used, which is no longer
needed since this function is stable on unsigned integers. Also, empty
lines in doc comments were handled individually.

## Testing

I ran `cargo clippy --workspace --all-targets --all-features --fix
--allow-staged` with the `clippy::needless_lifetimes` lint marked as
`allow` in `Cargo.toml` to avoid fixing that too. It now passes with all
but the listed lint.
2024-10-14 20:52:35 +00:00
Alice Cecile
a7e9330af9
Implement WorldQuery for MainWorld and RenderWorld components (#15745)
# Objective

#15320 is a particularly painful breaking change, and the new
`RenderEntity` in particular is very noisy, with a lot of `let entity =
entity.id()` spam.

## Solution

Implement `WorldQuery`, `QueryData` and `ReadOnlyQueryData` for
`RenderEntity` and `WorldEntity`.

These work the same as the `Entity` impls from a user-facing
perspective: they simply return an owned (copied) `Entity` identifier.
This dramatically reduces noise and eases migration.

Under the hood, these impls defer to the implementations for `&T` for
everything other than the "call .id() for the user" bit, as they involve
read-only access to component data. Doing it this way (as opposed to
implementing a custom fetch, as tried in the first commit) dramatically
reduces the maintenance risk of complex unsafe code outside of
`bevy_ecs`.

To make this easier (and encourage users to do this themselves!), I've
made `ReadFetch` and `WriteFetch` slightly more public: they're no
longer `doc(hidden)`. This is a good change, since trying to vendor the
logic is much worse than just deferring to the existing tested impls.

## Testing

I've run a handful of rendering examples (breakout, alien_cake_addict,
auto_exposure, fog_volumes, box_shadow) and nothing broke.

## Follow-up

We should lint for the uses of `&RenderEntity` and `&MainEntity` in
queries: this is just less nice for no reason.

---------

Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
2024-10-13 20:58:46 +00:00
Pablo Reinhardt
d96a9d15f6
Migrate from Query::single and friends to Single (#15872)
# Objective

- closes #15866

## Solution

- Simply migrate where possible.

## Testing

- Expect that CI will do most of the work. Examples is another way of
testing this, as most of the work is in that area.
---

## Notes
For now, this PR doesn't migrate `QueryState::single` and friends as for
now, this look like another issue. So for example, QueryBuilders that
used single or `World::query` that used single wasn't migrated. If there
is a easy way to migrate those, please let me know.

Most of the uses of `Query::single` were removed, the only other uses
that I found was related to tests of said methods, so will probably be
removed when we remove `Query::single`.
2024-10-13 20:32:06 +00:00
JaySpruce
3d6b24880e
Add insert_batch and variations (#15702)
# Objective

`insert_or_spawn_batch` exists, but a version for just inserting doesn't
- Closes #2693 
- Closes #8384 
- Adopts/supersedes #8600 

## Solution

Add `insert_batch`, along with the most common `insert` variations:
- `World::insert_batch`
- `World::insert_batch_if_new`
- `World::try_insert_batch`
- `World::try_insert_batch_if_new`
- `Commands::insert_batch`
- `Commands::insert_batch_if_new`
- `Commands::try_insert_batch`
- `Commands::try_insert_batch_if_new`

## Testing

Added tests, and added a benchmark for `insert_batch`.
Performance is slightly better than `insert_or_spawn_batch` when only
inserting:


![Code_HPnUN0QeWe](https://github.com/user-attachments/assets/53091e4f-6518-43f4-a63f-ae57d5470c66)

<details>
<summary>old benchmark</summary>

This was before reworking it to remove the `UnsafeWorldCell`:


![Code_QhXJb8sjlJ](https://github.com/user-attachments/assets/1061e2a7-a521-48e1-a799-1b6b8d1c0b93)
</details>

---

## Showcase

Usage is the same as `insert_or_spawn_batch`:
```
use bevy_ecs::{entity::Entity, world::World, component::Component};
#[derive(Component)]
struct A(&'static str);
#[derive(Component, PartialEq, Debug)]
struct B(f32);

let mut world = World::new();
let entity_a = world.spawn_empty().id();
let entity_b = world.spawn_empty().id();
world.insert_batch([
    (entity_a, (A("a"), B(0.0))),
    (entity_b, (A("b"), B(1.0))),
]);

assert_eq!(world.get::<B>(entity_a), Some(&B(0.0)));

```
2024-10-13 18:14:16 +00:00
Andrew
6a39c33d49
Use oslog for ios (#13364)
# Objective

On mobile devices, it's best to use the OS's native logging due to the
difficulty of accessing the console. This is already done for Android.

This is an updated version of
https://github.com/bevyengine/bevy/pull/4462.

## Solution

This PR uses Absolucy's
[tracing-oslog](https://github.com/Absolucy/tracing-oslog) ([ZLib
license](https://github.com/Absolucy/tracing-oslog/blob/main/LICENSE.md))
for iOS in order to use Apple's `os_log`.

## Testing

I ran `examples/mobile` with the logging from `examples/app/logs.rs` on
an iOS device, I then checked the logs could be filtered in the MacOS
Console.app.

## Changelog

 - Change bevy_log to use Apple's os_log on iOS.

## Questions for Reviewers
It's worth noting that the dependency this adds hasn't had bug fixes
released in a few years, so we may want to consider one or more of:
 1. a feature flag to opt-in, and it would also allow `os_log` on MacOS
 2. merge as-is and have some (minor?) upstream bugs
 3. hold off on this PR until a suitable alternative dependency arises
 4. maintain our own implementation

## Future work

In a follow-up PR it might be good to make the `subsystem` field have a
better default value, like [this
one](https://github.com/bevyengine/bevy/blob/main/examples/mobile/bevy_mobile_example.xcodeproj/project.pbxproj#L363).
That value can be retrieved programmatically if we bind another system
API (For posterity in Swift this is `Bundle.main.bundleIdentifier`, but
the C/ObjC equivalent is likely easier to bind). This would almost
always be the correct value, while the current default is unlikely to
ever be correct.

---------

Co-authored-by: Dusty DeWeese <dustin.deweese@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-10-11 08:58:14 +00:00
Gino Valente
da4e7769ad
bevy_ecs: Special-case Entity::PLACEHOLDER formatting (#15839)
# Objective

Oftentimes, users will store an entity on a component or resource. To
make this component/resource `Default`-able, they might initialize it
with `Entity::PLACEHOLDER`. This is sometimes done to avoid the need for
an `Option<Entity>`, especially if it complicates other logic.

For example, it's used in this `Selection` resource to denote "no
selection":

```rust
#[derive(Resource, Debug)]
struct Selection(Entity);

impl Default for Selection {
    fn default() -> Self {
        Self(Entity::PLACEHOLDER)
    }
}
```

The problem is that if we try to `Debug` the current `Selection`, we get
back: `4294967295v1#8589934591`. It's not immediately obvious whether or
not the entity is an actual entity or the placeholder.

Now while it doesn't take long to realize that this is in fact just the
value of `Entity::PLACEHOLDER`, it would be a lot clearer if this was
made explicit, especially for these particular use cases.

## Solution

This PR makes the `Debug` and `Display` impls for `Entity` return
`PLACEHOLDER` for the `Entity::PLACEHOLDER` constant.

~~Feel free to bikeshed the actual value returned here. I think
`PLACEHOLDER` on its own could work too.~~ Swapped to `PLACEHOLDER` from
`Entity::PLACEHOLDER`.

## Testing

You can test locally by running:

```
cargo test --package bevy_ecs
```

---

## Migration Guide

The `Debug` and `Display` impls for `Entity` now return `PLACEHOLDER`
for the `Entity::PLACEHOLDER` constant. If you had any code relying on
these values, you may need to account for this change.
2024-10-11 03:12:01 +00:00