Commit graph

1439 commits

Author SHA1 Message Date
Gino Valente
83356b12c9
bevy_reflect: Replace "value" terminology with "opaque" (#15240)
# Objective

Currently, the term "value" in the context of reflection is a bit
overloaded.

For one, it can be used synonymously with "data" or "variable". An
example sentence would be "this function takes a reflected value".

However, it is also used to refer to reflected types which are
`ReflectKind::Value`. These types are usually either primitives, opaque
types, or types that don't fall into any other `ReflectKind` (or perhaps
could, but don't due to some limitation/difficulty). An example sentence
would be "this function takes a reflected value type".

This makes it difficult to write good documentation or other learning
material without causing some amount of confusion to readers. Ideally,
we'd be able to move away from the `ReflectKind::Value` usage and come
up with a better term.

## Solution

This PR replaces the terminology of "value" with "opaque" across
`bevy_reflect`. This includes in documentation, type names, variant
names, and macros.

The term "opaque" was chosen because that's essentially how the type is
treated within the reflection API. In other words, its internal
structure is hidden. All we can do is work with the type itself.

### Primitives

While primitives are not technically opaque types, I think it's still
clearer to refer to them as "opaque" rather than keep the confusing
"value" terminology.

We could consider adding another concept for primitives (e.g.
`ReflectKind::Primitive`), but I'm not sure that provides a lot of
benefit right now. In most circumstances, they'll be treated just like
an opaque type. They would also likely use the same macro (or two copies
of the same macro but with different names).

## Testing

You can test locally by running:

```
cargo test --package bevy_reflect --all-features
```

---

## Migration Guide

The reflection concept of "value type" has been replaced with a clearer
"opaque type". The following renames have been made to account for this:

- `ReflectKind::Value` → `ReflectKind::Opaque`
- `ReflectRef::Value` → `ReflectRef::Opaque`
- `ReflectMut::Value` → `ReflectMut::Opaque`
- `ReflectOwned::Value` → `ReflectOwned::Opaque`
- `TypeInfo::Value` → `TypeInfo::Opaque`
- `ValueInfo` → `OpaqueInfo`
- `impl_reflect_value!` → `impl_reflect_opaque!`
- `impl_from_reflect_value!` → `impl_from_reflect_opaque!`

Additionally, declaring your own opaque types no longer uses
`#[reflect_value]`. This attribute has been replaced by
`#[reflect(opaque)]`:

```rust
// BEFORE
#[derive(Reflect)]
#[reflect_value(Default)]
struct MyOpaqueType(u32);

// AFTER
#[derive(Reflect)]
#[reflect(opaque)]
#[reflect(Default)]
struct MyOpaqueType(u32);
```

Note that the order in which `#[reflect(opaque)]` appears does not
matter.
2024-09-23 18:04:57 +00:00
Christian Hughes
c7ec456e50
Support systems that take references as input (#15184)
# Objective

- Fixes #14924
- Closes #9584

## Solution

- We introduce a new trait, `SystemInput`, that serves as a type
function from the `'static` form of the input, to its lifetime'd
version, similarly to `SystemParam` or `WorldQuery`.
- System functions now take the lifetime'd wrapped version,
`SystemInput::Param<'_>`, which prevents the issue presented in #14924
(i.e. `InRef<T>`).
- Functions for running systems now take the lifetime'd unwrapped
version, `SystemInput::Inner<'_>` (i.e. `&T`).
- Due to the above change, system piping had to be re-implemented as a
standalone type, rather than `CombinatorSystem` as it was previously.
- Removes the `Trigger<'static, E, B>` transmute in observer runner
code.

## Testing

- All current tests pass.
- Added additional tests and doc-tests.

---

## Showcase

```rust
let mut world = World::new();

let mut value = 2;

// Currently possible:
fn square(In(input): In<usize>) -> usize {
    input * input
}
value = world.run_system_once_with(value, square);

// Now possible:
fn square_mut(InMut(input): InMut<usize>) {
    *input *= *input;
}
world.run_system_once_with(&mut value, square_mut);

// Or:
fn square_ref(InRef(input): InRef<usize>) -> usize {
    *input * *input
}
value = world.run_system_once_with(&value, square_ref);
```

## Migration Guide

- All current explicit usages of the following types must be changed in
the way specified:
    - `SystemId<I, O>` to `SystemId<In<I>, O>`
    - `System<In = T>` to `System<In = In<T>>`
    - `IntoSystem<I, O, M>` to `IntoSystem<In<I>, O, M>`
    - `Condition<M, T>` to `Condition<M, In<T>>`
- `In<Trigger<E, B>>` is no longer a valid input parameter type. Use
`Trigger<E, B>` directly, instead.

---------

Co-authored-by: Giacomo Stevanato <giaco.stevanato@gmail.com>
2024-09-23 17:37:29 +00:00
Benjamin Brienen
8a6d0b063c
Use crate: disqualified (#15372)
# Objective

Fixes #15351 

## Solution

- Created new external crate and ported over the code

## Testing

- CI

## Migration guide

Replace references to `bevy_utils::ShortName` with
`disqualified::ShortName`.
2024-09-23 17:34:17 +00:00
Patrick Walton
8154164f1b
Allow animation clips to animate arbitrary properties. (#15282)
Currently, Bevy restricts animation clips to animating
`Transform::translation`, `Transform::rotation`, `Transform::scale`, or
`MorphWeights`, which correspond to the properties that glTF can
animate. This is insufficient for many use cases such as animating UI,
as the UI layout systems expect to have exclusive control over UI
elements' `Transform`s and therefore the `Style` properties must be
animated instead.

This commit fixes this, allowing for `AnimationClip`s to animate
arbitrary properties. The `Keyframes` structure has been turned into a
low-level trait that can be implemented to achieve arbitrary animation
behavior. Along with `Keyframes`, this patch adds a higher-level trait,
`AnimatableProperty`, that simplifies the task of animating single
interpolable properties. Built-in `Keyframes` implementations exist for
translation, rotation, scale, and morph weights. For the most part, you
can migrate by simply changing your code from
`Keyframes::Translation(...)` to `TranslationKeyframes(...)`, and
likewise for rotation, scale, and morph weights.

An example `AnimatableProperty` implementation for the font size of a
text section follows:

     #[derive(Reflect)]
     struct FontSizeProperty;

     impl AnimatableProperty for FontSizeProperty {
         type Component = Text;
         type Property = f32;
fn get_mut(component: &mut Self::Component) -> Option<&mut
Self::Property> {
             Some(&mut component.sections.get_mut(0)?.style.font_size)
         }
     }

In order to keep this patch relatively small, this patch doesn't include
an implementation of `AnimatableProperty` on top of the reflection
system. That can be a follow-up.

This patch builds on top of the new `EntityMutExcept<>` type in order to
widen the `AnimationTarget` query to include write access to all
components. Because `EntityMutExcept<>` has some performance overhead
over an explicit query, we continue to explicitly query `Transform` in
order to avoid regressing the performance of skeletal animation, such as
the `many_foxes` benchmark. I've measured the performance of that
benchmark and have found no significant regressions.

A new example, `animated_ui`, has been added. This example shows how to
use Bevy's built-in animation infrastructure to animate font size and
color, which wasn't possible before this patch.

## Showcase


https://github.com/user-attachments/assets/1fa73492-a9ce-405a-a8f2-4aacd7f6dc97

## Migration Guide

* Animation keyframes are now an extensible trait, not an enum. Replace
`Keyframes::Translation(...)`, `Keyframes::Scale(...)`,
`Keyframes::Rotation(...)`, and `Keyframes::Weights(...)` with
`Box::new(TranslationKeyframes(...))`, `Box::new(ScaleKeyframes(...))`,
`Box::new(RotationKeyframes(...))`, and
`Box::new(MorphWeightsKeyframes(...))` respectively.
2024-09-23 17:14:12 +00:00
MiniaczQ
e312da8c52
Reduce runtime panics through SystemParam validation (#15276)
# Objective

The goal of this PR is to introduce `SystemParam` validation in order to
reduce runtime panics.

Fixes #15265

## Solution

`SystemParam` now has a new method `validate_param(...) -> bool`, which
takes immutable variants of `get_param` arguments. The returned value
indicates whether the parameter can be acquired from the world. If
parameters cannot be acquired for a system, it won't be executed,
similarly to run conditions. This reduces panics when using params like
`Res`, `ResMut`, etc. as well as allows for new, ergonomic params like
#15264 or #15302.

Param validation happens at the level of executors. All validation
happens directly before executing a system, in case of normal systems
they are skipped, in case of conditions they return false.

Warning about system skipping is primitive and subject to change in
subsequent PRs.

## Testing

Two executor tests check that all executors:
- skip systems which have invalid parameters:
  - piped systems get skipped together,
  - dependent systems still run correctly,
- skip systems with invalid run conditions:
  - system conditions have invalid parameters,
  - system set conditions have invalid parameters.
2024-09-23 16:54:21 +00:00
Ben Frankel
f78856b3bd
Add cached run_system API (#14920)
# Objective

Working with `World` is painful due to lifetime issues and a lack of
ergonomics, so you may want to delegate to the system API. Your current
options are:

- `world.run_system_once`, which initializes the system each time it's
called (performance cost) and doesn't support `Local`. The docs
recommend users not use this method outside of diagnostic use cases like
unit tests.
- `world.run_system`, which requires you to register the system and
store the `SystemId` somewhere (made easier by implementing `FromWorld`
for a newtyped `Local`, unless you're in e.g. a custom `Command` impl).

These options work, but you're choosing between a performance cost and
an ergonomic challenge.

## Solution

Provide a cached `run_system` API that accepts an `S: IntoSystem` and
checks for a `CachedSystemId<S::System>(SystemId)` resource. If it
doesn't exist, it will register the system and save its `SystemId` in
that resource.

In other words, it hides the "save the `SystemId` in a `Local` or
`Resource`" pattern as an implementation detail.

Prior work: https://github.com/bevyengine/bevy/pull/10469.

## Testing

This approach worked in a proof-of-concept:
b34ee29531/src/util/patch/run_system_cached.rs (L35).

A new unit test was added and it passes in CI.
2024-09-23 16:35:29 +00:00
Christian Hughes
4682f33e0c
Add World::trigger_ref and World::trigger_targets_ref (#14894)
# Objective

Closes #14888.

## Solution

Add non-consuming trigger functions:

```rust
impl World {
    pub fn trigger_ref(&mut self, event: &mut impl Event);
    pub fn trigger_targets_ref(&mut self, event: &mut impl Event, targets: impl TriggerTargets);
}
```

## Testing

- Added two new tests, `observer_trigger_ref` and
`observer_trigger_targets_ref`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-09-23 16:31:44 +00:00
Benjamin Brienen
02a9ed4b0b
move ShortName to bevy_reflect (#15340)
# Objective

- Goal is to minimize bevy_utils #11478

## Solution

- Move the file short_name wholesale into bevy_reflect

## Testing

- Unit tests
- CI

## Migration Guide

- References to `bevy_utils::ShortName` should instead now be
`bevy_reflect::ShortName`.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-21 20:52:46 +00:00
poopy
66a474a9d9
change return type of World::resource_ref to Ref (#15263)
# Objective

Closes #11825

## Solution

Change return type of `get_resource_ref` and `resource_ref` from `Res`
to `Ref` and implement `From Res<T> for Ref<T>`.
2024-09-21 19:11:13 +00:00
Rich Churcher
fd329c0426
Allow to expect (adopted) (#15301)
# Objective

> Rust 1.81 released the #[expect(...)] attribute, which works like
#[allow(...)] but throws a warning if the lint isn't raised. This is
preferred to #[allow(...)] because it tells us when it can be removed.

- Adopts the parts of #15118 that are complete, and updates the branch
so it can be merged.
- There were a few conflicts, let me know if I misjudged any of 'em.

Alice's
[recommendation](https://github.com/bevyengine/bevy/issues/15059#issuecomment-2349263900)
seems well-taken, let's do this crate by crate now that @BD103 has done
the lion's share of this!

(Relates to, but doesn't yet completely finish #15059.)

Crates this _doesn't_ cover:

- bevy_input
- bevy_gilrs
- bevy_window
- bevy_winit
- bevy_state
- bevy_render
- bevy_picking
- bevy_core_pipeline
- bevy_sprite
- bevy_text
- bevy_pbr
- bevy_ui
- bevy_gltf
- bevy_gizmos
- bevy_dev_tools
- bevy_internal
- bevy_dylib

---------

Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Ben Frankel <ben.frankel7@gmail.com>
Co-authored-by: Antony <antony.m.3012@gmail.com>
2024-09-20 19:16:42 +00:00
Robert Walter
5484d2d6f8
Choose more descriptive field names for ReserveEntitiesIterator (#15168)
No hard feelings if you don't want to make this change. This is just
something I stumbled over in my very first read of the `bevy_ecs` crate.

# Objective

- the general goal here is to improve DX slightly
- make the code easier to read in general. The previous names make the
code harder to read, especially since they are so similar.

## Solution

- choose more specific names for the fields
- `index_iter` -> `freelist_indices` : "freelist" is a well established
term in the rest of the docs in this module, so we might want to reuse
it
- `index_range` -> `new_indices` : Nothing besides the doc comment
stated that these indices were actually new/fresh

## Testing

Note that the fields are private so that this is no breaking change.
They are also only used in this one module.
2024-09-20 19:13:35 +00:00
Benjamin Brienen
1b8c1c1242
simplify std::mem references (#15315)
# Objective
- Fixes #15314

## Solution

- Remove unnecessary usings and simplify references to those functions.

## Testing

CI
2024-09-19 21:28:16 +00:00
Giacomo Stevanato
106db47f69
Fix subtle/weird UB in the multi threaded executor (#15309)
# Objective

- The multithreaded executor has some weird UB related to stacked
borrows and async blocks
- See my explanation on discord
https://discord.com/channels/691052431525675048/749335865876021248/1286359267921887232
- Closes #15296 (can this be used to close PRs?)

## Solution

- Don't create a `&mut World` reference outside `async` blocks and then
capture it, but instead directly create it inside the `async` blocks.
This avoids it being captured, which has some weird requirement on its
validity.

## Testing

- Added a regression test
2024-09-19 18:15:58 +00:00
Mike
7ad27f4759
Fix memory leak in world's command queue (#15295)
# Objective

- I was running miri locally to check the UB in #15276 and it detected
an unrelated memory leak, due to the `RawCommandQueue` changes. (I
probably should have turned the leak detection off because we do
purposely leak interned string labels and I assume that's why CI didn't
detect it.)

## Solution

- The memory allocated to `RawCommandQueue` needs to be manually
dropped. This was being done for `bytes` and `cursor`, but was missed
for `panic_recovery`.

## Testing

- Ran miri locally and the related memory leaks errors when away.
2024-09-19 16:44:15 +00:00
Zachary Harrold
fcfa60844a
Remove allocation in get_short_name (#15294)
`ShortName` is lazily evaluated and does not allocate, instead providing
`Display` and `Debug` implementations which write directly to a
formatter using the original algorithm. When using `ShortName` in format
strings (`panic`, `dbg`, `format`, etc.) you can directly use the
`ShortName` type. If you require a `String`, simply call
`ShortName(...).to_string()`.

# Objective

- Remove the requirement for allocation when using `get_short_name`

## Solution

- Added new type `ShortName` which wraps a name and provides its own
`Debug` and `Display` implementations, using the original
`get_short_name` algorithm without the need for allocating.
- Removed `get_short_name`, as `ShortName(...)` is more performant and
ergonomic.
- Added `ShortName::of::<T>` method to streamline the common use-case
for name shortening.

## Testing

- CI

## Migration Guide

### For `format!`, `dbg!`, `panic!`, etc.

```rust
// Before
panic!("{} is too short!", get_short_name(name));

// After
panic!("{} is too short!", ShortName(name));
```

### Need a `String` Value

```rust
// Before
let short: String = get_short_name(name);

// After
let short: String = ShortName(name).to_string();
```

## Notes

`ShortName` lazily evaluates, and directly writes to a formatter via
`Debug` and `Display`, which removes the need to allocate a `String`
when printing a shortened type name. Because the implementation has been
moved into the `fmt` method, repeated printing of the `ShortName` type
may be less performant than converting it into a `String`. However, no
instances of this are present in Bevy, and the user can get the original
behaviour by calling `.to_string()` at no extra cost.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-09-19 15:34:03 +00:00
poopy
3a66d88c83
command based entry api with EntityCommands::entry (#15274)
# Objective

It's convenient to be able to modify a component if it exist, and insert
a default value if it doesn't. You can already do most of this with
`EntityCommands::insert_if_new`, and all of this using a custom command.
However, that does not spark joy in my opinion.

Closes #10669

## Solution

Introduce a new commands type `EntityEntryCommands`, along with a method
to access it, `EntityCommands::entry`.

`EntityEntryCommands` exposes a subset of the entry API (`and_modify`,
`or_insert`, etc), however it's not an enum so it doesn't allow pattern
matching. Also, `or_insert` won't return the component because it's all
based on commands.

## Testing

Added a new test `entity_commands_entry`.

---

## Showcase

```rust
commands
    .entity(player)
    .entry::<Level>()
    .and_modify(|mut lvl| lvl.0 += 1)
    .or_default();
```

---------

Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
2024-09-19 15:20:13 +00:00
TheBigCheese
b1273d48cb
Enable clippy::check-private-items so that missing_safety_doc will apply to private functions as well (#15161)
Enabled `check-private-items` in `clippy.toml` and then fixed the
resulting errors. Most of these were simply misformatted and of the
remaining:
- ~Added `#[allow(clippy::missing_safety_doc)]` to~ Removed unsafe from
a pair of functions in `bevy_utils/futures` which are only unsafe so
that they can be passed to a function which requires `unsafe fn`
- Removed `unsafe` from `UnsafeWorldCell::observers` as from what I can
tell it is always safe like `components`, `bundles` etc. (this should be
checked)
- Added safety docs to:
- `Bundles::get_storage_unchecked`: Based on the function that writes to
`dynamic_component_storages`
- `Bundles::get_storages_unchecked`: Based on the function that writes
to `dynamic_bundle_storages`
   - `QueryIterationCursor::init_empty`: Duplicated from `init`
- `QueryIterationCursor::peek_last`: Thanks Giooschi (also added
internal unsafe blocks)
   - `tests::drop_ptr`: Moved safety comment out to the doc string
 
This lint would also apply to `missing_errors_doc`, `missing_panics_doc`
and `unnecessary_safety_doc` if we chose to enable any of those at some
point, although there is an open
[issue](https://github.com/rust-lang/rust-clippy/issues/13074) to
separate these options.
2024-09-18 15:28:41 +00:00
Christian Hughes
378dcacf82
Group IntoSystemConfigs impls together (#15254)
# Objective

Two of the `IntoSystemConfigs` `impl`s are out of place near the top of
the file.

## Solution

Put them below the `IntoSystemConfigs` trait definition, alongside the
other `impl`.
2024-09-17 17:57:22 +00:00
Patrick Walton
3c41586154
Add EntityRefExcept and EntityMutExcept world queries, in preparation for generalized animation. (#15207)
This commit adds two new `WorldQuery` types: `EntityRefExcept` and
`EntityMutExcept`. These types work just like `EntityRef` and
`EntityMut`, but they prevent access to a statically-specified list of
components. For example, `EntityMutExcept<(AnimationPlayer,
Handle<AnimationGraph>)>` provides mutable access to all components
except for `AnimationPlayer` and `Handle<AnimationGraph>`. These types
are useful when you need to be able to process arbitrary queries while
iterating over the results of another `EntityMut` query.

The motivating use case is *generalized animation*, which is an upcoming
feature that allows animation of any component property, not just
rotation, translation, scaling, or morph weights. To implement this, we
must change the current `AnyOf<(&mut Transform, &mut MorphWeights)>` to
instead be `EntityMutExcept<(AnimationPlayer, Handle<AnimationGraph>)>`.
It's possible to use `FilteredEntityMut` in conjunction with a
dynamically-generated system instead, but `FilteredEntityMut` isn't
optimized for the use case of a large number of allowed components
coupled with a small set of disallowed components. No amount of
optimization of `FilteredEntityMut` produced acceptable performance on
the `many_foxes` benchmark. `Query<EntityMut, Without<AnimationPlayer>>`
will not suffice either, as it's legal and idiomatic for an
`AnimationTarget` and an `AnimationPlayer` to coexist on the same
entity.

An alternate proposal was to implement a somewhat-more-general
`Except<Q, CL>` feature, where Q is a `WorldQuery` and CL is a
`ComponentList`. I wasn't able to implement that proposal in a
reasonable way, because of the fact that methods like
`EntityMut::get_mut` and `EntityRef::get` are inherent methods instead
of methods on `WorldQuery`, and therefore there was no way to delegate
methods like `get` and `get_mut` to the inner query in a generic way.
Refactoring those methods into a trait would probably be possible.
However, I didn't see a use case for a hypothetical `Except` with
arbitrary queries: `Query<Except<(&Transform, &Visibility),
Visibility>>` would just be a complicated equivalent to
`Query<&Transform>`, for instance. So, out of a desire for simplicity, I
omitted a generic `Except` mechanism.

I've tested the performance of generalized animation on `many_foxes` and
found that, with this patch, `animate_targets` has a 7.4% slowdown over
`main`. With `FilteredEntityMut` optimized to use `Arc<Access>`, the
slowdown is 75.6%, due to contention on the reference count. Without
`Arc<Access>`, the slowdown is even worse, over 2x.

## Testing

New tests have been added that check that `EntityRefExcept` and
`EntityMutExcept` allow and disallow access to components properly and
that the query engine can correctly reject conflicting queries involving
those types.

A Tracy profile of `many_foxes` with 10,000 foxes showing generalized
animation using `FilteredEntityMut` (red) vs. main (yellow) is as
follows:

![Screenshot 2024-09-12
225914](https://github.com/user-attachments/assets/2993d74c-a513-4ba4-85bd-225672e7170a)

A Tracy profile of `many_foxes` with 10,000 foxes showing generalized
animation using this `EntityMutExcept` (yellow) vs. main (red) is as
follows:

![Screenshot 2024-09-14
205831](https://github.com/user-attachments/assets/4241015e-0c5d-44ef-835b-43f78a24e604)
2024-09-17 14:53:39 +00:00
Benjamin Brienen
b45d83ebda
Rename Add to Queue for methods with deferred semantics (#15234)
# Objective

- Fixes #15106

## Solution

- Trivial refactor to rename the method. The duplicate method `push` was
removed as well. This will simpify the API and make the semantics more
clear. `Add` implies that the action happens immediately, whereas in
reality, the command is queued to be run eventually.
- `ChildBuilder::add_command` has similarly been renamed to
`queue_command`.

## Testing

Unit tests should suffice for this simple refactor.

---

## Migration Guide

- `Commands::add` and `Commands::push` have been replaced with
`Commnads::queue`.
- `ChildBuilder::add_command` has been renamed to
`ChildBuilder::queue_command`.
2024-09-17 00:17:49 +00:00
Navneet Aman
c2d54f5f04
Don't leak SEND resource, even if thread is panicking. (#15247)
# Objective
Currently the resource doesn't get dropped if thread panics. This is
presumably to prevent !SEND resource from being dropped by wrong thread.
But, this logic is not needed for SEND resources. So we don't need this
check for SEND resource.

Fixes #15144 

## Solution

We check if resource is !SEND before, validating that correct thread is
dropping the resource.

## Testing

- Did you test these changes? If so, how?
I did run cargo test on bevy.
- 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?
Nothing special
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
x86_64 desktop
2024-09-16 23:36:52 +00:00
Benjamin Brienen
29508f065f
Fix floating point math (#15239)
# Objective

- Fixes #15236

## Solution

- Use bevy_math::ops instead of std floating point operations.

## Testing

- Did you test these changes? If so, how?
Unit tests and `cargo run -p ci -- test`

- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Execute `cargo run -p ci -- test` on Windows.

- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Windows

## Migration Guide

- Not a breaking change
- Projects should use bevy math where applicable

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-09-16 23:28:12 +00:00
Cole Varner
17b1bcde95
Add missing insert API commands (#15166)
# Objective

- Adds the missing API commands `insert_if_new_and` and
`try_insert_if_new_and` (resolves #15105)
- Adds some test coverage for existing insert commands

## Testing

- Implemented additional unit tests to add coverage
2024-09-16 23:00:00 +00:00
Chris Russell
382917fbb3
Improve type inference in DynSystemParam::downcast() by making the type parameter match the return value. (#15103)
# Objective

Right now, `DynSystemParam::downcast()` always requires the type
parameter to be specified with a turbofish. Make it so that it can be
inferred from the use of the return value, like:

```rust
fn expects_res_a(mut param: DynSystemParam) {
    let res: Res<A> = param.downcast().unwrap();
}
```

## Solution

The reason this doesn't currently work is that the type parameter is a
`'static` version of the `SystemParam` so that it can be used with
`Any::downcast_mut()`. Change the method signature so that the type
parameter matches the return type, and use `T::Item<'static, 'static>`
to get the `'static` version. That means we wind up returning a
`T::Item<'static, 'static>::Item<'w, 's>`, so constrain that to be equal
to `T`. That works with every `SystemParam` implementation, since they
have `T::Item == T` up to lifetimes.
2024-09-16 22:56:57 +00:00
Adam
9bda913e36
Remove redundent information and optimize dynamic allocations in Table (#12929)
# Objective

- fix #12853
- Make `Table::allocate` faster

## Solution
The PR consists of multiple steps:

1) For the component data: create a new data-structure that's similar to
`BlobVec` but doesn't store `len` & `capacity` inside of it: "BlobArray"
(name suggestions welcome)
2) For the `Tick` data: create a new data-structure that's similar to
`ThinSlicePtr` but supports dynamic reallocation: "ThinArrayPtr" (name
suggestions welcome)
3) Create a new data-structure that's very similar to `Column` that
doesn't store `len` & `capacity` inside of it: "ThinColumn"
4) Adjust the `Table` implementation to use `ThinColumn` instead of
`Column`

The result is that only one set of `len` & `capacity` is stored in
`Table`, in `Table::entities`

### Notes Regarding Performance
Apart from shaving off some excess memory in `Table`, the changes have
also brought noteworthy performance improvements:
The previous implementation relied on `Vec::reserve` &
`BlobVec::reserve`, but that redundantly repeated the same if statement
(`capacity` == `len`). Now that check could be made at the `Table` level
because the capacity and length of all the columns are synchronized;
saving N branches per allocation. The result is a respectable
performance improvement per every `Table::reserve` (and subsequently
`Table::allocate`) call.

I'm hesitant to give exact numbers because I don't have a lot of
experience in profiling and benchmarking, but these are the results I
got so far:

*`add_remove_big/table` benchmark after the implementation:*


![after_add_remove_big_table](https://github.com/bevyengine/bevy/assets/46227443/b667da29-1212-4020-8bb0-ec0f15bb5f8a)

*`add_remove_big/table` benchmark in main branch (measured in comparison
to the implementation):*


![main_add_remove_big_table](https://github.com/bevyengine/bevy/assets/46227443/41abb92f-3112-4e01-b935-99696eb2fe58)

*`add_remove_very_big/table` benchmark after the implementation:*


![after_add_remove_very_big](https://github.com/bevyengine/bevy/assets/46227443/f268a155-295b-4f55-ab02-f8a9dcc64fc2)

*`add_remove_very_big/table` benchmark in main branch (measured in
comparison to the implementation):*


![main_add_remove_very_big](https://github.com/bevyengine/bevy/assets/46227443/78b4e3a6-b255-47c9-baee-1a24c25b9aea)

cc @james7132 to verify

---

## Changelog

- New data-structure that's similar to `BlobVec` but doesn't store `len`
& `capacity` inside of it: `BlobArray`
- New data-structure that's similar to `ThinSlicePtr` but supports
dynamic allocation:`ThinArrayPtr`
- New data-structure that's very similar to `Column` that doesn't store
`len` & `capacity` inside of it: `ThinColumn`
- Adjust the `Table` implementation to use `ThinColumn` instead of
`Column`
- New benchmark: `add_remove_very_big` to benchmark the performance of
spawning a lot of entities with a lot of components (15) each

## Migration Guide

`Table` now uses `ThinColumn` instead of `Column`. That means that
methods that previously returned `Column`, will now return `ThinColumn`
instead.

`ThinColumn` has a much more limited and low-level API, but you can
still achieve the same things in `ThinColumn` as you did in `Column`.
For example, instead of calling `Column::get_added_tick`, you'd call
`ThinColumn::get_added_ticks_slice` and index it to get the specific
added tick.

---------

Co-authored-by: James Liu <contact@jamessliu.com>
2024-09-16 22:52:05 +00:00
Federico Rinaldi
262b068bc3
Substitute trivial fallible conversions with infallible function calls (#10846)
Clippy for rust 1.75 will introduce the
[`unnecessary_fallible_conversions`][1] lint. This PR solves the trivial
ones.

[1]:
https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
2024-09-16 22:46:54 +00:00
Blazepaws
21e39360f7
Example for bevy_ecs::event::Events uses deprecated function get_reader (#15216)
Fixes https://github.com/bevyengine/bevy/issues/15214
2024-09-15 14:24:04 +00:00
Blazepaws
53d2bc9482
Reflect derived traits on all components and resources: bevy_ecs (#15215)
Solves https://github.com/bevyengine/bevy/issues/15187 for bevy_ecs
2024-09-15 14:23:54 +00:00
kivi
3f425da66e
Improve schedule note of .after/.before & encourage to use .chain ins… (#14986)
# Objective

- Fixes #14552 
- Make the current note of `before` and `after` understandable. 
- > The given set is not implicitly added to the schedule when this
system set is added.

## Solution

- Replace note in docs of [`after` and
`before`](https://docs.rs/bevy/latest/bevy/ecs/prelude/trait.IntoSystemConfigs.html#method.before)
- Note of after was removed completely, and links to `before`, because
they notes would be identical.
- Also encourage to use `.chain`, which is much simpler and safer to use

## Testing

- Checked the docs after running `cargo doc` and `cargo run -p ci --
lints`
- Are there any parts that need more testing?
- no need to test, but please review the text. If it is still including
the intended message and especially if its understandable.

---------

Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-09-14 21:03:21 +00:00
Wybe Westra
70808af776
Added ordering information to observer tests (#14332) (#15178)
Fixes #14332 by recording the order in which the events occur.
2024-09-14 14:08:49 +00:00
SpecificProtagonist
f570f52aa2
Optimize observer unregistration (#15150)
# Objective

Fixes #14980

## Solution

Only iterate over archetypes containing the component.

## Alternatives

Additionally, for each archetype, cache how many observers are watching
one of its components & use this to speed up the check for each affected
archetype ([implemented
here](55c89aa033)).
Benchmarking showed this to lead only to a minor speedup.

## Testing

There's both already a test checking that observers don't run after
being despawned as well as a regression test for the bug that
necessitates the check this PR optimizes.
2024-09-13 15:47:24 +00:00
Robert Walter
327e1ddba4
Micro typo in bevy_ecs (#15167)
I'm currently reading through the code and docs and found this.
2024-09-12 08:43:33 +00:00
Niashi
8bfe635c3e
Finish enhancing ReflectCommandExt to work with Bundles (#15152)
# Objective

- Finish resolving https://github.com/bevyengine/bevy/issues/15125
- Inserting bundles was implemented in
https://github.com/bevyengine/bevy/pull/15128 but removing bundles still
needed to be implemented.

## Solution

- Modified `bevy_ecs::reflect::entity_commands::remove_reflect` to
handle both components and bundles
- Modified documentation of `ReflectCommandExt` methods to reflect that
one can now use bundles with these commands.

## Testing

- Three tests were added to match the ones for inserting components.
2024-09-11 03:19:28 +00:00
Zachary Harrold
be35cba801
Removed Type Parameters from Observer (#15151)
# Objective

- Remove any ambiguity around how multiple `Observer` components work on
a single `Entity` by completely removing the concept.
- Fixes #15122

## Solution

- Removed type parameters from `Observer`, relying on a function pointer
to provide type information into the relevant aspects of running an
observer.

## Testing

- Ran CI locally.
- Checked `observers.rs` example continued to function as expected.

## Notes

This communicates to users of observers that only a single `Observer`
can be inserted onto an entity at a time within the established type
system. This has been achieved by erasing the type information from the
stored `ObserverSystem` and retrieving it again using a function
pointer. This has the downside of increasing the size of the `Observer`
component and increases the complexity of the observer runner. However,
this complexity was already present, and is in my opinion a worthwhile
tradeoff for the clearer user experience.

The other notable benefit is users no longer need to use the
`ObserverState` component to filter for `Observer` entities, and can
instead use `Observer` directly.

Technically this is a breaking change, since the type signature for
`Observer` has changed. However, it was so cumbersome to use that I
don't believe there are any instances in the wild of users directly
naming `Observer` types, instead relying on `ObserverState`, and the
methods provided by `App` and `World`. As can be seen in the diff, this
change had very little knock-on effects across Bevy.

## Migration Guide

If you filtered for observers using `Observer<A, B>`, instead filter for
an `Observer`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-09-11 01:14:28 +00:00
Han Damin
0cf276f239
Enhance ReflectCommandExt (#15128)
# Objective

- Enhance #15125

## Solution

- Modified `ReflectCommandExt::insert_reflect` to accept and handle both
components and bundles.

---------

Co-authored-by: Gonçalo Rica Pais da Silva <bluefinger@gmail.com>
Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
2024-09-09 22:34:44 +00:00
Christian Hughes
79f6fcd1eb
EntityRef/Mut get_components (immutable variants only) (#15089)
# Objective

Smaller scoped version of #13375 without the `_mut` variants which
currently have unsoundness issues.

## Solution

Same as #13375, but without the `_mut` variants.

## Testing

- The same test from #13375 is reused.

---

## Migration Guide

- Renamed `FilteredEntityRef::components` to
`FilteredEntityRef::accessed_components` and
`FilteredEntityMut::components` to
`FilteredEntityMut::accessed_components`.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: Periwink <charlesbour@gmail.com>
2024-09-09 16:29:44 +00:00
Tim
5adacf014c
Use associated type bounds for iter_many and friends (#15040)
# Objective

Make the bounds for these query methods less intimidating.
Continuation of #14107

<sub>My last pr was back in february 💀
2024-09-09 16:24:39 +00:00
Zachary Harrold
85e41ddace
Add observer to Trigger (#15066)
# Objective

- Fixes  #15061

## Solution

- Added `observer` to `Trigger`, which returns the entity observing the
triggered event.

## Testing

- CI passed locally.
2024-09-09 16:23:14 +00:00
Christian Hughes
e939d6c33f
Remove remnant EntityHash and related types from bevy_utils (#15039)
# Objective

`EntityHash` and related types were moved from `bevy_utils` to
`bevy_ecs` in #11498, but seemed to have been accidentally reintroduced
a week later in #11707.

## Solution

Remove the old leftover code.

---

## Migration Guide

- Uses of `bevy::utils::{EntityHash, EntityHasher, EntityHashMap,
EntityHashSet}` now have to be imported from `bevy::ecs::entity`.
2024-09-09 15:24:17 +00:00
Chris Russell
a9d2a9ea37
Make QueryFilter an unsafe trait (#14790)
# Objective

It's possible to create UB using an implementation of `QueryFilter` that
performs mutable access, but that does not violate any documented safety
invariants.

This code: 
```rust
#[derive(Component)]
struct Foo(usize);

// This derive is a simple way to get a valid WorldQuery impl.  The QueryData impl isn't used.
#[derive(QueryData)]
#[query_data(mutable)]
struct BadFilter<'w> {
    foo: &'w mut Foo,
}

impl QueryFilter for BadFilter<'_> {
    const IS_ARCHETYPAL: bool = false;

    unsafe fn filter_fetch(
        fetch: &mut Self::Fetch<'_>,
        entity: Entity,
        table_row: TableRow,
    ) -> bool {
        // SAFETY: fetch and filter_fetch have the same safety requirements
        let f: &mut usize = &mut unsafe { Self::fetch(fetch, entity, table_row) }.foo.0;
        println!("Got &mut at     {f:p}");
        true
    }
}

let mut world = World::new();
world.spawn(Foo(0));
world.run_system_once(|query: Query<&Foo, BadFilter>| {
    let f: &usize = &query.iter().next().unwrap().0;
    println!("Got & at        {f:p}");
    query.iter().next().unwrap();
    println!("Still have & at {f:p}");
});
```

prints: 

```
Got &mut at     0x1924b92dfb0
Got & at        0x1924b92dfb0
Got &mut at     0x1924b92dfb0
Still have & at 0x1924b92dfb0
```

Which means it had an `&` and `&mut` alive at the same time.

The only `unsafe` there is around `Self::fetch`, but I believe that call
correctly upholds the safety invariant, and matches what `Added` and
`Changed` do.


## Solution

Make `QueryFilter` an unsafe trait and document the requirement that the
`WorldQuery` implementation be read-only.

## Migration Guide

`QueryFilter` is now an `unsafe trait`. If you were manually
implementing it, you will need to verify that the `WorldQuery`
implementation is read-only and then add the `unsafe` keyword to the
`impl`.
2024-09-09 15:23:12 +00:00
Rob Parrett
0a79a0ac8c
Fix error link (#15082)
# Objective

A previous issue describes the same problem: #14248.

This particular link was seemingly missed by #14276.

## Solution

- Search repo for `bevyengine.org/learn/errors/#`
- Remove `#`
- Verify link goes to right place
2024-09-08 17:11:17 +00:00
BD103
6ec6a55645
Unify crate-level preludes (#15080)
# Objective

- Crate-level prelude modules, such as `bevy_ecs::prelude`, are plagued
with inconsistency! Let's fix it!

## Solution

Format all preludes based on the following rules:

1. All preludes should have brief documentation in the format of:
   > The _name_ prelude.
   >
> This includes the most common types in this crate, re-exported for
your convenience.
2. All documentation should be outer, not inner. (`///` instead of
`//!`.)
3. No prelude modules should be annotated with `#[doc(hidden)]`. (Items
within them may, though I'm not sure why this was done.)

## Testing

- I manually searched for the term `mod prelude` and updated all
occurrences by hand. 🫠

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
2024-09-08 17:10:57 +00:00
re0312
739007f148
Opportunistically use dense iter for archetypal iteration in Par_iter (#14673)
# Objective

- follow of #14049 ,we could use it on our Parallel Iterator,this pr
also unified the used function in both regular iter and parallel
iterations.


## Performance 


![image](https://github.com/user-attachments/assets/cba700bc-169c-4b58-b504-823bdca8ec05)

no performance regression for regular itertaion

3.5X faster in hybrid parallel iteraion,this number is far greater than
the benefits obtained in regular iteration(~1.81) because mutable
iterations on continuous memory can effectively reduce the cost of
mataining core cache coherence
2024-09-03 23:41:10 +00:00
Martín Maita
32f40f11b5
Bump crate-ci/typos from 1.24.1 to 1.24.3 (#15024)
# Objective

- Adopts #15015

## Solution

- Fixed a typo that broke the build and prevented updating
`crate-ci/typos`.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2024-09-03 00:57:41 +00:00
Chris Russell
f1414cba23
Use #[doc(fake_variadic)] for SystemParamBuilder tuple impls. (#14962)
# Objective

Make the documentation for `SystemParamBuilder` nicer by combining the
tuple implementations into a single line of documentation.

## Solution

Use `#[doc(fake_variadic)]` for `SystemParamBuilder` tuple impls.


![image](https://github.com/user-attachments/assets/b4665861-c405-467f-b30b-82b4b1d99bf7)

(This got missed originally because #14050 and #14703 were open at the
same time.)
2024-09-02 16:51:23 +00:00
no-materials
3a8d5598ad
Interpolate WorldQuery path in docs of generated types (#14985)
# Objective

Fixes #14972

## Solution

Uses the `concat!` macro to interpolate the `path` variable.

## Testing

* Run `cargo doc --workspace --open`
* Check functionality of `WorldQuery` links within `NodeQueryItem`,
`NodeQueryReadOnly`, `NodeQueryReadOnlyItem` docs
2024-09-01 22:18:13 +00:00
Zachary Harrold
bc13161416
Migrated NonZero* to NonZero<*> (#14978)
# Objective

- Fixes #14974

## Solution

- Replace all* instances of `NonZero*` with `NonZero<*>`

## Testing

- CI passed locally.

---

## Notes

Within the `bevy_reflect` implementations for `std` types,
`impl_reflect_value!()` will continue to use the type aliases instead,
as it inappropriately parses the concrete type parameter as a generic
argument. If the `ZeroablePrimitive` trait was stable, or the macro
could be modified to accept a finite list of types, then we could fully
migrate.
2024-08-30 02:37:47 +00:00
Alix Bott
f2cf02408f
Fix observer unregistering unsetting archetype flags (#14963)
# Objective

- Fixes https://github.com/bevyengine/bevy/issues/14961

## Solution

- Check that the archetypes don't contain any other observed components
before unsetting their flags

## Testing

- I added a regression test: `observer_despawn_archetype_flags`
2024-08-30 00:43:56 +00:00
Chris Russell
4be8e497ca
SystemParamBuilder - Allow deriving a SystemParamBuilder struct when deriving SystemParam. (#14818)
# Objective

Allow `SystemParamBuilder` implementations for custom system parameters
created using `#[derive(SystemParam)]`.

## Solution

Extend the derive macro to accept a `#[system_param(builder)]`
attribute. When present, emit a builder type with a field corresponding
to each field of the param.

## Example

```rust
#[derive(SystemParam)]
#[system_param(builder)]
struct CustomParam<'w, 's> {
    query: Query<'w, 's, ()>,
    local: Local<'s, usize>,
}

let system = (CustomParamBuilder {
    local: LocalBuilder(100),
    query: QueryParamBuilder::new(|builder| {
        builder.with::<A>();
    }),
},)
    .build_state(&mut world)
    .build_system(|param: CustomParam| *param.local + param.query.iter().count());
```
2024-08-28 18:24:52 +00:00
Zachary Harrold
371e07e77d
Updated FromWorld Documentation to mention Default (#14954)
# Objective

- Fixes #14860

## Solution

- Added a line of documentation to `FromWorld`'s trait definition
mention the `Default` blanket implementation.
- Added custom documentation to the `from_world` method for the
`Default` blanket implementation. This ensures when inspecting the
`from_world` function within an IDE, the tooltip will explicitly state
the `default()` method will be used for any `Default` types.

## Testing

- CI passes.
2024-08-28 11:37:31 +00:00