20dbf790a6
5567 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tim
|
20dbf790a6
|
Get rid of unnecessary mutable access in ui picking backend (#15630)
## Solution Yeet ## Testing Tested the `simple_picking` example |
||
|
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> |
||
|
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`
|
||
|
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. |
||
|
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.
|
||
|
ickshonpe
|
1e61092604
|
Fix extract_text2d_sprite entity leak (#15625)
# Objective `extract_2d_sprite` still uses `spawn_empty()`, replace with `spawn(TemporaryRenderEntity)` . |
||
|
ickshonpe
|
9bb27e97c5
|
Fix entity leak in extract_uinode_borders (#15626)
# Objective Fix for another leak, this time when extracting outlines. |
||
|
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. |
||
|
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. |
||
|
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> |
||
|
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> |
||
|
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**  **AFTER**  (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. |
||
|
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. --- |
||
|
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. |
||
|
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 |
||
|
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` |
||
|
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> |
||
|
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?  ## 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> |
||
|
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. |
||
|
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> |
||
|
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 |
||
|
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> |
||
|
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>
|
||
|
Viktor Gustavsson
|
f86ee32576
|
Add UI GhostNode (#15341)
# Objective - Fixes #14826 - For context, see #15238 ## Solution Add a `GhostNode` component to `bevy_ui` and update all the relevant systems to use it to traverse for UI children. - [x] `ghost_hierarchy` module - [x] Add `GhostNode` - [x] Add `UiRootNodes` system param for iterating (ghost-aware) UI root nodes - [x] Add `UiChildren` system param for iterating (ghost-aware) UI children - [x] Update `layout::ui_layout_system` - [x] Use ghost-aware root nodes for camera updates - [x] Update and remove children in taffy - [x] Initial spawn - [x] Detect changes on nested UI children - [x] Use ghost-aware children traversal in `update_uinode_geometry_recursive` - [x] Update the rest of the UI systems to use the ghost hierarchy - [x] `stack::ui_stack_system` - [x] `update::` - [x] `update_clipping_system` - [x] `update_target_camera_system` - [x] `accessibility::calc_name` ## Testing - [x] Added a new example `ghost_nodes` that can be used as a testbed. - [x] Added unit tests for _some_ of the traversal utilities in `ghost_hierarchy` - [x] Ensure this fulfills the needs for currently known use cases - [x] Reactivity libraries (test with `bevy_reactor`) - [ ] Text spans (mentioned by koe [on discord](https://discord.com/channels/691052431525675048/1285371432460881991/1285377442998915246)) --- ## Performance [See comment below](https://github.com/bevyengine/bevy/pull/15341#issuecomment-2385456820) ## Migration guide Any code that previously relied on `Parent`/`Children` to iterate UI children may now want to use `bevy_ui::UiChildren` to ensure ghost nodes are skipped, and their first descendant Nodes included. UI root nodes may now be children of ghost nodes, which means `Without<Parent>` might not query all root nodes. Use `bevy_ui::UiRootNodes` where needed to iterate root nodes instead. ## Potential future work - Benchmarking/optimizations of hierarchies containing lots of ghost nodes - Further exploration of UI hierarchies and markers for root nodes/leaf nodes to create better ergonomics for things like `UiLayer` (world-space ui) --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com> |
||
|
UkoeHB
|
3df281ba7b
|
Refactor TextPipeline::update_buffer to accept an interator (#15581)
# Objective - Prepare `TextPipeline` to work with multi-entity text blocks. See https://github.com/bevyengine/bevy/discussions/15014 ## Solution - Refactor `TextPipeline::update_buffer` to accept an iterator instead of slice. Adjust `update_buffer` implementation to only iterate spans once instead of three times (which would require iterating a hierarchy three times with multi-entity blocks). ## Testing - Tested with `text_debug` example. |
||
|
Liam Gallagher
|
d6cfafdfd4
|
Make SystemIdMarker reflect-able (#15556)
Make `SystemIdMarker` reflect-able. |
||
|
Joona Aalto
|
22af24aacf
|
Migrate motion blur, TAA, SSAO, and SSR to required components (#15572)
# Objective Again, a step forward in the migration to required components: a bunch of camera rendering cormponents! Note that this does not include the camera components themselves yet, because the naming and API for `Camera` hasn't been fully decided yet. ## Solution As per the [selected proposals](https://hackmd.io/@bevy/required_components/%2FpiqD9GOdSFKZZGzzh3C7Uw): - Deprecate `MotionBlurBundle` in favor of the `MotionBlur` component - Deprecate `TemporalAntiAliasBundle` in favor of the `TemporalAntiAliasing` component - Deprecate `ScreenSpaceAmbientOcclusionBundle` in favor of the `ScreenSpaceAmbientOcclusion` component - Deprecate `ScreenSpaceReflectionsBundle` in favor of the `ScreenSpaceReflections` component --- ## Migration Guide `MotionBlurBundle`, `TemporalAntiAliasBundle`, `ScreenSpaceAmbientOcclusionBundle`, and `ScreenSpaceReflectionsBundle` have been deprecated in favor of the `MotionBlur`, `TemporalAntiAliasing`, `ScreenSpaceAmbientOcclusion`, and `ScreenSpaceReflections` components instead. Inserting them will now also insert the other components required by them automatically. |
||
|
Joona Aalto
|
ed151e756c
|
Migrate audio to required components (#15573)
# Objective What's that? Another PR for the grand migration to required components? This time, audio! ## Solution Deprecate `AudioSourceBundle`, `AudioBundle`, and `PitchBundle`, as per the [chosen proposal](https://hackmd.io/@bevy/required_components/%2Fzxgp-zMMRUCdT7LY1ZDQwQ). However, we cannot call the component `AudioSource`, because that's what the stored asset is called. I deliberated on a few names, like `AudioHandle`, or even just `Audio`, but landed on `AudioPlayer`, since it's probably the most accurate and "nice" name for this. Open to alternatives though. --- ## Migration Guide Replace all insertions of `AudioSoucreBundle`, `AudioBundle`, and `PitchBundle` with the `AudioPlayer` component. The other components required by it will now be inserted automatically. In cases where the generics cannot be inferred, you may need to specify them explicitly. For example: ```rust commands.spawn(AudioPlayer::<AudioSource>(asset_server.load("sounds/sick_beats.ogg"))); ``` |
||
|
Tim
|
eb51b4c28e
|
Migrate scenes to required components (#15579)
# Objective A step in the migration to required components: scenes! ## Solution As per the [selected proposal](https://hackmd.io/@bevy/required_components/%2FPJtNGVMMQhyM0zIvCJSkbA): - Deprecate `SceneBundle` and `DynamicSceneBundle`. - Add `SceneRoot` and `DynamicSceneRoot` components, which wrap a `Handle<Scene>` and `Handle<DynamicScene>` respectively. ## Migration Guide Asset handles for scenes and dynamic scenes must now be wrapped in the `SceneRoot` and `DynamicSceneRoot` components. Raw handles as components no longer spawn scenes. Additionally, `SceneBundle` and `DynamicSceneBundle` have been deprecated. Instead, use the scene components directly. Previously: ```rust let model_scene = asset_server.load(GltfAssetLabel::Scene(0).from_asset("model.gltf")); commands.spawn(SceneBundle { scene: model_scene, transform: Transform::from_xyz(-4.0, 0.0, -3.0), ..default() }); ``` Now: ```rust let model_scene = asset_server.load(GltfAssetLabel::Scene(0).from_asset("model.gltf")); commands.spawn(( SceneRoot(model_scene), Transform::from_xyz(-4.0, 0.0, -3.0), )); ``` |
||
|
UkoeHB
|
ead84e0e3d
|
Rename BreakLineOn to LineBreak (#15583)
# Objective - Improve code quality in preparation for https://github.com/bevyengine/bevy/discussions/15014 ## Solution - Rename BreakLineOn to LineBreak. ## Migration Guide `BreakLineOn` was renamed to `LineBreak`, and paramters named `linebreak_behavior` were renamed to `linebreak`. |
||
|
Litttle_fish
|
e924df0e1a
|
Add features to switch NativeActivity and GameActivity usage (#12095)
# Objective Add two features to switch bevy to use `NativeActivity` or `GameActivity` on Android, use `GameActivity` by default. Also close #12058 and probably #12026 . ## Solution Add two features to the corresponding crates so you can toggle it, like what `winit` and `android-activity` crate did. --- ## Changelog Removed default `NativeActivity` feature implementation for Android, added two new features to enable `NativeActivity` and `GameActivity`, and use `GameActivity` by default. ## Migration Guide Because `cargo-apk` is not compatible with `GameActivity`, building/running using `cargo apk build/run -p bevy_mobile_example` is no longer possible. Users should follow the new workflow described in document. --------- Co-authored-by: François Mockers <francois.mockers@vleue.com> Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> Co-authored-by: Rich Churcher <rich.churcher@gmail.com> |
||
|
ickshonpe
|
f53af2846c
|
Update the UI texture slice pipeline to work with the retained render world changes (#15578)
# Objective Update the UI's texture slice extraction to work with the changes from the retained render world PR (#15320). |
||
|
Joona Aalto
|
54006b107b
|
Migrate meshes and materials to required components (#15524)
# Objective A big step in the migration to required components: meshes and materials! ## Solution As per the [selected proposal](https://hackmd.io/@bevy/required_components/%2Fj9-PnF-2QKK0on1KQ29UWQ): - Deprecate `MaterialMesh2dBundle`, `MaterialMeshBundle`, and `PbrBundle`. - Add `Mesh2d` and `Mesh3d` components, which wrap a `Handle<Mesh>`. - Add `MeshMaterial2d<M: Material2d>` and `MeshMaterial3d<M: Material>`, which wrap a `Handle<M>`. - Meshes *without* a mesh material should be rendered with a default material. The existence of a material is determined by `HasMaterial2d`/`HasMaterial3d`, which is required by `MeshMaterial2d`/`MeshMaterial3d`. This gets around problems with the generics. Previously: ```rust commands.spawn(MaterialMesh2dBundle { mesh: meshes.add(Circle::new(100.0)).into(), material: materials.add(Color::srgb(7.5, 0.0, 7.5)), transform: Transform::from_translation(Vec3::new(-200., 0., 0.)), ..default() }); ``` Now: ```rust commands.spawn(( Mesh2d(meshes.add(Circle::new(100.0))), MeshMaterial2d(materials.add(Color::srgb(7.5, 0.0, 7.5))), Transform::from_translation(Vec3::new(-200., 0., 0.)), )); ``` If the mesh material is missing, previously nothing was rendered. Now, it renders a white default `ColorMaterial` in 2D and a `StandardMaterial` in 3D (this can be overridden). Below, only every other entity has a material:   Why white? This is still open for discussion, but I think white makes sense for a *default* material, while *invalid* asset handles pointing to nothing should have something like a pink material to indicate that something is broken (I don't handle that in this PR yet). This is kind of a mix of Godot and Unity: Godot just renders a white material for non-existent materials, while Unity renders nothing when no materials exist, but renders pink for invalid materials. I can also change the default material to pink if that is preferable though. ## Testing I ran some 2D and 3D examples to test if anything changed visually. I have not tested all examples or features yet however. If anyone wants to test more extensively, it would be appreciated! ## Implementation Notes - The relationship between `bevy_render` and `bevy_pbr` is weird here. `bevy_render` needs `Mesh3d` for its own systems, but `bevy_pbr` has all of the material logic, and `bevy_render` doesn't depend on it. I feel like the two crates should be refactored in some way, but I think that's out of scope for this PR. - I didn't migrate meshlets to required components yet. That can probably be done in a follow-up, as this is already a huge PR. - It is becoming increasingly clear to me that we really, *really* want to disallow raw asset handles as components. They caused me a *ton* of headache here already, and it took me a long time to find every place that queried for them or inserted them directly on entities, since there were no compiler errors for it. If we don't remove the `Component` derive, I expect raw asset handles to be a *huge* footgun for users as we transition to wrapper components, especially as handles as components have been the norm so far. I personally consider this to be a blocker for 0.15: we need to migrate to wrapper components for asset handles everywhere, and remove the `Component` derive. Also see https://github.com/bevyengine/bevy/issues/14124. --- ## Migration Guide Asset handles for meshes and mesh materials must now be wrapped in the `Mesh2d` and `MeshMaterial2d` or `Mesh3d` and `MeshMaterial3d` components for 2D and 3D respectively. Raw handles as components no longer render meshes. Additionally, `MaterialMesh2dBundle`, `MaterialMeshBundle`, and `PbrBundle` have been deprecated. Instead, use the mesh and material components directly. Previously: ```rust commands.spawn(MaterialMesh2dBundle { mesh: meshes.add(Circle::new(100.0)).into(), material: materials.add(Color::srgb(7.5, 0.0, 7.5)), transform: Transform::from_translation(Vec3::new(-200., 0., 0.)), ..default() }); ``` Now: ```rust commands.spawn(( Mesh2d(meshes.add(Circle::new(100.0))), MeshMaterial2d(materials.add(Color::srgb(7.5, 0.0, 7.5))), Transform::from_translation(Vec3::new(-200., 0., 0.)), )); ``` If the mesh material is missing, a white default material is now used. Previously, nothing was rendered if the material was missing. The `WithMesh2d` and `WithMesh3d` query filter type aliases have also been removed. Simply use `With<Mesh2d>` or `With<Mesh3d>`. --------- Co-authored-by: Tim Blackbird <justthecooldude@gmail.com> Co-authored-by: Carter Anderson <mcanders1@gmail.com> |
||
|
Joona Aalto
|
0fe17b8b30
|
Migrate fog volumes to required components (#15568)
# Objective Another part of the migration to required components: fog volumes! ## Solution Deprecate `FogVolumeBundle` and make `FogVolume` require `Transform` and `Visibility`, as per the [chosen proposal](https://hackmd.io/@bevy/required_components/%2FcO7JPSAQR5G0J_j5wNwtOQ). --- ## Migration Guide Replace all insertions of `FogVolumeBundle` with the `Visibility` component. The other components required by it will now be inserted automatically. |
||
|
Josh Robson Chase
|
40e88dceff
|
Change ReflectMapEntities to operate on components before insertion (#15422)
Previous PR https://github.com/bevyengine/bevy/pull/14549 was closed in error and couldn't be reopened since I had updated the branch 😿 # Objective Fixes #14465 ## Solution `ReflectMapEntities` now works similarly to `MapEntities` in that it works on the reflected value itself rather than the component in the world after insertion. This makes it so that observers see the remapped entities on insertion rather than the entity IDs from the scene. `ReflectMapEntities` now works for both components and resources, so we only need the one. ## Testing * New unit test for `Observer`s + `DynamicScene`s * New unit test for `Observer`s + `Scene`s * Open to suggestions for other tests! --- ## Migration Guide - Consumers of `ReflectMapEntities` will need to call `map_entities` on values prior to inserting them into the world. - Implementors of `MapEntities` will need to remove the `mappings` method, which is no longer needed for `ReflectMapEntities` and has been removed from the trait. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> |
||
|
vero
|
6465e3bd9f
|
Fix Mesh allocator bug and reduce Mesh data copies by two (#15566)
# Objective - First step towards #15558 ## Solution - Rename `get_vertex_buffer_data` to `create_packed_vertex_buffer_data` to make it clear that it is not "free" and actually allocates - Compute length analytically for preallocation instead of creating the buffer to get its length and immediately discard it - Use existing vertex attribute size calculation method to reduce code duplication - Fix a bug where mesh index data was being replaced by unnecessarily newly created mesh vertex data in some cases - Overall reduces mesh copies by two. We still have plenty to go, but these were the easy ones. ## Testing - I ran 3d_scene, lighting, and many_cubes, they look fine. - Benchmarks would be nice, but this is very obviously a win in perf and correctness. --- ## Migration Guide - `Mesh::create_packed_vertex_buffer_data` has been renamed `Mesh::create_packed_vertex_buffer_data` to reflect the fact that it copies data and allocates. ## Showcase - look mom, less copies |
||
|
aecsocket
|
1df8238e8d
|
bevy_asset: Improve NestedLoader API (#15509)
# Objective
The `NestedLoader` API as it stands right now is somewhat lacking:
- It consists of several types `NestedLoader`, `UntypedNestedLoader`,
`DirectNestedLoader`, and `UntypedDirectNestedLoader`, where a typestate
pattern on `NestedLoader` would be make it more obvious what it does,
and allow centralising the documentation
- The term "untyped" in the asset loader code is overloaded. It can mean
either:
- we have literally no idea what the type of this asset will be when we
load it (I dub this "unknown type")
- we know what type of asset it will be, but we don't know it statically
- we only have a TypeId (I dub this "dynamic type" / "erased")
- There is no way to get an `UntypedHandle` (erased) given a `TypeId`
## Solution
Changes `NestedLoader` into a type-state pattern, adding two type
params:
- `T` determines the typing
- `StaticTyped`, the default, where you pass in `A` statically into `fn
load<A>() -> ..`
- `DynamicTyped`, where you give a `TypeId`, giving you a
`UntypedHandle`
- `UnknownTyped`, where you have literally no idea what type of asset
you're loading, giving you a `Handle<LoadedUntypedAsset>`
- `M` determines the "mode" (bikeshedding TBD, I couldn't come up with a
better name)
- `Deferred`, the default, won't load the asset when you call `load`,
but it does give you a `Handle` to it (this is nice since it can be a
sync fn)
- `Immediate` will load the asset as soon as you call it, and give you
access to it, but you must be in an async context to call it
Changes some naming of internals in `AssetServer` to fit the new
definitions of "dynamic type" and "unknown type". Note that I didn't do
a full pass over this code to keep the diff small. That can probably be
done in a new PR - I think the definiton I laid out of unknown type vs.
erased makes it pretty clear where each one applies.
<details>
<summary>Old issue</summary>
The only real problem I have with this PR is the requirement to pass in
`type_name` (from `core::any::type_name`) into Erased. Users might not
have that type name, only the ID, and it just seems sort of weird to
*have* to give an asset type name. However, the reason we need it is
because of this:
```rs
pub(crate) fn get_or_create_path_handle_erased(
&mut self,
path: AssetPath<'static>,
type_id: TypeId,
type_name: &str,
loading_mode: HandleLoadingMode,
meta_transform: Option<MetaTransform>,
) -> (UntypedHandle, bool) {
let result = self.get_or_create_path_handle_internal(
path,
Some(type_id),
loading_mode,
meta_transform,
);
// it is ok to unwrap because TypeId was specified above
unwrap_with_context(result, type_name).unwrap()
}
pub(crate) fn unwrap_with_context<T>(
result: Result<T, GetOrCreateHandleInternalError>,
type_name: &str,
) -> Option<T> {
match result {
Ok(value) => Some(value),
Err(GetOrCreateHandleInternalError::HandleMissingButTypeIdNotSpecified) => None,
Err(GetOrCreateHandleInternalError::MissingHandleProviderError(_)) => {
panic!("Cannot allocate an Asset Handle of type '{type_name}' because the asset type has not been initialized. \
Make sure you have called app.init_asset::<{type_name}>()")
}
}
}
```
This `unwrap_with_context` is literally the only reason we need the
`type_name`. Potentially, this can be turned into an `impl
Into<Option<&str>>`, and output a different error message if the type
name is missing. Since if we are loading an asset where we only know the
type ID, by definition we can't output that error message, since we
don't have the type name. I'm open to suggestions on this.
</details>
## Testing
Not sure how to test this, since I kept most of the actual NestedLoader
logic the same. The only new API is loading an `UntypedHandle` when in
the `DynamicTyped, Immediate` state.
## Migration Guide
Code which uses `bevy_asset`'s `LoadContext::loader` / `NestedLoader`
will see some naming changes:
- `untyped` is replaced by `with_unknown_type`
- `with_asset_type` is replaced by `with_static_type`
- `with_asset_type_id` is replaced by `with_dynamic_type`
- `direct` is replaced by `immediate` (the opposite of "immediate" is
"deferred")
|
||
|
m-edlund
|
c323db02e0
|
Add sub_camera_view, enabling sheared projection (#15537)
# Objective - This PR fixes #12488 ## Solution - This PR adds a new property to `Camera` that emulates the functionality of the [setViewOffset()](https://threejs.org/docs/#api/en/cameras/PerspectiveCamera.setViewOffset) API in three.js. - When set, the perspective and orthographic projections will restrict the visible area of the camera to a part of the view frustum defined by `offset` and `size`. ## Testing - In the new `camera_sub_view` example, a fixed, moving and control sub view is created for both perspective and orthographic projection - Run the example with `cargo run --example camera_sub_view` - The code can be tested by adding a `SubCameraView` to a camera --- ## Showcase  - Left Half: Perspective Projection - Right Half: Orthographic Projection - Small boxes in order: - Sub view of the left half of the full image - Sub view moving from the top left to the bottom right of the full image - Sub view of the full image (acting as a control) - Large box: No sub view <details> <summary>Shortened camera setup of `camera_sub_view` example</summary> ```rust // Main perspective Camera commands.spawn(Camera3dBundle { transform, ..default() }); // Perspective camera left half commands.spawn(Camera3dBundle { camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view camera to the left half of the full image full_size: uvec2(500, 500), offset: ivec2(0, 0), size: uvec2(250, 500), }), order: 1, ..default() }, transform, ..default() }); // Perspective camera moving commands.spawn(( Camera3dBundle { camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view camera to a fifth of the full view and // move it in another system full_size: uvec2(500, 500), offset: ivec2(0, 0), size: uvec2(100, 100), }), order: 2, ..default() }, transform, ..default() }, MovingCameraMarker, )); // Perspective camera control commands.spawn(Camera3dBundle { camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view to the full image, to ensure that it matches // the projection without sub view full_size: uvec2(450, 450), offset: ivec2(0, 0), size: uvec2(450, 450), }), order: 3, ..default() }, transform, ..default() }); // Main orthographic camera commands.spawn(Camera3dBundle { projection: OrthographicProjection { ... } .into(), camera: Camera { order: 4, ..default() }, transform, ..default() }); // Orthographic camera left half commands.spawn(Camera3dBundle { projection: OrthographicProjection { ... } .into(), camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view camera to the left half of the full image full_size: uvec2(500, 500), offset: ivec2(0, 0), size: uvec2(250, 500), }), order: 5, ..default() }, transform, ..default() }); // Orthographic camera moving commands.spawn(( Camera3dBundle { projection: OrthographicProjection { ... } .into(), camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view camera to a fifth of the full view and // move it in another system full_size: uvec2(500, 500), offset: ivec2(0, 0), size: uvec2(100, 100), }), order: 6, ..default() }, transform, ..default() }, MovingCameraMarker, )); // Orthographic camera control commands.spawn(Camera3dBundle { projection: OrthographicProjection { ... } .into(), camera: Camera { sub_camera_view: Some(SubCameraView { // Set the sub view to the full image, to ensure that it matches // the projection without sub view full_size: uvec2(450, 450), offset: ivec2(0, 0), size: uvec2(450, 450), }), order: 7, ..default() }, transform, ..default() }); ``` </details> |
||
|
Ensar Sarajčić
|
956d9ccbb1
|
Add directory related functions to AndroidAssetReader (#11495)
# Objective - Fixes #9968 ## Solution - Uses [open_dir](https://docs.rs/ndk/latest/ndk/asset/struct.AssetManager.html#method.open_dir) to read directories and collects child list, since it can't be shared across threads. - For `is_directory`, uses result of [open](https://docs.rs/ndk/latest/ndk/asset/struct.AssetManager.html#method.open), which will fail for directories. I tried using the result of `open_dir` for this, but it was successful for files too, which made loading folders return empty lists, since `open_dir` was successful and treated all files as empty directories. - Ignoring `meta` files was copied from filesystem implementation --- ## Changelog - Fixed: Android's AssetReader implementation now supports read_directory and is_directory. ## Notes I noticed late that there was the #9968 issue (I only noticed #9591), so I have also missed that a PR was already open (#9969). Feel free to copy over the fixes from this one over there. The only difference I notice between these 2, is that I have used `open` instead of `open_dir` for `is_directory` implementation. I have tried with `open_dir` too, but unfortunately that didn't work. I tested this on an actual device, using the mobile example, by making some minor changes: ```rust #[derive(Resource)] struct FolderAssets(Handle<LoadedFolder>); // the `bevy_main` proc_macro generates the required boilerplate for iOS and Android #[bevy_main] fn main() { // ... .add_systems(Startup, (setup_scene, load_music_files)) .add_systems( Update, // Removed the handle_lifetime since AudioBundle is added later (touch_camera, button_handler, setup_music), ); // ... } fn load_music_files(asset_server: Res<AssetServer>, mut commands: Commands) { let sounds = asset_server.load_folder("sounds"); commands.insert_resource(FolderAssets(sounds)); } fn setup_music( mut commands: Commands, folders: Res<Assets<LoadedFolder>>, mut loaded_event: EventReader<AssetEvent<LoadedFolder>>, ) { for event in loaded_event.read() { if let AssetEvent::LoadedWithDependencies { id } = event { if let Some(folder) = folders.get(*id) { warn!("Folder items: {:?}", folder.handles); if let Some(source) = folder.handles.first() { commands.spawn(AudioBundle { source: source.clone().typed::<AudioSource>(), settings: PlaybackSettings::LOOP, }); } } } } } ``` --------- Co-authored-by: Kanabenki <lucien.menassol@gmail.com> Co-authored-by: François Mockers <francois.mockers@vleue.com> |
||
|
Ludwig DUBOS
|
f08f07785b
|
Replace AsyncSeek trait by AsyncSeekForward for Reader to address #12880 (#14194)
# Objective The primary motivation behind this PR is to (partially?) address the limitations imposed by the recently added `AsyncSeek` trait bound discussed in issue #12880. While the `AsyncSeek` trait add some flexibility to the reader, it inadvertently restricts the ability to write asset readers that can truly stream bytes, particularly in scenarios like HTTP requests where backward seeking is not supported. It is also challenging in contexts where assets are stored in compressed formats or require other kinds of transformations. The logic behind this change is that currently, with `AsyncSeek`, an asset Reader based on streamed data will either 1) fail silently, 2) return an error, or 3) use a buffer to satisfy the trait constraint. I believe that being able to advance in the file without having to "read" it is a good thing. The only issue here is the ability to seek backward. It is highly likely that in this context, we only need to seek forward in the file because we would have already read an entry table upstream and just want to access one or more resources further in the file. I understand that in some cases, this may not be applicable, but I think it is more beneficial not to constrain `Reader`s that want to stream than to allow "Assets" to read files in a completely arbitrary order. ## Solution Replace the current `AsyncSeek` trait with `AsyncSeekForward` on asset `Reader` ## Changelog - Introduced a new custom trait, `AsyncSeekForward`, for the asset Reader. - Replaced the current `AsyncSeek` trait with `AsyncSeekForward` for all asset `Reader` implementations. ## Migration Guide Replace all instances of `AsyncSeek` with `AsyncSeekForward` in your asset reader implementations. |
||
|
Joona Aalto
|
de888a373d
|
Migrate lights to required components (#15554)
# Objective Another step in the migration to required components: lights! Note that this does not include `EnvironmentMapLight` or reflection probes yet, because their API hasn't been fully chosen yet. ## Solution As per the [selected proposals](https://hackmd.io/@bevy/required_components/%2FLLnzwz9XTxiD7i2jiUXkJg): - Deprecate `PointLightBundle` in favor of the `PointLight` component - Deprecate `SpotLightBundle` in favor of the `PointLight` component - Deprecate `DirectionalLightBundle` in favor of the `DirectionalLight` component ## Testing I ran some examples with lights. --- ## Migration Guide `PointLightBundle`, `SpotLightBundle`, and `DirectionalLightBundle` have been deprecated. Use the `PointLight`, `SpotLight`, and `DirectionalLight` components instead. Adding them will now insert the other components required by them automatically. |
||
|
Andreas
|
383c2e5bd7
|
15540 Make World::flush_commands private (#15553)
# Objective Fixes #15540 End-users risk using `World::flush_commands` instead of `World::flush`, which panics if any queued commands are `spawn`. Hiding `World::flush_commands` would help avoid calling a potentially panicky function, and helps alleviate end-user API confusion. ## Solution This PR updates the function visibility to crate-level, like `World::flush_entities`, hiding it from the end-user while still making it accessible for the tests that are currently set up. ## Testing The change was tested by executing the available tests for `bevy_ecs`. From what I've gathered, `World::flush_commands` is not used in any other bevy crate. If further testing is recommended, please inform me! --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Kristoffer Søholm
|
73af2b7d29
|
Cleanup unneeded lifetimes in bevy_asset (#15546)
# Objective Fixes #15541 A bunch of lifetimes were added during the Assets V2 rework, but after moving to async traits in #12550 they can be elided. That PR mentions that this might be the case, but apparently it wasn't followed up on at the time. ~~I ended up grepping for `<'a` and finding a similar case in `bevy_reflect` which I also fixed.~~ (edit: that one was needed apparently) Note that elided lifetimes are unstable in `impl Trait`. If that gets stabilized then we can elide even more. ## Solution Remove the extra lifetimes. ## Testing Everything still compiles. If I have messed something up there is a small risk that some user code stops compiling, but all the examples still work at least. --- ## Migration Guide The traits `AssetLoader`, `AssetSaver` and `Process` traits from `bevy_asset` now use elided lifetimes. If you implement these then remove the named lifetime. |
||
|
Liam Gallagher
|
10068f4a26
|
Add content-type header to BRP HTTP responses (#15552)
This makes HTTP clients like httpie format the response as JSON rather than text for a much nicer experience testing things. |
||
|
Matty
|
429987ebf8
|
Curve-based animation (#15434)
# Objective This PR extends and reworks the material from #15282 by allowing arbitrary curves to be used by the animation system to animate arbitrary properties. The goals of this work are to: - Allow far greater flexibility in how animations are allowed to be defined in order to be used with `bevy_animation`. - Delegate responsibility over keyframe interpolation to `bevy_math` and the `Curve` libraries and reduce reliance on keyframes in animation definitions generally. - Move away from allowing the glTF spec to completely define animations on a mechanical level. ## Solution ### Overview At a high level, curves have been incorporated into the animation system using the `AnimationCurve` trait (closely related to what was `Keyframes`). From the top down: 1. In `animate_targets`, animations are driven by `VariableCurve`, which is now a thin wrapper around a `Box<dyn AnimationCurve>`. 2. `AnimationCurve` is something built out of a `Curve`, and it tells the animation system how to use the curve's output to actually mutate component properties. The trait looks like this: ```rust /// A low-level trait that provides control over how curves are actually applied to entities /// by the animation system. /// /// Typically, this will not need to be implemented manually, since it is automatically /// implemented by [`AnimatableCurve`] and other curves used by the animation system /// (e.g. those that animate parts of transforms or morph weights). However, this can be /// implemented manually when `AnimatableCurve` is not sufficiently expressive. /// /// In many respects, this behaves like a type-erased form of [`Curve`], where the output /// type of the curve is remembered only in the components that are mutated in the /// implementation of [`apply`]. /// /// [`apply`]: AnimationCurve::apply pub trait AnimationCurve: Reflect + Debug + Send + Sync { /// Returns a boxed clone of this value. fn clone_value(&self) -> Box<dyn AnimationCurve>; /// The range of times for which this animation is defined. fn domain(&self) -> Interval; /// Write the value of sampling this curve at time `t` into `transform` or `entity`, /// as appropriate, interpolating between the existing value and the sampled value /// using the given `weight`. fn apply<'a>( &self, t: f32, transform: Option<Mut<'a, Transform>>, entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle<AnimationGraph>)>, weight: f32, ) -> Result<(), AnimationEvaluationError>; } ``` 3. The conversion process from a `Curve` to an `AnimationCurve` involves using wrappers which communicate the intent to animate a particular property. For example, here is `TranslationCurve`, which wraps a `Curve<Vec3>` and uses it to animate `Transform::translation`: ```rust /// This type allows a curve valued in `Vec3` to become an [`AnimationCurve`] that animates /// the translation component of a transform. pub struct TranslationCurve<C>(pub C); ``` ### Animatable Properties The `AnimatableProperty` trait survives in the transition, and it can be used to allow curves to animate arbitrary component properties. The updated documentation for `AnimatableProperty` explains this process: <details> <summary>Expand AnimatableProperty example</summary An `AnimatableProperty` is a value on a component that Bevy can animate. You can implement this trait on a unit struct in order to support animating custom components other than transforms and morph weights. Use that type in conjunction with `AnimatableCurve` (and perhaps `AnimatableKeyframeCurve` to define the animation itself). For example, in order to animate font size of a text section from 24 pt. to 80 pt., you might use: ```rust #[derive(Reflect)] struct FontSizeProperty; impl AnimatableProperty for FontSizeProperty { type Component = Text; type Property = f32; fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> { Some(&mut component.sections.get_mut(0)?.style.font_size) } } ``` You can then create an `AnimationClip` to animate this property like so: ```rust let mut animation_clip = AnimationClip::default(); animation_clip.add_curve_to_target( animation_target_id, AnimatableKeyframeCurve::new( [ (0.0, 24.0), (1.0, 80.0), ] ) .map(AnimatableCurve::<FontSizeProperty, _>::from_curve) .expect("Failed to create font size curve") ); ``` Here, the use of `AnimatableKeyframeCurve` creates a curve out of the given keyframe time-value pairs, using the `Animatable` implementation of `f32` to interpolate between them. The invocation of `AnimatableCurve::from_curve` with `FontSizeProperty` indicates that the `f32` output from that curve is to be used to animate the font size of a `Text` component (as configured above). </details> ### glTF Loading glTF animations are now loaded into `Curve` types of various kinds, depending on what is being animated and what interpolation mode is being used. Those types get wrapped into and converted into `Box<dyn AnimationCurve>` and shoved inside of a `VariableCurve` just like everybody else. ### Morph Weights There is an `IterableCurve` abstraction which allows sampling these from a contiguous buffer without allocating. Its only reason for existing is that Rust disallows you from naming function types, otherwise we would just use `Curve` with an iterator output type. (The iterator involves `Map`, and the name of the function type would have to be able to be named, but it is not.) A `WeightsCurve` adaptor turns an `IterableCurve` into an `AnimationCurve`, so it behaves like everything else in that regard. ## Testing Tested by running existing animation examples. Interpolation logic has had additional tests added within the `Curve` API to replace the tests in `bevy_animation`. Some kinds of out-of-bounds errors have become impossible. Performance testing on `many_foxes` (`animate_targets`) suggests that performance is very similar to the existing implementation. Here are a couple trace histograms across different runs (yellow is this branch, red is main). <img width="669" alt="Screenshot 2024-09-27 at 9 41 50 AM" src="https://github.com/user-attachments/assets/5ba4e9ac-3aea-452e-aaf8-1492acc2d7fc"> <img width="673" alt="Screenshot 2024-09-27 at 9 45 18 AM" src="https://github.com/user-attachments/assets/8982538b-04cf-46b5-97b2-164c6bc8162e"> --- ## Migration Guide Most user code that does not directly deal with `AnimationClip` and `VariableCurve` will not need to be changed. On the other hand, `VariableCurve` has been completely overhauled. If you were previously defining animation curves in code using keyframes, you will need to migrate that code to use curve constructors instead. For example, a rotation animation defined using keyframes and added to an animation clip like this: ```rust animation_clip.add_curve_to_target( animation_target_id, VariableCurve { keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0], keyframes: Keyframes::Rotation(vec![ Quat::IDENTITY, Quat::from_axis_angle(Vec3::Y, PI / 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.), Quat::IDENTITY, ]), interpolation: Interpolation::Linear, }, ); ``` would now be added like this: ```rust animation_clip.add_curve_to_target( animation_target_id, AnimatableKeyframeCurve::new([0.0, 1.0, 2.0, 3.0, 4.0].into_iter().zip([ Quat::IDENTITY, Quat::from_axis_angle(Vec3::Y, PI / 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.), Quat::IDENTITY, ])) .map(RotationCurve) .expect("Failed to build rotation curve"), ); ``` Note that the interface of `AnimationClip::add_curve_to_target` has also changed (as this example shows, if subtly), and now takes its curve input as an `impl AnimationCurve`. If you need to add a `VariableCurve` directly, a new method `add_variable_curve_to_target` accommodates that (and serves as a one-to-one migration in this regard). ### For reviewers The diff is pretty big, and the structure of some of the changes might not be super-obvious: - `keyframes.rs` became `animation_curves.rs`, and `AnimationCurve` is based heavily on `Keyframes`, with the adaptors also largely following suite. - The Curve API adaptor structs were moved from `bevy_math::curve::mod` into their own module `adaptors`. There are no functional changes to how these adaptors work; this is just to make room for the specialized reflection implementations since `mod.rs` was getting kind of cramped. - The new module `gltf_curves` holds the additional curve constructions that are needed by the glTF loader. Note that the loader uses a mix of these and off-the-shelf `bevy_math` curve stuff. - `animatable.rs` no longer holds logic related to keyframe interpolation, which is now delegated to the existing abstractions in `bevy_math::curve::cores`. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com> |
||
|
Joona Aalto
|
f3e8ae03cd
|
Runtime required components (#15458)
# Objective Fixes #15367. Currently, required components can only be defined through the `require` macro attribute. While this should be used in most cases, there are also several instances where you may want to define requirements at runtime, commonly in plugins. Example use cases: - Require components only if the relevant optional plugins are enabled. For example, a `SleepTimer` component (for physics) is only relevant if the `SleepPlugin` is enabled. - Third party crates can define their own requirements for first party types. For example, "each `Handle<Mesh>` should require my custom rendering data components". This also gets around the orphan rule. - Generic plugins that add marker components based on the existence of other components, like a generic `ColliderPlugin<C: AnyCollider>` that wants to add a `ColliderMarker` component for all types of colliders. - This is currently relevant for the retained render world in #15320. The `ExtractComponentPlugin<C>` should add `SyncToRenderWorld` to all components that should be extracted. This is currently done with observers, which is more expensive than required components, and causes archetype moves. - Replace some built-in components with custom versions. For example, if `GlobalTransform` required `Transform` through `TransformPlugin`, but we wanted to use a `CustomTransform` type, we could replace `TransformPlugin` with our own plugin. (This specific example isn't good, but there are likely better use cases where this may be useful) See #15367 for more in-depth reasoning. ## Solution Add `register_required_components::<T, R>` and `register_required_components_with::<T, R>` methods for `Default` and custom constructors respectively. These methods exist on `App` and `World`. ```rust struct BirdPlugin; impl Plugin for BirdPlugin { fn plugin(app: &mut App) { // Make `Bird` require `Wings` with a `Default` constructor. app.register_required_components::<Bird, Wings>(); // Make `Wings` require `FlapSpeed` with a custom constructor. // Fun fact: Some hummingbirds can flutter their wings 80 times per second! app.register_required_components_with::<Wings, FlapSpeed>(|| FlapSpeed::from_duration(1.0 / 80.0)); } } ``` The custom constructor is a function pointer to match the `require` API, though it could take a raw value too. Requirement inheritance works similarly as with the `require` attribute. If `Bird` required `FlapSpeed` directly, it would take precedence over indirectly requiring it through `Wings`. The same logic applies to all levels of the inheritance tree. Note that registering the same component requirement more than once will panic, similarly to trying to add multiple component hooks of the same type to the same component. This avoids constructor conflicts and confusing ordering issues. ### Implementation Runtime requirements have two additional challenges in comparison to the `require` attribute. 1. The `require` attribute uses recursion and macros with clever ordering to populate hash maps of required components for each component type. The expected semantics are that "more specific" requirements override ones deeper in the inheritance tree. However, at runtime, there is no representation of how "specific" each requirement is. 2. If you first register the requirement `X -> Y`, and later register `Y -> Z`, then `X` should also indirectly require `Z`. However, `Y` itself doesn't know that it is required by `X`, so it's not aware that it should update the list of required components for `X`. My solutions to these problems are: 1. Store the depth in the inheritance tree for each entry of a given component's `RequiredComponents`. This is used to determine how "specific" each requirement is. For `require`-based registration, these depths are computed as part of the recursion. 2. Store and maintain a `required_by` list in each component's `ComponentInfo`, next to `required_components`. For `require`-based registration, these are also added after each registration, as part of the recursion. When calling `register_required_components`, it works as follows: 1. Get the required components of `Foo`, and check that `Bar` isn't already a *direct* requirement. 3. Register `Bar` as a required component for `Foo`, and add `Foo` to the `required_by` list for `Bar`. 4. Find and register all indirect requirements inherited from `Bar`, adding `Foo` to the `required_by` list for each component. 5. Iterate through components that require `Foo`, registering the new inherited requires for them as indirect requirements. The runtime registration is likely slightly more expensive than the `require` version, but it is a one-time cost, and quite negligible in practice, unless projects have hundreds or thousands of runtime requirements. I have not benchmarked this however. This does also add a small amount of extra cost to the `require` attribute for updating `required_by` lists, but I expect it to be very minor. ## Testing I added some tests that are copies of the `require` versions, as well as some tests that are more specific to the runtime implementation. I might add a few more tests though. ## Discussion - Is `register_required_components` a good name? Originally I went for `register_component_requirement` to be consistent with `register_component_hooks`, but the general feature is often referred to as "required components", which is why I changed it to `register_required_components`. - Should we *not* panic for duplicate requirements? If so, should they just be ignored, or should the latest registration overwrite earlier ones? - If we do want to panic for duplicate, conflicting registrations, should we at least not panic if the registrations are *exactly* the same, i.e. same component and same constructor? The current implementation panics for all duplicate direct registrations regardless of the constructor. ## Next Steps - Allow `register_required_components` to take a `Bundle` instead of a single required component. - I could also try to do it in this PR if that would be preferable. - Not directly related, but archetype invariants? |
||
|
Trashtalk217
|
56f8e526dd
|
The Cooler 'Retain Rendering World' (#15320)
- Adopted from #14449 - Still fixes #12144. ## Migration Guide The retained render world is a complex change: migrating might take one of a few different forms depending on the patterns you're using. For every example, we specify in which world the code is run. Most of the changes affect render world code, so for the average Bevy user who's using Bevy's high-level rendering APIs, these changes are unlikely to affect your code. ### Spawning entities in the render world Previously, if you spawned an entity with `world.spawn(...)`, `commands.spawn(...)` or some other method in the rendering world, it would be despawned at the end of each frame. In 0.15, this is no longer the case and so your old code could leak entities. This can be mitigated by either re-architecting your code to no longer continuously spawn entities (like you're used to in the main world), or by adding the `bevy_render::world_sync::TemporaryRenderEntity` component to the entity you're spawning. Entities tagged with `TemporaryRenderEntity` will be removed at the end of each frame (like before). ### Extract components with `ExtractComponentPlugin` ``` // main world app.add_plugins(ExtractComponentPlugin::<ComponentToExtract>::default()); ``` `ExtractComponentPlugin` has been changed to only work with synced entities. Entities are automatically synced if `ComponentToExtract` is added to them. However, entities are not "unsynced" if any given `ComponentToExtract` is removed, because an entity may have multiple components to extract. This would cause the other components to no longer get extracted because the entity is not synced. So be careful when only removing extracted components from entities in the render world, because it might leave an entity behind in the render world. The solution here is to avoid only removing extracted components and instead despawn the entire entity. ### Manual extraction using `Extract<Query<(Entity, ...)>>` ```rust // in render world, inspired by bevy_pbr/src/cluster/mod.rs pub fn extract_clusters( mut commands: Commands, views: Extract<Query<(Entity, &Clusters, &Camera)>>, ) { for (entity, clusters, camera) in &views { // some code commands.get_or_spawn(entity).insert(...); } } ``` One of the primary consequences of the retained rendering world is that there's no longer a one-to-one mapping from entity IDs in the main world to entity IDs in the render world. Unlike in Bevy 0.14, Entity 42 in the main world doesn't necessarily map to entity 42 in the render world. Previous code which called `get_or_spawn(main_world_entity)` in the render world (`Extract<Query<(Entity, ...)>>` returns main world entities). Instead, you should use `&RenderEntity` and `render_entity.id()` to get the correct entity in the render world. Note that this entity does need to be synced first in order to have a `RenderEntity`. When performing manual abstraction, this won't happen automatically (like with `ExtractComponentPlugin`) so add a `SyncToRenderWorld` marker component to the entities you want to extract. This results in the following code: ```rust // in render world, inspired by bevy_pbr/src/cluster/mod.rs pub fn extract_clusters( mut commands: Commands, views: Extract<Query<(&RenderEntity, &Clusters, &Camera)>>, ) { for (render_entity, clusters, camera) in &views { // some code commands.get_or_spawn(render_entity.id()).insert(...); } } // in main world, when spawning world.spawn(Clusters::default(), Camera::default(), SyncToRenderWorld) ``` ### Looking up `Entity` ids in the render world As previously stated, there's now no correspondence between main world and render world `Entity` identifiers. Querying for `Entity` in the render world will return the `Entity` id in the render world: query for `MainEntity` (and use its `id()` method) to get the corresponding entity in the main world. This is also a good way to tell the difference between synced and unsynced entities in the render world, because unsynced entities won't have a `MainEntity` component. --------- 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: Emerson Coskey <56370779+ecoskey@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com> |
||
|
TheBigCheese
|
01dce4742f
|
Add to_inner_rectangle, area and perimeter methods to Capsule2d (#15388)
# Objective Unlike `Capsule3d` which has the `.to_cylinder` method, `Capsule2d` doesn't have an equivalent `.to_inner_rectangle` method and as shown by #15191 this is surprisingly easy to get wrong ## Solution Implemented a `Capsule2d::to_inner_rectangle` method as it is implemented in the fixed `Capsule2d` shape sampling, and as I was adding tests I noticed `Capsule2d` didn't implement `Measure2d` so I did this as well. ## Changelog ### Added - `Capsule2d::to_inner_rectangle`, `Capsule2d::area` and `Capsule2d::perimeter` --------- Co-authored-by: Joona Aalto <jondolf.dev@gmail.com> Co-authored-by: James Liu <contact@jamessliu.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
ickshonpe
|
c5742ff43e
|
Simplified ui_stack_system (#9889)
# Objective `ui_stack_system` generates a tree of `StackingContexts` which it then flattens to get the `UiStack`. But there's no need to construct a new tree. We can query for nodes with a global `ZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, ignoring any branches that have a global `Zindex`. Fixes #9877 ## Solution Split the `ZIndex` enum into two separate components, `ZIndex` and `GlobalZIndex` Query for nodes with a `GlobalZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, filtering branches by `Without<GlobalZIndex>` so we don't revisit nodes. ``` cargo run --profile stress-test --features trace_tracy --example many_buttons ``` <img width="672" alt="ui-stack-system-walk-split-enum" src="https://github.com/bevyengine/bevy/assets/27962798/11e357a5-477f-4804-8ada-c4527c009421"> (Yellow is this PR, red is main) --- ## Changelog `Zindex` * The `ZIndex` enum has been split into two separate components `ZIndex` (which replaces `ZIndex::Local`) and `GlobalZIndex` (which replaces `ZIndex::Global`). An entity can have both a `ZIndex` and `GlobalZIndex`, in comparisons `ZIndex` breaks ties if two `GlobalZIndex` values are equal. `ui_stack_system` * Instead of generating a tree of `StackingContexts`, query for nodes with a `GlobalZIndex`, add those nodes to the root nodes list and then build the `UiStack` from a walk of the existing layout tree, filtering branches by `Without<GlobalZIndex` so we don't revisit nodes. ## Migration Guide The `ZIndex` enum has been split into two separate components `ZIndex` (which replaces `ZIndex::Local`) and `GlobalZIndex` (which replaces `ZIndex::Global`). An entity can have both a `ZIndex` and `GlobalZIndex`, in comparisons `ZIndex` breaks ties if two `GlobalZindex` values are equal. --------- Co-authored-by: Gabriel Bourgeois <gabriel.bourgeoisv4si@gmail.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com> |
||
|
Matty
|
93aa2a2cc4
|
Make SampleCurve/UnevenSampleCurve succeed at reflection (#15493)
(Note: #15434 implements something very similar to this for functional curve adaptors, which is why they aren't present in this PR.) # Objective Previously, there was basically no chance that the explicitly-interpolating sample curve structs from the `Curve` API would actually be `Reflect`. The reason for this is functional programming: the structs contain an explicit interpolation `I: Fn(&T, &T, f32) -> T` which, under typical circumstances, will never be `Reflect`, which prevents the derive from realistically succeeding. In fact, they won't be a lot of other things either, notably including both`Debug` and `TypePath`, which are also required for reflection to succeed. The goal of this PR is to weaken the implementations of reflection traits for these structs so that they can implement `Reflect` under reasonable circumstances. (Notably, they will still not be `FromReflect`, which is unavoidable.) ## Solution The function fields are marked as `#[reflect(ignore)]`, and the derive macro for `Reflect` has `FromReflect` disabled. (This is not fully optimal, but we don't presently have any kind of "read-only" attribute for these fields.) Additionally, these structs receive custom `Debug` and `TypePath` implementations that display the function's (unstable!) type name instead of its value or type path (respectively). In the case of `TypePath`, this is a bit janky, but the instability of `type_name` won't generally present an issue for generics, which would have to be registered manually in the type registry anyway, which is impossible because the function type parameters cannot be named. (And in general, the "blessed" route for such cases would generally involve manually monomorphizing the function parameter away, which also allows access to `FromReflect` etc. through very ordinary use of the derive macro.) ## Testing Tests in the new `bevy_math::curve::sample_curves` module guarantee that these are actually `Reflect` under reasonable circumstances. --- ## Future changes If and when function item types become `Default`, these types will need to receive custom `FromReflect` implementations that exploit it. Such a custom implementation would also be desirable if users start doing things like wrapping function items in `Default`/`FromReflect` wrappers that still implement a `Fn` trait. Additionally, if function types become nameable in user-space, the stance on `Debug`/`TypePath` may bear reexamination, since partial monomorphization through wrappers would make implementing reflect traits for function types potentially more viable. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> |