Commit graph

7286 commits

Author SHA1 Message Date
vero
4a23dc4216
Split out bevy_mesh from bevy_render (#15666)
# Objective

- bevy_render is gargantuan

## Solution

- Split out bevy_mesh

## Testing

- Ran some examples, everything looks fine

## Migration Guide

`bevy_render::mesh::morph::inherit_weights` is now
`bevy_render::mesh::inherit_weights`

if you were using `Mesh::compute_aabb`, you will need to `use
bevy_render::mesh::MeshAabb;` now

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-10-06 14:18:11 +00:00
Chang Guo
7c4a0683c7
Add with_headers() method to RemoteHttpPlugin (#15651)
# Objective

- fulfill the needs presented in this issue, which requires the ability
to set custom HTTP headers for responses in the Bevy Remote Protocol
server. #15551

## Solution

- Created a `Headers` struct to store custom HTTP headers as key-value
pairs.
- Added a `headers` field to the `RemoteHttpPlugin` struct.
- Implemented a `with_headers` method in `RemoteHttpPlugin` to allow
users to set custom headers.
- Passed the headers into the processing chain.

## Testing

- I added cors_headers in example/remote/server.rs and tested it with a
static html
[file](https://github.com/spacemen0/bevy/blob/test_file/test.html)

---
2024-10-06 13:21:21 +00:00
poopy
d9190e4ff6
Add Support for Triggering Events via AnimationEvents (#15538)
# Objective

Add support for events that can be triggered from animation clips. This
is useful when you need something to happen at a specific time in an
animation. For example, playing a sound every time a characters feet
hits the ground when walking.

Closes #15494 

## Solution

Added a new field to `AnimationClip`: `events`, which contains a list of
`AnimationEvent`s. These are automatically triggered in
`animate_targets` and `trigger_untargeted_animation_events`.

## Testing

Added a couple of tests and example (`animation_events.rs`) to make sure
events are triggered when expected.

---

## Showcase

`Events` need to also implement `AnimationEvent` and `Reflect` to be
used with animations.

```rust
#[derive(Event, AnimationEvent, Reflect)]
struct SomeEvent;
```

Events can be added to an `AnimationClip` by specifying a time and
event.

```rust
// trigger an event after 1.0 second
animation_clip.add_event(1.0, SomeEvent);
```

And optionally, providing a target id.

```rust
let id = AnimationTargetId::from_iter(["shoulder", "arm", "hand"]);
animation_clip.add_event_to_target(id, 1.0, HandEvent);
```

I modified the `animated_fox` example to show off the feature.

![CleanShot 2024-10-05 at 02 41
57](https://github.com/user-attachments/assets/0bb47db7-24f9-4504-88f1-40e375b89b1b)

---------

Co-authored-by: Matty <weatherleymatthew@gmail.com>
Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-10-06 10:03:05 +00:00
Vlady Veselinov
856cab56f9
Fix wrong link in error (#15672)
Hi y'all, I got an error that leads to a wrong link:
https://bevyengine.org/learn/errors/#b0002

It should be: https://bevyengine.org/learn/errors/b0002


![image](https://github.com/user-attachments/assets/863ae8cd-6a3b-4830-b125-2944a211a737)
2024-10-06 08:14:50 +00:00
Tom Frantz
0305f2edc0
Fix doc comment (#15673)
# Objective

I noticed a weird break in a doc comment, I assume it must be a typo.

## Solution

Put the missing doc comment in there.

## Testing

It looks better in my IDE now
2024-10-06 08:12:58 +00:00
ickshonpe
92f39354a3
Display the bounds of each text node in the text_debug ui example (#15622)
# Objective

Add a background colour to each text node in the `text_debug` example to
visualize their bounds.

## Showcase

<img width="961" alt="deb"
src="https://github.com/user-attachments/assets/deec3e15-b0f0-411f-9af1-597587ac2a83">

In the bottom right you can see the empty space at the bottom of the
text node, making it much more obvious that there is a bug causing the
size of the bounds to be calculated incorrectly.
2024-10-05 22:48:57 +00:00
UkoeHB
0b5a360585
Fix text measurement when multiple font sizes are present (#15669)
# Objective

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

## Solution

- Add up line heights to get text block height instead of using
`Metrics`, which only records the largest line height.

## Testing

- [x] Fixed issue shown in https://github.com/bevyengine/bevy/pull/15622
2024-10-05 22:46:37 +00:00
mgi388
7c03ca2562
Fix QuerySingle -> Single missed in example (#15667)
Missed in https://github.com/bevyengine/bevy/pull/15507
2024-10-05 22:24:03 +00:00
Matty
42e0771633
Fix additive blending of quaternions (#15662)
# Objective

Fixes #13832

## Solution

Additively blend quaternions like this:
```rust
rotation = Quat::slerp(Quat::IDENTITY, incoming_rotation, weight) * rotation;
```

## Testing

Ran `animation_masks`, which behaves the same as before. (In the context
of an animation being blended only onto the base pose, there is no
difference.)

We should create some examples that actually exercise more of the
capabilities of the `AnimationGraph` so that issues like this can become
more visible in general. (On the other hand, I'm quite certain this was
wrong before.)

## Migration Guide

This PR changes the implementation of `Quat: Animatable`, which was not
used internally by Bevy prior to this release version. If you relied on
the old behavior of additive quaternion blending in manual applications,
that code will have to be updated, as the old behavior was incorrect.
2024-10-05 20:03:10 +00:00
Joona Aalto
25bfa80e60
Migrate cameras to required components (#15641)
# Objective

Yet another PR for migrating stuff to required components. This time,
cameras!

## Solution

As per the [selected
proposal](https://hackmd.io/tsYID4CGRiWxzsgawzxG_g#Combined-Proposal-1-Selected),
deprecate `Camera2dBundle` and `Camera3dBundle` in favor of `Camera2d`
and `Camera3d`.

Adding a `Camera` without `Camera2d` or `Camera3d` now logs a warning,
as suggested by Cart [on
Discord](https://discord.com/channels/691052431525675048/1264881140007702558/1291506402832945273).
I would personally like cameras to work a bit differently and be split
into a few more components, to avoid some footguns and confusing
semantics, but that is more controversial, and shouldn't block this core
migration.

## Testing

I ran a few 2D and 3D examples, and tried cameras with and without
render graphs.

---

## Migration Guide

`Camera2dBundle` and `Camera3dBundle` have been deprecated in favor of
`Camera2d` and `Camera3d`. Inserting them will now also insert the other
components required by them automatically.
2024-10-05 01:59:52 +00:00
m-edlund
ac9b0c848c
fix: corrected projection calculation of camera sub view (#15646)
# Objective

- Fixes #15600

## Solution

- The projection calculations did not use the aspect ratio of
`full_size`, this was amended

## Testing

- I created a test example on [this
fork](https://github.com/m-edlund/bevy/tree/bug/main/subViewProjectionBroken)
to allow testing with different aspect ratios and offsets
- The sub view is bound to a view port that can move across the screen.
The image in the moving sub view should "match" the full image exactly

## Showcase

Current version:


https://github.com/user-attachments/assets/17ad1213-d5ae-4181-89c1-81146edede7d

Fixed version:


https://github.com/user-attachments/assets/398e0927-e1dd-4880-897d-8157aa4398e6
2024-10-05 01:36:47 +00:00
urben1680
2e89e98931
Deprecate Events::oldest_id (#15658)
# Objective

Fixes #15617 

## Solution

The original author confirmed it was not intentional that both these
methods exist.

They do the same, one has the better implementation and the other the
better name.

## Testing

I just ran the unit tests of the module.

---

## Migration Guide

- Change usages of `Events::oldest_id` to `Events::oldest_event_count`
- If `Events::oldest_id` was used to get the actual oldest
`EventId::id`, note that the deprecated method never reliably did that
in the first place as the buffers may contain no id currently.
2024-10-05 01:35:44 +00:00
Kristoffer Søholm
ddd4b4daf8
Fix deferred rendering (#15656)
# Objective

Fixes #15525

The deferred and mesh pipelines tonemapping LUT bindings were
accidentally out of sync, breaking deferred rendering.

As noted in the issue it's still broken on wasm due to hitting a texture
limit.

## Solution

Add constants for these instead of hardcoding them.

## Testing

Test with `cargo run --example deferred_rendering` and see it works, run
the same on main and see it crash.
2024-10-04 22:51:23 +00:00
Patrick Walton
0094bcbc07
Implement additive blending for animation graphs. (#15631)
*Additive blending* is an ubiquitous feature in game engines that allows
animations to be concatenated instead of blended. The canonical use case
is to allow a character to hold a weapon while performing arbitrary
poses. For example, if you had a character that needed to be able to
walk or run while attacking with a weapon, the typical workflow is to
have an additive blend node that combines walking and running animation
clips with an animation clip of one of the limbs performing a weapon
attack animation.

This commit adds support for additive blending to Bevy. It builds on top
of the flexible infrastructure in #15589 and introduces a new type of
node, the *add node*. Like blend nodes, add nodes combine the animations
of their children according to their weights. Unlike blend nodes,
however, add nodes don't normalize the weights to 1.0.

The `animation_masks` example has been overhauled to demonstrate the use
of additive blending in combination with masks. There are now controls
to choose an animation clip for every limb of the fox individually.

This patch also fixes a bug whereby masks were incorrectly accumulated
with `insert()` during the graph threading phase, which could cause
corruption of computed masks in some cases.

Note that the `clip` field has been replaced with an `AnimationNodeType`
enum, which breaks `animgraph.ron` files. The `Fox.animgraph.ron` asset
has been updated to the new format.

Closes #14395.

## Showcase


https://github.com/user-attachments/assets/52dfe05f-fdb3-477a-9462-ec150f93df33

## Migration Guide

* The `animgraph.ron` format has changed to accommodate the new
*additive blending* feature. You'll need to change `clip` fields to
instances of the new `AnimationNodeType` enum.
2024-10-04 22:13:22 +00:00
vero
7eadc1d467
Zero Copy Mesh (#15569)
# Objective

- Another step towards #15558

## Solution

- Instead of allocating a Vec and then having wgpu copy it into a
staging buffer, write directly into the staging buffer.
- gets rid of another hidden copy, in `pad_to_alignment`.

future work:
- why is there a gcd implementation in here (and its subpar, use
binary_gcd. its in the hot path, run twice for every mesh, every frame i
think?) make it better and put it in bevy_math
- zero-copy custom mesh api to avoid having to write out a Mesh from a
custom rep

## Testing

- lighting and many_cubes run fine (and slightly faster. havent
benchmarked though)

---

## Showcase

- look ma... no copies

at least when RenderAssetUsage is GPU only :3

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
2024-10-04 21:24:44 +00:00
vero
8b0388c74a
Split off bevy_image from bevy_render (#15650)
# Objective

- bevy_render is gargantuan

## Solution

- Split off bevy_image

## Testing

- Ran some examples
2024-10-04 20:16:47 +00:00
Zachary Harrold
53adcd7667
Minor fixes for bevy_utils in no_std (#15463)
# Objective

- Contributes to #15460

## Solution

- Made `web-time` a `wasm32`-only dependency.
- Moved time-related exports to its own module for clarity.
- Feature-gated allocator requirements for `hashbrown` behind `alloc`.
- Enabled compile-time RNG for `ahash` (runtime RNG will preferentially
used in `std` environments)
- Made `thread_local` optional by feature-gating the `Parallel` type.

## Testing

- Ran CI locally.
- `cargo build -p bevy_utils --target "x86_64-unknown-none"
--no-default-features`
2024-10-04 19:25:49 +00:00
Eero Lehtinen
d0edbdac78
Fix cargo-ndk build command (#15648)
# Objective

- Fix cargo-ndk build command documentation in readme.

```sh
❯ cargo ndk -t arm64-v8a build -o android_example/app/src/main/jniLibs
    Building arm64-v8a (aarch64-linux-android)
error: unexpected argument '-o' found
```

## Solution

- Move "build" to the end of the command.

## Testing

- With the new command order building works.
```sh
❯ cargo ndk -t arm64-v8a -o android_example/app/src/main/jniLibs build
    Building arm64-v8a (aarch64-linux-android)
   Compiling bevy_ptr v0.15.0-dev (/home/eero/repos/bevy/crates/bevy_ptr)
   Compiling bevy_macro_utils v0.15.0-dev (/home/eero/repos/bevy/crates/bevy_macro_utils)
   Compiling event-listener v5.3.1

... rest of compilation ...
```
2024-10-04 19:20:25 +00:00
robtfm
e72b9625d7
drop info locks in single threaded (#15522)
# Objective

addresses half of issue #15508
avoid asset server deadlock when `multi_threaded` feature is not
enabled.

## Solution

drop the locks in the single-threaded case.

the lock is still held with the `multi-threaded` feature enabled to
avoid re-locking to insert the load task. i guess this might possibly
cause issues on single-core machines ... is that something we should
worry about?

---------

Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com>
Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
Co-authored-by: Emerson Coskey <56370779+ecoskey@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Tim <JustTheCoolDude@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: s-puig <39652109+s-puig@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: Liam Gallagher <liam@liamgallagher.dev>
Co-authored-by: Matty <weatherleymatthew@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
Co-authored-by: charlotte <charlotte.c.mcelwain@gmail.com>
Co-authored-by: akimakinai <105044389+akimakinai@users.noreply.github.com>
Co-authored-by: Antony <antony.m.3012@gmail.com>
Co-authored-by: JohnTheCoolingFan <43478602+JohnTheCoolingFan@users.noreply.github.com>
Co-authored-by: hshrimp <182684536+hooded-shrimp@users.noreply.github.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Dokkae <90514461+Dokkae6949@users.noreply.github.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Sou1gh0st <xiuyutong1994@163.com>
Co-authored-by: Robert Walter <26892280+RobWalt@users.noreply.github.com>
Co-authored-by: eckz <567737+eckz@users.noreply.github.com>
Co-authored-by: Matty <2975848+mweatherley@users.noreply.github.com>
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Co-authored-by: Giacomo Stevanato <giaco.stevanato@gmail.com>
Co-authored-by: Clar Fon <15850505+clarfonthey@users.noreply.github.com>
Co-authored-by: andriyDev <andriydzikh@gmail.com>
Co-authored-by: TheBigCheese <32036861+13ros27@users.noreply.github.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
Co-authored-by: Josh Robson Chase <josh@robsonchase.com>
Co-authored-by: Erik Živković <erik@zivkovic.se>
Co-authored-by: ChosenName <69129796+ChosenName@users.noreply.github.com>
Co-authored-by: mgi388 <135186256+mgi388@users.noreply.github.com>
Co-authored-by: SpecificProtagonist <specificprotagonist@posteo.org>
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Co-authored-by: Gabriel Bourgeois <gabriel.bourgeoisv4si@gmail.com>
Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>
Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
Co-authored-by: re0312 <re0312@outlook.com>
Co-authored-by: re0312 <45868716+re0312@users.noreply.github.com>
Co-authored-by: Periwink <charlesbour@gmail.com>
Co-authored-by: Anselmo Sampietro <ans.samp@gmail.com>
Co-authored-by: rudderbucky <anandkwork7@gmail.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
Co-authored-by: Andreas <34456840+nilsiker@users.noreply.github.com>
Co-authored-by: Ludwig DUBOS <ludwig.dubos@pm.me>
Co-authored-by: Ensar Sarajčić <dev@ensarsarajcic.com>
Co-authored-by: Kanabenki <lucien.menassol@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Co-authored-by: m-edlund <me@fwegmann.com>
Co-authored-by: vero <rodol@rivalrebels.com>
Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Litttle_fish <38809254+Litttlefish@users.noreply.github.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
Co-authored-by: Rich Churcher <rich.churcher@gmail.com>
Co-authored-by: Viktor Gustavsson <villor94@gmail.com>
Co-authored-by: Dragoș Tiselice <dragostiselice@gmail.com>
Co-authored-by: Miles Silberling-Cook <NthTensor@users.noreply.github.com>
Co-authored-by: notmd <33456881+notmd@users.noreply.github.com>
Co-authored-by: Matt Tracy <matt.r.tracy@gmail.com>
Co-authored-by: Patrick Walton <pcwalton@mimiga.net>
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
Co-authored-by: rewin <rewin1996@gmail.com>
Co-authored-by: a.yamaev <a.yamaev@smartengines.com>
Co-authored-by: fluffiac <f@ggot.wtf>
2024-10-04 12:28:57 +00:00
vero
2530f262f5
Remove bevy_animation dependency on bevy_text (#15642)
# Objective

- Fixes #15640

## Solution

- Do it

## Testing

- ran many_foxes
2024-10-04 12:22:15 +00:00
Tim
2526410096
Clean up the simple_picking example (#15633)
## Solution

- Removed superfluous `Pickable` components
- Slightly simplified the code for updating the text color
- Removed the `Pointer<Click>` observer from the mesh entirely since
that doesn't support picking yet
2024-10-04 01:31:21 +00:00
vero
0b9a461d5d
Invert the dependency between bevy_animation and bevy_ui (#15634)
# Objective

- Improve crate dependency graph

## Solution

- Invert a dependency

## Testing

- Tested ui and animation examples
2024-10-04 01:27:20 +00:00
Joona Aalto
61e11ea440
Fix audio not playing (#15638)
# Objective

Someone (let's not name names here) might've been a bit of a goofball,
and happened to forget that "playing audio" should cause this thing
called "sound" to be emitted! That someone might not have realized that
queries should be updated to account for audio using wrapper components
instead of raw asset handles after #15573.

## Solution

Update systems, and listen to the relaxing soundscapes of `Windless
Slopes.ogg` 🎵
2024-10-04 01:07:09 +00:00
Liam Gallagher
26808745cf
Fix bevy_window and bevy_winit readme badges (#15637)
## Objective

Fix the badges for `bevy_window` and `bevy_winit`.

## Solution

Replace the placeholder `bevy_name` wit the correct crate name.
2024-10-04 00:38:49 +00:00
fluffiac
f0704cffa4
Allow a closure to be used as a required component default (#15269)
# Objective

Allow required component default values to be provided in-line.

```rust
#[derive(Component)]
#[require(
    FocusPolicy(block_focus_policy)
)]
struct SomeComponent;

fn block_focus_policy() -> FocusPolicy {
    FocusPolicy::Block
}
```

May now be expressed as:

```rust
#[derive(Component)]
#[require(
    FocusPolicy(|| FocusPolicy::Block)
)]
struct SomeComponent;
```

## Solution

Modified the #[require] proc macro to accept a closure. 

## Testing

Tested using my branch as a dependency, and switching between the inline
closure syntax and function syntax for a bunch of different components.
2024-10-04 00:34:39 +00:00
Tim
20dbf790a6
Get rid of unnecessary mutable access in ui picking backend (#15630)
## Solution

Yeet

## Testing

Tested the `simple_picking` example
2024-10-03 21:30:52 +00:00
rewin
8bf5d99d86
Add method to remove component and all required components for removed component (#15026)
## Objective
The new Required Components feature (#14791) in Bevy allows spawning a
fixed set of components with a single method with cool require macro.
However, there's currently no corresponding method to remove all those
components together. This makes it challenging to keep insertion and
removal code in sync, especially for simple using cases.
```rust
#[derive(Component)]
#[require(Y)]
struct X;

#[derive(Component, Default)]
struct Y;

world.entity_mut(e).insert(X); // Spawns both X and Y
world.entity_mut(e).remove::<X>(); 
world.entity_mut(e).remove::<Y>(); // We need to manually remove dependencies without any sync with the `require` macro
```
## Solution
Simplifies component management by providing operations for removal
required components.
This PR introduces simple 'footgun' methods to removes all components of
this bundle and its required components.

Two new methods are introduced:
For Commands:
```rust
commands.entity(e).remove_with_requires::<B>();
```
For World:
```rust
world.entity_mut(e).remove_with_requires::<B>();
```

For performance I created new field in Bundels struct. This new field
"contributed_bundle_ids" contains cached ids for dynamic bundles
constructed from bundle_info.cintributed_components()

## Testing
The PR includes three test cases:

1. Removing a single component with requirements using World.
2. Removing a bundle with requirements using World.
3. Removing a single component with requirements using Commands.
4. Removing a single component with **runtime** requirements using
Commands

These tests ensure the feature works as expected across different
scenarios.

## Showcase
Example:
```rust
use bevy_ecs::prelude::*;

#[derive(Component)]
#[require(Y)]
struct X;

#[derive(Component, Default)]
#[require(Z)]
struct Y;

#[derive(Component, Default)]
struct Z;

#[derive(Component)]
struct W;

let mut world = World::new();

// Spawn an entity with X, Y, Z, and W components
let entity = world.spawn((X, W)).id();

assert!(world.entity(entity).contains::<X>());
assert!(world.entity(entity).contains::<Y>());
assert!(world.entity(entity).contains::<Z>());
assert!(world.entity(entity).contains::<W>());

// Remove X and required components Y, Z
world.entity_mut(entity).remove_with_requires::<X>();

assert!(!world.entity(entity).contains::<X>());
assert!(!world.entity(entity).contains::<Y>());
assert!(!world.entity(entity).contains::<Z>());

assert!(world.entity(entity).contains::<W>());
```

## Motivation for PR
#15580 

## Performance

I made simple benchmark
```rust
let mut world = World::default();
let entity = world.spawn_empty().id();

let steps = 100_000_000;

let start = std::time::Instant::now();
for _ in 0..steps {
    world.entity_mut(entity).insert(X);
    world.entity_mut(entity).remove::<(X, Y, Z, W)>();
}
let end = std::time::Instant::now();
println!("normal remove: {:?} ", (end - start).as_secs_f32());
println!("one remove: {:?} micros", (end - start).as_secs_f64() / steps as f64 * 1_000_000.0);

let start = std::time::Instant::now();
for _ in 0..steps {
    world.entity_mut(entity).insert(X);
    world.entity_mut(entity).remove_with_requires::<X>();
}
let end = std::time::Instant::now();
println!("remove_with_requires: {:?} ", (end - start).as_secs_f32());
println!("one remove_with_requires: {:?} micros", (end - start).as_secs_f64() / steps as f64 * 1_000_000.0);
```

Output:

CPU: Amd Ryzen 7 2700x

```bash
normal remove: 17.36135 
one remove: 0.17361348299999999 micros
remove_with_requires: 17.534006 
one remove_with_requires: 0.17534005400000002 micros
```

NOTE: I didn't find any tests or mechanism in the repository to update
BundleInfo after creating new runtime requirements with an existing
BundleInfo. So this PR also does not contain such logic.

## Future work (outside this PR)

Create cache system for fast removing components in "safe" mode, where
"safe" mode is remove only required components that will be no longer
required after removing root component.

---------

Co-authored-by: a.yamaev <a.yamaev@smartengines.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-10-03 20:35:08 +00:00
IceSentry
0628255c45
send_events is ambiguous_with_all (#15629)
# Objective

> Alice 🌹 — Today at 3:43 PM
bevy_dev_tools::ci_testing::systems::send_events
This system should be marked as ambiguous with everything I think

## Solution

- Mark it as `ambiguous_with_all`
2024-10-03 20:02:52 +00:00
Matty
528ca4f95e
Eliminate redundant clamping from sample-interpolated curves (#15620)
# Objective

Currently, sample-interpolated curves (such as those used by the glTF
loader for animations) do unnecessary extra work when `sample_clamped`
is called, since their implementations of `sample_unchecked` are already
clamped. Eliminating this redundant sampling is a small, easy
performance win which doesn't compromise on the animation system's
internal usage of `sample_clamped`, which guarantees that it never
samples curves out-of-bounds.

## Solution

For sample-interpolated curves, define `sample_clamped` in the way
`sample_unchecked` is currently defined, and then redirect
`sample_unchecked` to `sample_clamped`. This is arguably a more
idiomatic way of using the `cores` as well, which is nice.

## Testing

Ran `many_foxes` to make sure I didn't break anything.
2024-10-03 18:26:41 +00:00
Chris Russell
46180a75f8
System param for dynamic resources (#15189)
# Objective

Support accessing dynamic resources in a dynamic system, including
accessing them by component id. This is similar to how dynamic
components can be queried using `Query<FilteredEntityMut>`.

## Solution

Create `FilteredResources` and `FilteredResourcesMut` types that act
similar to `FilteredEntityRef` and `FilteredEntityMut` and that can be
used as system parameters.

## Example

```rust
// Use `FilteredResourcesParamBuilder` to declare access to resources.
let system = (FilteredResourcesParamBuilder::new(|builder| {
    builder.add_read::<B>().add_read::<C>();
}),)
    .build_state(&mut world)
    .build_system(resource_system);

world.init_resource::<A>();
world.init_resource::<C>();

fn resource_system(res: FilteredResources) {
    // The resource exists, but we have no access, so we can't read it.
    assert!(res.get::<A>().is_none());
    // The resource doesn't exist, so we can't read it.
    assert!(res.get::<B>().is_none());
    // The resource exists and we have access, so we can read it.
    let c = res.get::<C>().unwrap();
    // The type parameter can be left out if it can be determined from use.
    let c: Res<C> = res.get().unwrap();
}
```

## Future Work

As a follow-up PR, `ReflectResource` can be modified to take `impl
Into<FilteredResources>`, similar to how `ReflectComponent` takes `impl
Into<FilteredEntityRef>`. That will allow dynamic resources to be
accessed using reflection.
2024-10-03 18:20:34 +00:00
ickshonpe
1e61092604
Fix extract_text2d_sprite entity leak (#15625)
# Objective

`extract_2d_sprite` still uses `spawn_empty()`, replace with
`spawn(TemporaryRenderEntity)` .
2024-10-03 18:15:36 +00:00
ickshonpe
9bb27e97c5
Fix entity leak in extract_uinode_borders (#15626)
# Objective

Fix for another leak, this time when extracting outlines.
2024-10-03 18:15:32 +00:00
Kristoffer Søholm
336c23c1aa
Rename observe to observe_entity on EntityWorldMut (#15616)
# Objective

The current observers have some unfortunate footguns where you can end
up confused about what is actually being observed. For apps you can
chain observe like `app.observe(..).observe(..)` which works like you
would expect, but if you try the same with world the first `observe()`
will return the `EntityWorldMut` for the created observer, and the
second `observe()` will only observe on the observer entity. It took
several hours for multiple people on discord to figure this out, which
is not a great experience.

## Solution

Rename `observe` on entities to `observe_entity`. It's slightly more
verbose when you know you have an entity, but it feels right to me that
observers for specific things have more specific naming, and it prevents
this issue completely.

Another possible solution would be to unify `observe` on `App` and
`World` to have the same kind of return type, but I'm not sure exactly
what that would look like.

## Testing

Simple name change, so only concern is docs really.

---


## Migration Guide

The `observe()` method on entities has been renamed to
`observe_entity()` to prevent confusion about what is being observed in
some cases.
2024-10-03 17:05:26 +00:00
rudderbucky
2da8d17a44
Add try_despawn methods to World/Commands (#15480)
# Objective

Fixes #14511.

`despawn` allows you to remove entities from the world. However, if the
entity does not exist, it emits a warning. This may not be intended
behavior for many users who have use cases where they need to call
`despawn` regardless of if the entity actually exists (see the issue),
or don't care in general if the entity already doesn't exist.

(Also trying to gauge interest on if this feature makes sense, I'd
personally love to have it, but I could see arguments that this might be
a footgun. Just trying to help here 😄 If there's no contention I could
also implement this for `despawn_recursive` and `despawn_descendants` in
the same PR)

## Solution

Add `try_despawn`, `try_despawn_recursive` and
`try_despawn_descendants`.

Modify `World::despawn_with_caller` to also take in a `warn` boolean
argument, which is then considered when logging the warning. Set
`log_warning` to `true` in the case of `despawn`, and `false` in the
case of `try_despawn`.

## Testing

Ran `cargo run -p ci` on macOS, it seemed fine.
2024-10-03 16:21:05 +00:00
rudderbucky
5e81154e9c
Despawn and despawn_recursive benchmarks (#15610)
# Objective

Add despawn and despawn_recursive benchmarks in a similar vein to the
spawn benchmark.

## Testing

Ran `cargo bench` from `benches` and it compiled fine.

On my machine:
```
despawn_world/1_entities
                        time:   [3.1495 ns 3.1574 ns 3.1652 ns]
Found 4 outliers among 100 measurements (4.00%)
  3 (3.00%) high mild
  1 (1.00%) high severe
despawn_world/10_entities
                        time:   [28.629 ns 28.674 ns 28.720 ns]
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
despawn_world/100_entities
                        time:   [286.95 ns 287.41 ns 287.90 ns]
Found 5 outliers among 100 measurements (5.00%)
  5 (5.00%) high mild
despawn_world/1000_entities
                        time:   [2.8739 µs 2.9001 µs 2.9355 µs]
Found 7 outliers among 100 measurements (7.00%)
  1 (1.00%) high mild
  6 (6.00%) high severe
despawn_world/10000_entities
                        time:   [28.535 µs 28.617 µs 28.698 µs]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

despawn_world_recursive/1_entities
                        time:   [5.2270 ns 5.2507 ns 5.2907 ns]
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) low mild
  6 (6.00%) high mild
  4 (4.00%) high severe
despawn_world_recursive/10_entities
                        time:   [57.495 ns 57.590 ns 57.691 ns]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
despawn_world_recursive/100_entities
                        time:   [514.43 ns 518.91 ns 526.88 ns]
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) high mild
  3 (3.00%) high severe
despawn_world_recursive/1000_entities
                        time:   [5.0362 µs 5.0463 µs 5.0578 µs]
Found 7 outliers among 100 measurements (7.00%)
  2 (2.00%) high mild
  5 (5.00%) high severe
despawn_world_recursive/10000_entities
                        time:   [51.159 µs 51.603 µs 52.215 µs]
Found 9 outliers among 100 measurements (9.00%)
  3 (3.00%) high mild
  6 (6.00%) high severe
```
2024-10-03 14:59:37 +00:00
MiniaczQ
acea4e7e6f
Better warnings about invalid parameters (#15500)
# Objective

System param validation warnings should be configurable and default to
"warn once" (per system).

Fixes: #15391

## Solution

`SystemMeta` is given a new `ParamWarnPolicy` field.
The policy decides whether warnings will be emitted by each system param
when it fails validation.
The policy is updated by the system after param validation fails.

Example warning:
```
2024-09-30T18:10:04.740749Z  WARN bevy_ecs::system::function_system: System fallible_params::do_nothing_fail_validation will not run because it requested inaccessible system parameter Single<(), (With<Player>, With<Enemy>)>
```

Currently, only the first invalid parameter is displayed.

Warnings can be disabled on function systems using
`.param_never_warn()`.
(there is also `.with_param_warn_policy(policy)`)

## Testing

Ran `fallible_params` example.

---------

Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
2024-10-03 13:16:55 +00:00
Patrick Walton
ca8dd06146
Impose a more sensible ordering for animation graph evaluation. (#15589)
This is an updated version of #15530. Review comments were addressed.

This commit changes the animation graph evaluation to be operate in a
more sensible order and updates the semantics of blend nodes to conform
to [the animation composition RFC]. Prior to this patch, a node graph
like this:

```
	    ┌─────┐
	    │     │
	    │  1  │
	    │     │
	    └──┬──┘
	       │
       ┌───────┴───────┐
       │               │
       ▼               ▼
    ┌─────┐         ┌─────┐
    │     │         │     │
    │  2  │         │  3  │
    │     │         │     │
    └──┬──┘         └──┬──┘
       │               │
   ┌───┴───┐       ┌───┴───┐
   │       │       │       │
   ▼       ▼       ▼       ▼
┌─────┐ ┌─────┐ ┌─────┐ ┌─────┐
│     │ │     │ │     │ │     │
│  4  │ │  6  │ │  5  │ │  7  │
│     │ │     │ │     │ │     │
└─────┘ └─────┘ └─────┘ └─────┘
```

Would be evaluated as (((4 ⊕ 5) ⊕ 6) ⊕ 7), with the blend (lerp/slerp)
operation notated as ⊕. As quaternion multiplication isn't commutative,
this is very counterintuitive and will especially lead to trouble with
the forthcoming additive blending feature (#15198).

This patch fixes the issue by changing the evaluation order to
postorder, with children of a node evaluated in ascending order by node
index.

To do so, this patch revamps `AnimationCurve` to be based on an
*evaluation stack* and a *blend register*. During target evaluation, the
graph evaluator traverses the graph in postorder. When encountering a
clip node, the evaluator pushes the possibly-interpolated value onto the
evaluation stack. When encountering a blend node, the evaluator pops
values off the stack into the blend register, accumulating weights as
appropriate. When the graph is completely evaluated, the top element on
the stack is *committed* to the property of the component.

A new system, the *graph threading* system, is added in order to cache
the sorted postorder traversal to avoid the overhead of sorting children
at animation evaluation time. Mask evaluation has been moved to this
system so that the graph only has to be traversed at most once per
frame. Unlike the `ActiveAnimation` list, the *threaded graph* is cached
from frame to frame and only has to be regenerated when the animation
graph asset changes.

This patch currently regresses the `animate_target` performance in
`many_foxes` by around 50%, resulting in an FPS loss of about 2-3 FPS.
I'd argue that this is an acceptable price to pay for a much more
intuitive system. In the future, we can mitigate the regression with a
fast path that avoids consulting the graph if only one animation is
playing. However, in the interest of keeping this patch simple, I didn't
do so here.

[the animation composition RFC]:
https://github.com/bevyengine/rfcs/blob/main/rfcs/51-animation-composition.md

# Objective

- Describe the objective or issue this PR addresses.
- If you're fixing a specific issue, say "Fixes #X".

## Solution

- Describe the solution used to achieve the objective above.

## Testing

- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Showcase

> This section is optional. If this PR does not include a visual change
or does not add a new feature, you can delete this section.

- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR adds a new feature or public API, consider adding a brief
pseudo-code snippet of it in action
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
  - If you want, you could even include a before/after comparison!
- If the Migration Guide adequately covers the changes, you can delete
this section

While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:

<details>
  <summary>Click to view showcase</summary>

```rust
println!("My super cool code.");
```

</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: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-03 00:36:42 +00:00
IceSentry
67744bb011
Use circle gizmos for capsule (#15602)
# Objective

- The capsule gizmo uses straight lines for the upper and lower circle
which looks pretty ugly.

## Solution

- Use the circle gizmo instead

---

## Showcase

**BEFORE**

![3d_gizmos_sy3CmKUvKO](https://github.com/user-attachments/assets/be014de4-751e-4b40-b814-b5b97bb72031)

**AFTER**

![3d_gizmos_nyADBAUJHg](https://github.com/user-attachments/assets/539ff765-f9d8-4afe-9ac6-41fe83e94e94)

(the circles are red for demonstration purposes only)

# Notes

I also tried using 3d arcs instead of circles but it looks like arcs
need a lot more computation for an almost identical end result. Circles
seem much simpler. The only thing I'm unsure about is if the rotation
stuff is correct. It worked in my testing though.
2024-10-02 19:47:56 +00:00
Matt Tracy
8fb55dbf59
Implement SystemParam::queue() method for blanket implementation of ParamSet (#15599)
# Objective

The `queue()` method is an optional trait method which is necessary for
deferred operations (such as command queues) to work properly in the
context of an observer.

This method was omitted from the proc_macro blanket implementation of
`ParamSet` for tuples; as a result, SystemParams with deferred
application (such as Commands) would not work in observers if they were
part of a ParamSet.

This appears to have been a simple omission, as `queue()` was already
implemented for the separate blanket implementation of `ParamSet` for
`Vec<T>`. In both cases, it is a simple pass-through to the component
SystemParams.

## Solution

Add the `queue()` method implementation to the `impl_param_set` proco
macro.

## Testing

Added a unit test which clearly demonstrates the issue. It fails before
the fix, and passes afterwards.

---
2024-10-02 19:46:50 +00:00
Matty
587a508ef9
Remove TransformCurve (#15598)
# Objective

It is somewhat unlikely we will actually be able to support
`TransformCurve` (introduced in #15434) after the `AnimationGraph`
evaluation order changes in the immediate future. This is because
correctly blending overlapping animation properties is nontrivial, and
`Transform` overlaps with all of its own fields. We could still
potentially create something like this in the future, but it's likely to
require significant design and implementation work. By way of contrast,
the single-property wrappers `TranslationCurve`, `ScaleCurve`, and
`RotationCurve` should work perfectly fine, since they are
non-overlapping.

In this version release, creating `TransformCurve` in userspace is also
quite easy if desired (see the deletions from this PR).

## Solution

Delete `TransformCurve`. 

## Migration Guide

There is no released version that contains this, but we should make sure
that `TransformCurve` is excluded from the release notes for #15434 if
we merge this pull request.
2024-10-02 19:46:38 +00:00
notmd
453c0167b2
Allow access a method handler (#15601)
# Objective

- I'm building a streaming plugin for `bevy_remote` and accessing to
builtin method will be very valuable

## Solution

- Add a method to allow access a handler by method name.

## Testing

- CI should pass
2024-10-02 19:45:18 +00:00
Christian Hughes
7c6057bc69
Enable EntityRef::get_by_id and friends to take multiple ids and get multiple pointers back (#15593)
# Objective

- Closes #15577 

## Solution

The following functions can now also take multiple component IDs and
return multiple pointers back:
- `EntityRef::get_by_id`
- `EntityMut::get_by_id`
- `EntityMut::into_borrow_by_id`
- `EntityMut::get_mut_by_id`
- `EntityMut::into_mut_by_id`
- `EntityWorldMut::get_by_id`
- `EntityWorldMut::into_borrow_by_id`
- `EntityWorldMut::get_mut_by_id`
- `EntityWorldMut::into_mut_by_id`

If you pass in X, you receive Y:
- give a single `ComponentId`, receive a single `Ptr`/`MutUntyped`
- give a `[ComponentId; N]` (array), receive a `[Ptr; N]`/`[MutUntyped;
N]`
- give a `&[ComponentId; N]` (array), receive a `[Ptr; N]`/`[MutUntyped;
N]`
- give a `&[ComponentId]` (slice), receive a
`Vec<Ptr>`/`Vec<MutUntyped>`
- give a `&HashSet<ComponentId>`, receive a `HashMap<ComponentId,
Ptr>`/`HashMap<ComponentId, MutUntyped>`

## Testing

- Added 4 new tests.

---

## Migration Guide

- The following functions now return an `Result<_,
EntityComponentError>` instead of a `Option<_>`: `EntityRef::get_by_id`,
`EntityMut::get_by_id`, `EntityMut::into_borrow_by_id`,
`EntityMut::get_mut_by_id`, `EntityMut::into_mut_by_id`,
`EntityWorldMut::get_by_id`, `EntityWorldMut::into_borrow_by_id`,
`EntityWorldMut::get_mut_by_id`, `EntityWorldMut::into_mut_by_id`
2024-10-02 19:02:20 +00:00
Tim
ca709ffb5a
Cleanup clearcoat example (#15594)
The components needed for `DirectionalLight` are added automatically
since #15554
`create_point_light` already existed and returns a `PointLight` with
these same settings
2024-10-02 16:15:27 +00:00
Robert Walter
59db6f9cca
add curve utilities to create curves interpolating/easing between two values (#14788)
# Objective

Citing @mweatherley 

> There is a lot of shortfall for simple cases— e.g., we should have
library functions for making a curve connecting two points, eased
versions of that, and so on.

## Solution

This PR implements

- a simple `Easing` trait which is implemented for all `impl Curve<f32>`
types. We can't really guarantee that these curves have unit interval
domain, which some people would probably expect, but it is documented
that this isn't the case for these types and we redirect to
`EasingCurve` which is used for that purpose
- an `EasingCurve` struct, which is used to interpolate between two
values `start` and `end` using a `impl Easing` curve where the curve
will be guaranteed to be reparametrized
- a `LinearCurve` which linearly interpolates between two values `start`
and `end`
- a `CubicBezierCurve` which interpolates between `start` and `end`
values using a `CubicSegment`
- a `StepCurve` which interpolates between `start` and `end` with an
step-function with `n` steps
- an `ElasticCurve` which interpolates between `start` and `end` with
spring like behavior where the elasticity of the spring is configurable
- some `FunctionCurve` easing curves for different popular functions
including: `quadratic_ease_in`, `quadratic_ease_out`, `smoothstep`,
`identity`

## Testing

- there are a few new tests for all of these in the main module

---------

Co-authored-by: eckz <567737+eckz@users.noreply.github.com>
Co-authored-by: Miles Silberling-Cook <NthTensor@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Matty <weatherleymatthew@gmail.com>
2024-10-02 14:29:05 +00:00
Dragoș Tiselice
ba7907cae7
Added visibility bitmask as an alternative SSAO method (#13454)
Early implementation. I still have to fix the documentation and consider
writing a small migration guide.

Questions left to answer:

* [x] should thickness be an overridable constant?
* [x] is there a better way to implement `Eq`/`Hash` for `SSAOMethod`?
* [x] do we want to keep the linear sampler for the depth texture?
* [x] is there a better way to separate the logic than preprocessor
macros?


![vbao](https://github.com/bevyengine/bevy/assets/4136413/2a8a0389-2add-4c2e-be37-e208e52dcd25)

## Migration guide

SSAO algorithm was changed from GTAO to VBAO (visibility bitmasks). A
new field, `constant_object_thickness`, was added to
`ScreenSpaceAmbientOcclusion`. `ScreenSpaceAmbientOcclusion` also lost
its `Eq` and `Hash` implementations.

---------

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
2024-10-02 13:43:35 +00:00
Benjamin Brienen
c841dd92a1
Documentation for variadics (#15387)
# Objective

Relevant: #15208

## Solution

I went ahead and added the variadics documentation in all applicable
locations.

## Testing

- I built the documentation and inspected it to see whether the feature
is there.
2024-10-02 12:48:36 +00:00
Tim
461305b3d7
Revert "Have EntityCommands methods consume self for easier chaining" (#15523)
As discussed in #15521

- Partial revert of #14897, reverting the change to the methods to
consume `self`
- The `insert_if` method is kept

The migration guide of #14897 should be removed
Closes #15521

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-02 12:47:26 +00:00
François Mockers
23b0dd6ffd
move ANDROID_APP to bevy_window (#15585)
# Objective

- Remove dependency in bevy_asset to bevy_winit
- First step for #15565 

## Solution

- the static `ANDROID_APP` and the `android_activity` reexport are now
in `bevy_window`

## Migration Guide

If you use the `android_activity` reexport from
`bevy::winit::android_activity`, it is now in
`bevy:🪟:android_activity`. Same for the `ANDROID_APP` static
2024-10-02 03:01:06 +00:00
Liam Gallagher
85dfd72631
Include errors along side successful components in BRP bevy/get method (#15516)
## Objective

I am using BRP for a web inspector. To get components from a entity is
first do a `bevy/list` on the specific entity and then use the result in
a `bevy/get` request. The problem with this is `bevy/list` returns all
components even if they aren't reflect-able (which is what I expect) but
when I then do a `bevy/get` request even if all bar one of the
components are reflect-able the request will fail.

## Solution

Update the `bevy/get` response to include a map of components like it
did for successful request and a map of errors. This means if one or
more components are not present on the entity or cannot be reflected it
will not fail the entire request.

I also only did `bevy/get` as I don't think any of the other methods
would benefit from this.

## Testing

I tested this with my inspector and with a http client and it worked as
expected.

---------

Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
2024-10-02 02:26:01 +00:00
Gino Valente
eaa37f3b45
bevy_reflect: Add DeserializeWithRegistry and SerializeWithRegistry (#8611)
# Objective

### The Problem

Currently, the reflection deserializers give little control to users for
how a type is deserialized. The most control a user can have is to
register `ReflectDeserialize`, which will use a type's `Deserialize`
implementation.

However, there are times when a type may require slightly more control.

For example, let's say we want to make Bevy's `Mesh` easier to
deserialize via reflection (assume `Mesh` actually implemented
`Reflect`). Since we want this to be extensible, we'll make it so users
can use their own types so long as they satisfy `Into<Mesh>`. The end
result should allow users to define a RON file like:

```rust
{
  "my_game::meshes::Sphere": (
    radius: 2.5
  )
}
```

### The Current Solution

Since we don't know the types ahead of time, we'll need to use
reflection. Luckily, we can access type information dynamically via the
type registry. Let's make a custom type data struct that users can
register on their types:

```rust
pub struct ReflectIntoMesh {
  // ...
}

impl<T: FromReflect + Into<Mesh>> FromType<T> for ReflectIntoMesh {
  fn from_type() -> Self {
    // ...
  }
}
```

Now we'll need a way to use this type data during deserialization.
Unfortunately, we can't use `Deserialize` since we need access to the
registry. This is where `DeserializeSeed` comes in handy:

```rust
pub struct MeshDeserializer<'a> {
  pub registry: &'a TypeRegistry
}

impl<'a, 'de> DeserializeSeed<'de> for MeshDeserializer<'a> {
  type Value = Mesh;

  fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
  where
    D: serde::Deserializer<'de>,
  {
    struct MeshVisitor<'a> {
      registry: &'a TypeRegistry
    }

    impl<'a, 'de> Visitor<'de> for MeshVisitor<'a> {
      fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result {
        write!(formatter, "map containing mesh information")
      }

      fn visit_map<A>(self, mut map: A) -> Result<Self::Value, serde:🇩🇪:Error> where A: MapAccess<'de> {
        // Parse the type name
        let type_name = map.next_key::<String>()?.unwrap();

        // Deserialize the value based on the type name
        let registration = self.registry
          .get_with_name(&type_name)
          .expect("should be registered");
        let value = map.next_value_seed(TypedReflectDeserializer {
          registration,
          registry: self.registry,
        })?;

        // Convert the deserialized value into a `Mesh`
        let into_mesh = registration.data::<ReflectIntoMesh>().unwrap();
        Ok(into_mesh.into(value))
      }
    }
  }
}
```

### The Problem with the Current Solution

The solution above works great when all we need to do is deserialize
`Mesh` directly. But now, we want to be able to deserialize a struct
like this:

```rust
struct Fireball {
  damage: f32,
  mesh: Mesh,
}
```

This might look simple enough and should theoretically be no problem for
the reflection deserializer to handle, but this is where our
`MeshDeserializer` solution starts to break down.

In order to use `MeshDeserializer`, we need to have access to the
registry. The reflection deserializers have access to that, but we have
no way of borrowing it for our own deserialization since they have no
way of knowing about `MeshDeserializer`.

This means we need to implement _another_ `DeserializeSeed`— this time
for `Fireball`!
And if we decided to put `Fireball` inside another type, well now we
need one for that type as well.

As you can see, this solution does not scale well and results in a lot
of unnecessary boilerplate for the user.

## Solution

> [!note]
> This PR originally only included the addition of
`DeserializeWithRegistry`. Since then, a corresponding
`SerializeWithRegistry` trait has also been added. The reasoning and
usage is pretty much the same as the former so I didn't bother to update
the full PR description.

Created the `DeserializeWithRegistry` trait and
`ReflectDeserializeWithRegistry` type data.

The `DeserializeWithRegistry` trait works like a standard `Deserialize`
but provides access to the registry. And by registering the
`ReflectDeserializeWithRegistry` type data, the reflection deserializers
will automatically use the `DeserializeWithRegistry` implementation,
just like it does for `Deserialize`.

All we need to do is make the following changes:

```diff
#[derive(Reflect)]
+ #[reflect(DeserializeWithRegistry)]
struct Mesh {
  // ...
}

- impl<'a, 'de> DeserializeSeed<'de> for MeshDeserializer<'a> {
-   type Value = Mesh;
-   fn deserialize<D>(self, deserializer: D) -> Result<Self::Value, D::Error>
+ impl<'de> DeserializeWithRegistry<'de> for Mesh {
+   fn deserialize<D>(deserializer: D, registry: &TypeRegistry) -> Result<Self, D::Error>
    where
      D: serde::Deserializer<'de>,
    {
      // ...
    }
}
```

Now, any time the reflection deserializer comes across `Mesh`, it will
opt to use its `DeserializeWithRegistry` implementation. And this means
we no longer need to create a whole slew of `DeserializeSeed` types just
to deserialize `Mesh`.

### Why not a trait like `DeserializeSeed`?

While this would allow for anyone to define a deserializer for `Mesh`,
the problem is that it means __anyone can define a deserializer for
`Mesh`.__ This has the unfortunate consequence that users can never be
certain that their registration of `ReflectDeserializeSeed` is the one
that will actually be used.

We could consider adding something like that in the future, but I think
this PR's solution is much safer and follows the example set by
`ReflectDeserialize`.

### What if we made the `TypeRegistry` globally available?

This is one potential solution and has been discussed before (#6101).
However, that change is much more controversial and comes with its own
set of disadvantages (can't have multiple registries such as with
multiple worlds, likely some added performance cost with each access,
etc.).

### Followup Work

Once this PR is merged, we should consider merging `ReflectDeserialize`
into `DeserializeWithRegistry`. ~~There is already a blanket
implementation to make this transition generally pretty
straightforward.~~ The blanket implementations were removed for the sake
of this PR and will need to be re-added in the followup. I would propose
that we first mark `ReflectDeserialize` as deprecated, though, before we
outright remove it in a future release.

---

## Changelog

- Added the `DeserializeReflect` trait and `ReflectDeserializeReflect`
type data
- Added the `SerializeReflect` trait and `ReflectSerializeReflect` type
data
- Added `TypedReflectDeserializer::of` convenience constructor

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
2024-10-02 01:54:32 +00:00