Commit Graph

1622 Commits

Author SHA1 Message Date
Zachary Harrold
5cc3f4727e
Implement Clone for QueryIter over read-only data (#17391)
# Objective

- Fix issue identified on the [Discord
server](https://discord.com/channels/691052431525675048/691052431974465548/1328922812530036839)

## Solution

- Implement `Clone` for `QueryIter` using the existing
`QueryIter::remaining` method

## Testing

- CI

---

## Showcase

Users can now explicitly clone a read-only `QueryIter`:

```rust
fn combinations(query: Query<&ComponentA>) {
    let mut iter = query.iter();
    while let Some(a) = iter.next() {
        // Can now clone rather than use remaining
        for b in iter.clone() {
            // Check every combination (a, b)
        }
    }
}
```


## Notes

This doesn't add any new functionality outside the context of generic
code (e.g., `T: Iterator<...> + Clone`), it's mostly for
discoverability. Users are more likely to be familiar with
`Clone::clone` than they are with the methods on `QueryIter`.
2025-01-15 21:56:11 +00:00
Alice Cecile
237c6b207e
Remove Event: Component trait bound using a wrapper type which impls Component (#17380)
# Objective

As raised in https://github.com/bevyengine/bevy/pull/17317, the `Event:
Component` trait bound is confusing to users.

In general, a type `E` (like `AppExit`) which implements `Event` should
not:

- be stored as a component on an entity
- be a valid option for `Query<&AppExit>`
- require the storage type and other component metadata to be specified

Events are not components (even if they one day use some of the same
internal mechanisms), and this trait bound is confusing to users.

We're also automatically generating `Component` impls with our derive
macro, which should be avoided when possible to improve explicitness and
avoid conflicts with user impls.

Closes #17317, closes #17333

## Solution

- We only care that each unique event type gets a unique `ComponentId`
- dynamic events need their own tools for getting identifiers anyways
- This avoids complicating the internals of `ComponentId` generation.
- Clearly document why this cludge-y solution exists.

In the medium term, I think that either a) properly generalizing
`ComponentId` (and moving it into `bevy_reflect?) or b) using a
new-typed `Entity` as the key for events is more correct. This change is
stupid simple though, and removes the offending trait bound in a way
that doesn't introduce complex tech debt and does not risk changes to
the internals.

This change does not:

- restrict our ability to implement dynamic buffered events (the main
improvement over #17317)
- there's still a fair bit of work to do, but this is a step in the
right direction
- limit our ability to store event metadata on entities in the future
- make it harder for users to work with types that are both events and
components (just add the derive / trait bound)

## Migration Guide

The `Event` trait no longer requires the `Component` trait. If you were
relying on this behavior, change your trait bounds from `Event` to
`Event + Component`. If you also want your `Event` type to implement
`Component`, add a derive.

---------

Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
2025-01-15 18:42:47 +00:00
MichiRecRoom
26bb0b40d2
Move #![warn(clippy::allow_attributes, clippy::allow_attributes_without_reason)] to the workspace Cargo.toml (#17374)
# Objective
Fixes https://github.com/bevyengine/bevy/issues/17111

## Solution
Move `#![warn(clippy::allow_attributes,
clippy::allow_attributes_without_reason)]` to the workspace `Cargo.toml`

## Testing
Lots of CI testing, and local testing too.

---------

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
2025-01-15 01:14:58 +00:00
Adrian Kortyczko
dcff8f3ecb
Make ObservedBy public (#17297)
# Objective
- Currently, the `ObservedBy`-component is only public within the
`bevy_ecs` crate. Sometimes it is desirable to refer to this component
in the "game-code". Two examples that come in mind:
- Clearing all components in an entity, but intending to keep the
existing observers: Making `ObservedBy` public allows us to use
`commands.entity(entity).retain::<ObservedBy>();`, which clears all
other components, but keeps `ObservedBy`, which prevents the Observers
from despawning.
- The opposite of the above, clearing all of entities' Observers:
`commands.entity(entity).remove::<ObservedBy>` will despawn all
associated Observers. Admittedly, a cleaner solution would be something
like `commands.entity(entity).clear_observers()`, but this is
sufficient.

## Solution

- Removed `(crate)` "rule" and added `ObservedBy` to the prelude-module

## Testing

- Linked `bevy_ecs` locally with another project to see if `ObservedBy`
could be referenced.
2025-01-14 21:53:42 +00:00
Jaso333
9ef1964d34
added any_match_filter common condition (#17327)
# Objective

resolves #17326.

## Solution

Simply added the suggested run condition.

## Testing

A self-explanatory run condition. Fully verified by the operation of
`QueryFilter` in a system.
2025-01-14 21:53:35 +00:00
Younes
276d6e8014
fix typo query.rs (#17366)
fix typo query.rs
2025-01-14 21:53:19 +00:00
MichiRecRoom
17c46f4add
bevy_ecs: Apply #![warn(clippy::allow_attributes, clippy::allow_attributes_without_reason)] (#17335)
# Objective
- https://github.com/bevyengine/bevy/issues/17111

## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `warn`, and bring
`bevy_ecs` in line with the new restrictions.

## Testing
This PR is a WIP; testing will happen after it's finished.
2025-01-14 21:37:41 +00:00
Younes
031bb09737
Refactor event system documentation in system_param.rs (#17364)
Just clarify the role of `ParamSet` in this code snippet
2025-01-14 19:38:25 +00:00
Alexis "spectria.limina" Horizon
4dfa87798f
Add std derives to SystemParam types (#16785)
# Objective

- Use `Clone` on `SystemParam`, when applicable, in a generic context.

## Solution

-  Add some derives

## Testing

- I ran `cargo test` once.
- I didn't even look at the output.

---------

Co-authored-by: François Mockers <mockersf@gmail.com>
2025-01-14 00:30:51 +00:00
AlephCubed
e808fbe987
Renamed members of ParamWarnPolicy to reflect new behaviour. (#17311)
- `Once` renamed to `Warn`.
- `param_warn_once()` renamed to `warn_param_missing()`.
- `never_param_warn()` renamed to `ignore_param_missing()`.

Also includes changes to the documentation of the above methods.

Fixes #17262.

## Migration Guide
- `ParamWarnPolicy::Once` has been renamed to `ParamWarnPolicy::Warn`.
- `ParamWarnPolicy::param_warn_once` has been renamed to
`ParamWarnPolicy::warn_param_missing`.
- `ParamWarnPolicy::never_param_warn` has been renamed to
`ParamWarnPolicy::ignore_param_missing`.
2025-01-12 05:40:04 +00:00
SpecificProtagonist
f5d38f30cc
Fix entity does not exist message on index reuse (#17264)
# Objective

With the `track_location` feature, the error message of trying to
acquire an entity that was despawned pointed to the wrong line if the
entity index has been reused.

## Showcase

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

fn main() {
    let mut world = World::new();
    let e = world.spawn_empty().id();
    world.despawn(e);
    world.flush();
    let _ = world.spawn_empty();
    world.entity(e);
}
```
Old message:
```
Entity 0v1 was despawned by src/main.rs:8:19
```
New message:
```
Entity 0v1 does not exist (its index has been reused)
```
2025-01-12 05:39:54 +00:00
Rob Parrett
b77e3ef33a
Fix a few typos (#17292)
# Objective

Stumbled upon a `from <-> form` transposition while reviewing a PR,
thought it was interesting, and went down a bit of a rabbit hole.

## Solution

Fix em
2025-01-10 22:48:30 +00:00
Carter Anderson
4bca7f1b6d
Improved Command Errors (#17215)
# Objective

Rework / build on #17043 to simplify the implementation. #17043 should
be merged first, and the diff from this PR will get much nicer after it
is merged (this PR is net negative LOC).

## Solution

1. Command and EntityCommand have been vastly simplified. No more marker
components. Just one function.
2. Command and EntityCommand are now generic on the return type. This
enables result-less commands to exist, and allows us to statically
distinguish between fallible and infallible commands, which allows us to
skip the "error handling overhead" for cases that don't need it.
3. There are now only two command queue variants: `queue` and
`queue_fallible`. `queue` accepts commands with no return type.
`queue_fallible` accepts commands that return a Result (specifically,
one that returns an error that can convert to
`bevy_ecs::result::Error`).
4. I've added the concept of the "default error handler", which is used
by `queue_fallible`. This is a simple direct call to the `panic()` error
handler by default. Users that want to override this can enable the
`configurable_error_handler` cargo feature, then initialize the
GLOBAL_ERROR_HANDLER value on startup. This is behind a flag because
there might be minor overhead with `OnceLock` and I'm guessing this will
be a niche feature. We can also do perf testing with OnceLock if someone
really wants it to be used unconditionally, but I don't personally feel
the need to do that.
5. I removed the "temporary error handler" on Commands (and all code
associated with it). It added more branching, made Commands bigger /
more expensive to initialize (note that we construct it at high
frequencies / treat it like a pointer type), made the code harder to
follow, and introduced a bunch of additional functions. We instead rely
on the new default error handler used in `queue_fallible` for most
things. In the event that a custom handler is required,
`handle_error_with` can be used.
6. EntityCommand now _only_ supports functions that take
`EntityWorldMut` (and all existing entity commands have been ported).
Removing the marker component from EntityCommand hinged on this change,
but I strongly believe this is for the best anyway, as this sets the
stage for more efficient batched entity commands.
7. I added `EntityWorldMut::resource` and the other variants for more
ergonomic resource access on `EntityWorldMut` (removes the need for
entity.world_scope, which also incurs entity-lookup overhead).

## Open Questions

1. I believe we could merge `queue` and `queue_fallible` into a single
`queue` which accepts both fallible and infallible commands (via the
introduction of a `QueueCommand` trait). Is this desirable?
2025-01-10 04:15:50 +00:00
mgi388
b20e23dd41
Add compile-time dyn compatible checks for DynEq, DynHash (#17254)
# Objective

- Shrink `bevy_utils` more.
- Refs #11478

## Solution

- Removes `assert_object_safe` from `bevy_utils` by using a compile time
check instead.

## Testing

- CI.

---

## Migration Guide

`assert_object_safe` is no longer exported by `bevy_utils`. Instead, you
can write a compile time check that your trait is "dyn compatible":

```rust
/// Assert MyTrait is dyn compatible
const _: Option<Box<dyn MyTrait>> = None;
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2025-01-09 07:30:54 +00:00
MichiRecRoom
3742e621ef
Allow clippy::too_many_arguments to lint without warnings (#17249)
# Objective
Many instances of `clippy::too_many_arguments` linting happen to be on
systems - functions which we don't call manually, and thus there's not
much reason to worry about the argument count.

## Solution
Allow `clippy::too_many_arguments` globally, and remove all lint
attributes related to it.
2025-01-09 07:26:15 +00:00
JaySpruce
ee4414159b
Add Result handling to Commands and EntityCommands (#17043)
## Objective

Fixes #2004
Fixes #3845
Fixes #7118
Fixes #10166

## Solution

- The crux of this PR is the new `Command::with_error_handling` method.
This wraps the relevant command in another command that, when applied,
will apply the original command and handle any resulting errors.
- To enable this, `Command::apply` and `EntityCommand::apply` now return
`Result`.
- `Command::with_error_handling` takes as a parameter an error handler
of the form `fn(&mut World, CommandError)`, which it passes the error
to.
- `CommandError` is an enum that can be either `NoSuchEntity(Entity)` or
`CommandFailed(Box<dyn Error>)`.

### Closures
- Closure commands can now optionally return `Result`, which will be
passed to `with_error_handling`.

### Commands
- Fallible commands can be queued with `Commands::queue_fallible` and
`Commands::queue_fallible_with`, which call `with_error_handling` before
queuing them (using `Commands::queue` will queue them without error
handling).
- `Commands::queue_fallible_with` takes an `error_handler` parameter,
which will be used by `with_error_handling` instead of a command's
default.
- The `command` submodule provides unqueued forms of built-in fallible
commands so that you can use them with `queue_fallible_with`.
- There is also an `error_handler` submodule that provides simple error
handlers for convenience.

### Entity Commands
- `EntityCommand` now automatically checks if the entity exists before
executing the command, and returns `NoSuchEntity` if it doesn't.
- Since all entity commands might need to return an error, they are
always queued with error handling.
- `EntityCommands::queue_with` takes an `error_handler` parameter, which
will be used by `with_error_handling` instead of a command's default.
- The `entity_command` submodule provides unqueued forms of built-in
entity commands so that you can use them with `queue_with`.

### Defaults
- In the future, commands should all fail according to the global error
handling setting. That doesn't exist yet though.
- For this PR, commands all fail the way they do on `main`.
- Both now and in the future, the defaults can be overridden by
`Commands::override_error_handler` (or equivalent methods on
`EntityCommands` and `EntityEntryCommands`).
- `override_error_handler` takes an error handler (`fn(&mut World,
CommandError)`) and passes it to every subsequent command queued with
`Commands::queue_fallible` or `EntityCommands::queue`.
- The `_with` variants of the queue methods will still provide an error
handler directly to the command.
- An override can be reset with `reset_error_handler`.

## Future Work

- After a universal error handling mode is added, we can change all
commands to fail that way by default.
- Once we have all commands failing the same way (which would require
either the full removal of `try` variants or just making them useless
while they're deprecated), `queue_fallible_with_default` could be
removed, since its only purpose is to enable commands having different
defaults.
2025-01-07 16:50:52 +00:00
Christian Hughes
d1e5702020
Replace unsafe blocks in World and DeferredWorld with safe equivalents (#17206)
# Objective

Reduce the number of unsafe blocks.

## Solution

Replaced 5 unsafe blocks with safe equivalents.

## Testing

Reusing current tests
2025-01-07 03:21:47 +00:00
Christian Hughes
f64f3ac997
Cleanup entity reference types (#17149)
# Objective

Cleanup `EntityRef`, `EntityMut`, and `EntityWorldMut` in preparation
for my "Scoped Entity References" PR.

## Solution

- Switched `EntityRef`/`EntityMut` from tuple structs to normal ones.
- Ensured all conversion trait impls use the same `entity` argument
name.
- Replaced some `unsafe` with delegated calls from `EntityMut` to
`EntityRef`
    - Added `EntityMut::into_readonly` to make the replacements clearer
- Replaced some `unsafe` with delegated calls from `EntityWorldMut` to
`EntityMut` and `EntityRef`
- Added `EntityWorldMut::into_readonly`, `::as_readonly`,
`::into_mutable`, `::as_mutable` to make the replacements clearer

## Testing

Reusing current tests.
2025-01-06 19:25:06 +00:00
vil'mo
b30ee2d051
Disallow requesting write resource access in Queries (#17116)
Related to https://github.com/bevyengine/bevy/pull/16843

Since `WorldQuery::Fetch` is `Clone`, it can't store mutable references
to resources, so it doesn't make sense to mutably access resources. In
that sense, it is hard to find usecases of mutably accessing resources
and to clearly define, what mutably accessing resources would mean, so
it's been decided to disallow write resource access.
Also changed documentation of safety requirements of
`WorldQuery::init_fetch` and `WorldQuery::fetch` to clearly state to the
caller, what safety invariants they need to uphold.
2025-01-06 19:04:26 +00:00
github-actions[bot]
573b980685
Bump Version after Release (#17176)
Bump version after release
This PR has been auto-generated

---------

Co-authored-by: Bevy Auto Releaser <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
2025-01-06 00:04:44 +00:00
Zachary Harrold
a371ee3019
Remove tracing re-export from bevy_utils (#17161)
# Objective

- Contributes to #11478

## Solution

- Made `bevy_utils::tracing` `doc(hidden)`
- Re-exported `tracing` from `bevy_log` for end-users
- Added `tracing` directly to crates that need it.

## Testing

- CI

---

## Migration Guide

If you were importing `tracing` via `bevy::utils::tracing`, instead use
`bevy::log::tracing`. Note that many items within `tracing` are also
directly re-exported from `bevy::log` as well, so you may only need
`bevy::log` for the most common items (e.g., `warn!`, `trace!`, etc.).
This also applies to the `log_once!` family of macros.

## Notes

- While this doesn't reduce the line-count in `bevy_utils`, it further
decouples the internal crates from `bevy_utils`, making its eventual
removal more feasible in the future.
- I have just imported `tracing` as we do for all dependencies. However,
a workspace dependency may be more appropriate for version management.
2025-01-05 23:06:34 +00:00
Benjamin Brienen
7112d5594e
Remove all deprecated code (#16338)
# Objective

Release cycle things

## Solution

Delete items deprecated in 0.15 and migrate bevy itself.

## Testing

CI
2025-01-05 20:33:39 +00:00
JaySpruce
4fde223831
Optimize Entities::entity_does_not_exist_error_details_message, remove UnsafeWorldCell from error (#17115)
## Objective

The error `EntityFetchError::NoSuchEntity` has an `UnsafeWorldCell`
inside it, which it uses to call
`Entities::entity_does_not_exist_error_details_message` when being
printed. That method returns a `String` that, if the `track_location`
feature is enabled, contains the location of whoever despawned the
relevant entity.

I initially had to modify this error while working on #17043. The
`UnsafeWorldCell` was causing borrow problems when being returned from a
command, so I tried replacing it with the `String` that the method
returns, since that was the world cell's only purpose.

Unfortunately, `String`s are slow, and it significantly impacted
performance (on top of that PR's performance hit):
<details>
<summary>17043 benchmarks</summary>

### With `String`

![error_handling_insert_slow](https://github.com/user-attachments/assets/5629ba6d-69fc-4c16-84c9-8be7e449232d)

### No `String`

![error_handling_insert_fixed](https://github.com/user-attachments/assets/6393e2d6-e61a-4558-8ff1-471ff8356c1c)

</details>

For that PR, I just removed the error details entirely, but I figured
I'd try to find a way to keep them around.

## Solution

- Replace the `String` with a helper struct that holds the location, and
only turn it into a string when someone actually wants to print it.
- Replace the `UnsafeWorldCell` with the aforementioned struct.
- Do the same for `QueryEntityError::NoSuchEntity`.

## Benchmarking

This had some interesting performance impact:

<details>
<summary>This PR vs main</summary>


![dne_rework_1](https://github.com/user-attachments/assets/05bf91b4-dddc-4d76-b2c4-41c9d25c7a57)

![dne_rework_2](https://github.com/user-attachments/assets/34aa76b2-d8a7-41e0-9670-c213207e457d)

![dne_rework_3](https://github.com/user-attachments/assets/8b9bd4e4-77c8-45a7-b058-dc0dfd3dd323)

</details>

## Other work

`QueryEntityError::QueryDoesNotMatch` also has an `UnsafeWorldCell`
inside it. This one would be more complicated to rework while keeping
the same functionality.

## Migration Guide

The errors `EntityFetchError::NoSuchEntity` and
`QueryEntityError::NoSuchEntity` now contain an
`EntityDoesNotExistDetails` struct instead of an `UnsafeWorldCell`. If
you were just printing these, they should work identically.

---------

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
2025-01-05 02:01:01 +00:00
Rob Parrett
651b22f31f
Update typos (#17126)
# Objective

Use the latest version of `typos` and fix the typos that it now detects

# Additional Info

By the way, `typos` has a "low priority typo suggestions issue" where we
can throw typos we find that `typos` doesn't catch.

(This link may go stale) https://github.com/crate-ci/typos/issues/1200
2025-01-03 17:44:26 +00:00
Zachary Harrold
0403948aa2
Remove Implicit std Prelude from no_std Crates (#17086)
# Background

In `no_std` compatible crates, there is often an `std` feature which
will allow access to the standard library. Currently, with the `std`
feature _enabled_, the
[`std::prelude`](https://doc.rust-lang.org/std/prelude/index.html) is
implicitly imported in all modules. With the feature _disabled_, instead
the [`core::prelude`](https://doc.rust-lang.org/core/prelude/index.html)
is implicitly imported. This creates a subtle and pervasive issue where
`alloc` items _may_ be implicitly included (if `std` is enabled), or
must be explicitly included (if `std` is not enabled).

# Objective

- Make the implicit imports for `no_std` crates consistent regardless of
what features are/not enabled.

## Solution

- Replace the `cfg_attr` "double negative" `no_std` attribute with
conditional compilation to _include_ `std` as an external crate.
```rust
// Before
#![cfg_attr(not(feature = "std"), no_std)]

// After
#![no_std]

#[cfg(feature = "std")]
extern crate std;
```
- Fix imports that are currently broken but are only now visible with
the above fix.

## Testing

- CI

## Notes

I had previously used the "double negative" version of `no_std` based on
general consensus that it was "cleaner" within the Rust embedded
community. However, this implicit prelude issue likely was considered
when forming this consensus. I believe the reason why is the items most
affected by this issue are provided by the `alloc` crate, which is
rarely used within embedded but extensively used within Bevy.
2025-01-03 01:58:43 +00:00
arunke
22bf3b9a62
Fix documentation for system set method (#17106)
# Objective

Fix incorrect comment on `IntoSystemSetConfigs::after` likely caused by
copy-paste error. It said "before" instead of "after".

## Solution

Update the comment to the correct text.

## Testing

CI tests pass. This is just updating a comment.
2025-01-02 22:38:26 +00:00
ickshonpe
2931e350b6
reduce nesting in the sparse_set module (#17066)
# Objective

Just being fussy but I hate this `.map(|v|
v.is_some()).unwrap_or(false)` stuff.

## Solution
Reduce it using `and_then`.

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2025-01-01 23:17:11 +00:00
Sean Kim
294e0db719
Rename track_change_detection flag to track_location (#17075)
# Objective

- As stated in the related issue, this PR is to better align the feature
flag name with what it actually does and the plans for the future.
- Fixes #16852 

## Solution

- Simple find / replace

## Testing

- Local run of `cargo run -p ci`

## Migration Guide

The `track_change_detection` feature flag has been renamed to
`track_location` to better reflect its extended capabilities.
2025-01-01 18:43:47 +00:00
ickshonpe
c817864e4f
Replace map_or(true, _) with is_none_or(_) (#17073)
# Objective

Replace uses of `map_or(true, _)` with the equivalent `is_none_or(_)`.
2025-01-01 04:05:26 +00:00
ickshonpe
7a5a734452
Replace map + unwrap_or(false) with is_some_and (#17067)
# Objective

The `my_option.map(|inner| inner.is_whatever).unwrap_or(false)` pattern
is fragile and ugly.

Replace it with `is_some_and` everywhere.
2024-12-31 20:28:02 +00:00
ickshonpe
c73daea341
Replace map + unwrap_or(true) with is_none_or (#17070)
# Objective

Reduce all varieties of `my_maybe.map(|x| x.is_true).unwrap_or(true)`
using `is_none_or`.
2024-12-31 20:17:03 +00:00
Jonathan Chan Kwan Yin
3c7fbee2d8
Add Mut::clone_from_if_neq (#17019)
# Objective

- Support more ergonomic conditional updates for types that can be
modified by `clone_into`.

## Solution

- Use `ToOwned::clone_into` to copy a reference provided by the caller
in `Mut::clone_from_if_neq`.

## Testing

- See doc tests.
2024-12-31 16:23:01 +00:00
Mike
ac43d5c94f
Convert to fallible system in IntoSystemConfigs (#17051)
# Objective

- #16589 added an enum to switch between fallible and infallible system.
This branching should be unnecessary if we wrap infallible systems in a
function to return `Ok(())`.

## Solution

- Create a wrapper system for `System<(), ()>`s that returns `Ok` on the
call to `run` and `run_unsafe`. The wrapper should compile out, but I
haven't checked.
- I removed the `impl IntoSystemConfigs for BoxedSystem<(), ()>` as I
couldn't figure out a way to keep the impl without double boxing.

## Testing

- ran `many_foxes` example to check if it still runs.

## Migration Guide

- `IntoSystemConfigs` has been removed for `BoxedSystem<(), ()>`. Either
use `InfallibleSystemWrapper` before boxing or make your system return
`bevy::ecs::prelude::Result`.
2024-12-31 00:39:29 +00:00
Zachary Harrold
3f19997096
Added modify_component to EntityWorldMut, DeferredWorld, and World (#16668)
# Objective

- Make working with immutable components more ergonomic
- Assist #16662

## Solution

Added `modify_component` to `World` and `EntityWorldMut`. This method
"removes" a component from an entity, gives a mutable reference to it to
a provided closure, and then "re-inserts" the component back onto the
entity. This replacement triggers the `OnReplace` and `OnInsert` hooks,
but does _not_ cause an archetype move, as the removal is purely
simulated.

## Testing

- Added doc-tests and a unit test.

---

## Showcase

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

/// An immutable component.
#[derive(Component, PartialEq, Eq, Debug)]
#[component(immutable)]
struct Foo(bool);

let mut world = World::default();

let mut entity = world.spawn(Foo(false));

assert_eq!(entity.get::<Foo>(), Some(&Foo(false)));

// Before the closure is executed, the `OnReplace` hooks/observers are triggered
entity.modify_component(|foo: &mut Foo| {
    foo.0 = true;
});
// After the closure is executed, `OnInsert` hooks/observers are triggered

assert_eq!(entity.get::<Foo>(), Some(&Foo(true)));
```

## Notes

- If the component is not available on the entity, the closure and hooks
aren't executed, and `None` is returned. I chose this as an alternative
to returning an error or panicking, but I'm open to changing that based
on feedback.
- This relies on `unsafe`, in particular for accessing the `Archetype`
to trigger hooks. All the unsafe operations are contained within
`DeferredWorld::modify_component`, and I would appreciate that this
function is given special attention to ensure soundness.
- The `OnAdd` hook can never be triggered by this method, since the
component must already be inserted. I have chosen to not trigger
`OnRemove`, as I believe it makes sense that this method is purely a
replacement operation, not an actual removal/insertion.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Malek <50841145+MalekiRe@users.noreply.github.com>
2024-12-31 00:23:44 +00:00
Benjamin Brienen
4460a4d9ed
Use -D warnings in all relevant CI (#17011)
# Objective

Fixes #17009

See:
https://doc.rust-lang.org/stable/clippy/continuous_integration/index.html

## Solution

Add the env

## Testing

CI should start to fail, then I'll fix it.

## Showcase


![image](https://github.com/user-attachments/assets/acd2888f-9fc0-445a-a96a-842ba9f1c6aa)
2024-12-31 00:15:28 +00:00
Zhixing Zhang
9cebc66486
Make 8 methods public and updated input parameter generics for SystemState::build_system_with_input (#17034)
# Objective

- Made certain methods public for advanced use cases. Methods that
returns mutable references are marked as unsafe due to the possibility
of violating internal lifetime constraint assumptions.
- Fixes an issue introduced by #15184
2024-12-30 23:04:14 +00:00
Vic
b78efd339d
Simplify sort/max_by calls (#17048)
# Objective

Some sort calls and `Ord` impls are unnecessarily complex.

## Solution

Rewrite the "match on cmp, if equal do another cmp" as either a
comparison on tuples, or `Ordering::then_with`, depending on whether the
compare keys need construction.

`sort_by` -> `sort_by_key` when symmetrical. Do the same for
`min_by`/`max_by`.

Note that `total_cmp` can only work with `sort_by`, and not on tuples.

When sorting collected query results that contain
`Entity`/`MainEntity`/`RenderEntity` in their `QueryData`, with that
`Entity` in the sort key:
stable -> unstable sort (all queried entities are unique)

If key construction is not simple, switch to `sort_by_cached_key` when
possible.

Sorts that are only performed to discover the maximal element are
replaced by `max_by_key`.

Dedicated comparison functions and structs are removed where simple.

Derive `PartialOrd`/`Ord` when useful.

Misc. closure style inconsistencies.

## Testing
- Existing tests.
2024-12-30 22:59:36 +00:00
JaySpruce
9ac7e17f2e
Refactor hierarchy-related commands to remove structs (#17029)
## Objective

Continuation of #16999.

This PR handles the following:
- Many hierarchy-related commands are wrappers around `World` and
`EntityWorldMut` methods and can be moved to closures:
  - `AddChild`
  - `InsertChildren`
  - `AddChildren`
  - `RemoveChildren`
  - `ClearChildren`
  - `ReplaceChildren`
  - `RemoveParent`
  - `DespawnRecursive`
  - `DespawnChildrenRecursive`
  - `AddChildInPlace`
  - `RemoveParentInPlace`
- `SendEvent` is a wrapper around `World` methods and can be moved to a
closure (and its file deleted).

## Migration Guide

If you were queuing the structs of hierarchy-related commands or
`SendEvent` directly, you will need to change them to the methods
implemented on `EntityCommands` (or `Commands` for `SendEvent`):

| Struct | Method |

|--------------------------------------------------------------------|---------------------------------------------------------------------------------------------|
| `commands.queue(AddChild { child, parent });` |
`commands.entity(parent).add_child(child);` OR
`commands.entity(child).set_parent(parent);` |
| `commands.queue(AddChildren { children, parent });` |
`commands.entity(parent).add_children(children);` |
| `commands.queue(InsertChildren { children, parent });` |
`commands.entity(parent).insert_children(children);` |
| `commands.queue(RemoveChildren { children, parent });` |
`commands.entity(parent).remove_children(children);` |
| `commands.queue(ReplaceChildren { children, parent });` |
`commands.entity(parent).replace_children(children);` |
| `commands.queue(ClearChildren { parent });` |
`commands.entity(parent).clear_children();` |
| `commands.queue(RemoveParent { child });` |
`commands.entity(child).remove_parent()` |
| `commands.queue(DespawnRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_recursive();` |
| `commands.queue(DespawnRecursive { entity, warn: false });` |
`commands.entity(entity).try_despawn_recursive();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: true });` |
`commands.entity(entity).despawn_descendants();` |
| `commands.queue(DespawnChildrenRecursive { entity, warn: false});` |
`commands.entity(entity).try_despawn_descendants();` |
| `commands.queue(SendEvent { event });` | `commands.send_event(event);`
|
2024-12-30 20:58:03 +00:00
Christian Hughes
b09bbfa905
Remove unsound Clone impl for EntityMutExcept (#17032)
# Objective

`EntityMutExcept` can currently be cloned, which can easily violate
aliasing rules.

## Solution

- Remove the `Clone` impl for `EntityMutExcept`
- Also manually derived `Clone` impl for `EntityRefExcept` so that `B:
Clone` isn't required, and also impl'd `Copy`

## Testing

Compile failure tests would be good for this, but I'm not exactly sure
how to set that up.

## Migration Guide

- `EntityMutExcept` can no-longer be cloned, as this violates Rust's
memory safety rules.
2024-12-30 05:17:46 +00:00
Zachary Harrold
46af46695b
Add no_std support to bevy_input (#16995)
# Objective

- Contributes to #15460

## Solution

- Added the following features:
  - `std` (default)
  - `smol_str` (default)
  - `portable-atomic`
  - `critical-section`
  - `libm`
- Fixed an existing issue where `bevy_reflect` wasn't properly feature
gated.

## Testing

- CI

## Notes

- There were some minor issues with `bevy_math` and `bevy_ecs` noticed
in this PR which I have also resolved here. I can split these out if
desired, but I've left them here for now as they're very small changes
and I don't consider this PR itself to be very controversial.
- `libm`, `portable-atomic`, and `critical-section` are shortcuts to
enable the relevant features in dependencies, making the usage of this
crate on atomically challenged platforms possible and simpler.
- `smol_str` is gated as it doesn't support atomically challenged
platforms (e.g., Raspberry Pi Pico). I have an issue and a
[PR](https://github.com/rust-analyzer/smol_str/pull/91) to discuss this
upstream.
2024-12-29 22:46:30 +00:00
JaySpruce
0f2b2de333
Move some structs that impl Command to methods on World and EntityWorldMut (#16999)
## Objective

Commands were previously limited to structs that implemented `Command`.
Now there are blanket implementations for closures, which (in my
opinion) are generally preferable.

Internal commands within `commands/mod.rs` have been switched from
structs to closures, but there are a number of internal commands in
other areas of the engine that still use structs. I'd like to tidy these
up by moving their implementations to methods on
`World`/`EntityWorldMut` and changing `Commands` to use those methods
through closures.

This PR handles the following:
- `TriggerEvent` and `EmitDynamicTrigger` double as commands and helper
structs, and can just be moved to `World` methods.
- Four structs that enabled insertion/removal of components via
reflection. This functionality shouldn't be exclusive to commands, and
can be added to `EntityWorldMut`.
- Five structs that mostly just wrapped `World` methods, and can be
replaced with closures that do the same thing.

## Solution

- __Observer Triggers__ (`observer/trigger_event.rs` and
`observer/mod.rs`)
- Moved the internals of `TriggerEvent` to the `World` methods that used
it.
  - Replaced `EmitDynamicTrigger` with two `World` methods:
    - `trigger_targets_dynamic`
    - `trigger_targets_dynamic_ref`
- `TriggerTargets` was now the only thing in
`observer/trigger_event.rs`, so it's been moved to `observer/mod.rs` and
`trigger_event.rs` was deleted.
- __Reflection Insert/Remove__ (`reflect/entity_commands.rs`)
- Replaced the following `Command` impls with equivalent methods on
`EntityWorldMut`:
    - `InsertReflect` -> `insert_reflect`
    - `InsertReflectWithRegistry` -> `insert_reflect_with_registry`
    - `RemoveReflect` -> `remove_reflect`
    - `RemoveReflectWithRegistry` -> `remove_reflect_with_registry`
- __System Registration__ (`system/system_registry.rs`)
- The following `Command` impls just wrapped a `World` method and have
been replaced with closures:
    - `RunSystemWith`
    - `UnregisterSystem`
    - `RunSystemCachedWith`
    - `UnregisterSystemCached`
- `RegisterSystem` called a helper function that basically worked as a
constructor for `RegisteredSystem` and made sure it came with a marker
component. That helper function has been replaced with
`RegisteredSystem::new` and a `#[require]`.

## Possible Addition

The extension trait that adds the reflection commands,
`ReflectCommandExt`, isn't strictly necessary; we could just `impl
EntityCommands`. We could even move them to the same files as the main
impls and put it behind a `#[cfg]`.

The PR that added it [had a similar
conversation](https://github.com/bevyengine/bevy/pull/8895#discussion_r1234713671)
and decided to stick with the trait, but we could revisit it here if so
desired.
2024-12-29 22:18:53 +00:00
Benjamin Brienen
8c34f00deb
Fix msrvs (#17012)
# Objective

The rust-versions are out of date.
Fixes #17008

## Solution

Update the values

Cherry-picked from #17006 in case it is controversial

## Testing

Validated locally and in #17006

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-12-29 20:00:19 +00:00
Benjamin Brienen
847c3a1719
Fix random clippy warning (#17010)
# Objective

Follow-up to #16984 

## Solution

Fix the lint

## Testing

```
PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.71s
PS C:\Users\BenjaminBrienen\source\bevy> cargo clippy -p bevy_ecs
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.21s
```
2024-12-29 19:28:59 +00:00
Benjamin Brienen
64efd08e13
Prefer Display over Debug (#16112)
# Objective

Fixes #16104

## Solution

I removed all instances of `:?` and put them back one by one where it
caused an error.

I removed some bevy_utils helper functions that were only used in 2
places and don't add value. See: #11478

## Testing

CI should catch the mistakes

## Migration Guide

`bevy::utils::{dbg,info,warn,error}` were removed. Use
`bevy::utils::tracing::{debug,info,warn,error}` instead.

---------

Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
2024-12-27 00:40:06 +00:00
François Mockers
394e82f4bc
panic on system error (#16979)
# Objective

- First step for #16718 
- #16589 introduced an api that can only ignore errors, which is risky

## Solution

- Panic instead of just ignoring the errors

## Testing

- Changed the `fallible_systems` example to return an error
```
Encountered an error in system `fallible_systems::setup`: TooManyVertices { subdivisions: 300, number_of_resulting_points: 906012 }
Encountered a panic in system `fallible_systems::setup`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
```
2024-12-26 23:44:46 +00:00
Benjamin Brienen
1675d68366
Fix beta CI (#16984)
# Objective

Fixes #16607

## Solution

Satisfy clippy

## Testing

Ran clippy
2024-12-26 22:17:51 +00:00
JaySpruce
1669ca676a
Remove vestigial helper functions for Commands and EntityCommands (#16936)
## Objective

I believe these started as structs, back when that was how commands had
to be implemented. Now they just hide implementation details.

## Solution

Remove the helper functions and move each implementation into its
respective method, except for the ones that actually reduce code
duplication.
2024-12-24 03:07:28 +00:00
Vic
5b899dcc3a
impl EntityBorrow for more types (#16917)
# Objective

Some types like `RenderEntity` and `MainEntity` are just wrappers around
`Entity`, so they should be able to implement
`EntityBorrow`/`TrustedEntityBorrow`. This allows using them with
`EntitySet` functionality.
The `EntityRef` family are more than direct wrappers around `Entity`,
but can still benefit from being unique in a collection.

## Solution

Implement `EntityBorrow` and `TrustedEntityBorrow` for simple `Entity`
newtypes and `EntityRef` types.
These impls are an explicit decision to have the `EntityRef` types
compare like just `Entity`.
`EntityWorldMut` is omitted from this impl, because it explicitly
contains a `&mut World` as well, and we do not ever use more than one at
a time.

Add `EntityBorrow` to the `bevy_ecs` prelude.

## Migration Guide

`NormalizedWindowRef::entity` has been replaced with an
`EntityBorrow::entity` impl.
2024-12-24 02:47:03 +00:00
MiniaczQ
460de77a55
Set panic as default fallible system param behavior (#16638)
# Objective

Fixes: #16578

## Solution

This is a patch fix, proper fix requires a breaking change.

Added `Panic` enum variant and using is as the system meta default.
Warn once behavior can be enabled same way disabling panic (originally
disabling wans) is.

To fix an issue with the current architecture, where **all** combinator
system params get checked together,
combinator systems only check params of the first system.
This will result in old, panicking behavior on subsequent systems and
will be fixed in 0.16.

## Testing

Ran unit tests and `fallible_params` example.

---------

Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-12-24 02:36:03 +00:00
Oliver Maskery
022c6b1d34
Prevent creation of superfluous empty table (#16935)
# Objective

- To fix a tiny bug in `bevy_ecs::storage::Tables` that, in one case,
means it accidentally allocates an additional "empty" `Table`, resulting
in two "empty" `Table`s:
- The one pre-allocated empty table at index 0 whose index is designed
to match up with `TableId::empty()`
- One extra empty table, at some non-0 index, that does not match up
with `TableId::empty()`.
- This PR aims to prevent this extraneous `Table`, ensuring that
entities with no components in table-storage reliably have their
archetype's table ID be equal to `TableId::empty()`.

## Solution

### Background

The issue occurs because:

- `Tables` contains:
- `tables: Vec<Table>` - The set of all `Table`s allocated in the world.
- `table_ids: HashMap<Box<[ComponentId]>, TableId>` - An index to
rapidly lookup the `Table` in `tables` by a set of `ComponentId`s.
- When `Tables` is constructed it pre-populates the `tables` `Vec` with
an empty `Table`.
- This ensures that the first entry (index 0) is always the `Table` for
entities with no components in table storage.
- In particular, `TableId::empty()` is a utility that returns a
`TableId` of `0`.
- However, the `table_ids` map is not initialised to associate an empty
`[ComponentId]` with `TableId` `0`.
- This means, the first time a structural change tries to access a
`Table` for an archetype with 0 table components:
  - `Tables::get_id_or_insert` is used to retrieve the target `Table`
- The function attempts to lookup the entry in the `table_ids` `HashMap`
whose key is the empty `ComponentId` set
- The empty `Table` created at startup won't be found, because it was
never inserted into `table_ids`
- It will instead create a new table, insert it into the `HashMap`
(preventing further instances of this issue), and return it.

### Changes

- I considered simply initialising the `table_ids` `HashMap` to know
about the pre-allocated `Table`
- However, I ended up using the proposed solution discussed on Discord
[#ecs-dev](https://discord.com/channels/691052431525675048/749335865876021248/1320430933152759958):
- Make `Tables::get_id_or_insert` simply early-exit if the requested
`component_ids` was empty.
- This avoids unnecessarily hashing the empty slice and looking it up in
the `HashMap`.
- The `table_ids` `HashMap` is not exposed outside this struct, and is
only used within `get_id_or_insert`, so it seems wasteful to defensively
populate it with the empty `Table`.

## Testing

This is my first Bevy contribution, so I don't really know the processes
that well. That said:
- I have introduced a little test that exercises the original issue and
shows that it is now resolved.
- I have run the `bevy_ecs` tests locally, so I have reasonable
confidence I haven't broken that.
- I haven't run any further test suites, mostly as when I tried to run
test suites for the whole project it filled my entire SSD with >600GB of
target directory output 😱😱😱
2024-12-22 23:04:32 +00:00