forked from bevyengine/bevy
-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move schedule name into Schedule #92
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Objective [Rust 1.72.0](https://blog.rust-lang.org/2023/08/24/Rust-1.72.0.html) is now stable. # Notes - `let-else` formatting has arrived! - I chose to allow `explicit_iter_loop` due to rust-lang/rust-clippy#11074. We didn't hit any of the false positives that prevent compilation, but fixing this did produce a lot of the "symbol soup" mentioned, e.g. `for image in &mut *image_events {`. Happy to undo this if there's consensus the other way. --------- Co-authored-by: François <mockersf@gmail.com>
# Objective We want to measure performance on path reflection parsing. ## Solution Benchmark path-based reflection: - Add a benchmark for `ParsedPath::parse` It's fairly noisy, this is why I added the 3% threshold. Ideally we would fix the noisiness though. Supposedly I'm seeding the RNG correctly, so there shouldn't be much observable variance. Maybe someone can help spot the issue.
# Objective - Unify the `ParsedPath` and `GetPath` APIs. They weirdly didn't play well together. - Make `ParsedPath` and `GetPath` API easier to use ## Solution - Add the `ReflectPath` trait. - `GetPath` methods now accept an `impl ReflectPath<'a>` instead of a `&'a str`, this mean it also can accepts a `&ParsedPath` - Make `GetPath: Reflect` and use default impl for `Reflect` types. - Add `GetPath` and `ReflectPath` to the `bevy_reflect` prelude --- ## Changelog - Add the `ReflectPath` trait. - `GetPath` methods now accept an `impl ReflectPath<'a>` instead of a `&'a str`, this mean it also can accept a `&ParsedPath` - Make `GetPath: Reflect` and use default impl for `Reflect` types. - Add `GetPath` and `ReflectPath` to the `bevy_reflect` prelude ## Migration Guide `GetPath` now requires `Reflect`. This reduces a lot of boilerplate on bevy's side. If you were implementing manually `GetPath` on your own type, please get in touch! `ParsedPath::element[_mut]` isn't an inherent method of `ParsedPath`, you must now import `ReflectPath`. This is only relevant if you weren't importing the bevy prelude. ```diff -use bevy::reflect::ParsedPath; +use bevy::reflect::{ParsedPath, ReflectPath}; parsed_path.element(reflect_type).unwrap() parsed_path.element(reflect_type).unwrap()
# Objective All delimiter symbols used by the path parser are ASCII, this means we can entirely ignore UTF8 handling. This may improve performance. ## Solution Instead of storing the path as an `&str` + the parser offset, and reading the path using `&self.path[self.offset..]`, we store the parser state in a `&[u8]`. This allows two optimizations: 1. Avoid UTF8 checking on `&self.path[self.offset..]` 2. Avoid any kind of bound checking, since the length of what is left to read is stored in the `&[u8]`'s reference metadata, and is assumed valid by the compiler. This is a major improvement when comparing to the previous parser. 1. `access_following` and `next_token` now inline in `PathParser::next` 2. Benchmarking show a 20% performance increase (bevyengine#9364) Please note that while we ignore UTF-8 handling, **utf-8 is still supported**. This is because we only handle "at the edges" what happens exactly before and after a recognized `SYMBOL`. utf-8 is handled transparently beyond that.
# Objective Resolves bevyengine#9580 ## Solution Relaxed the bounds on the `Debug` impl for `dyn System`
# Objective - Fixes bevyengine#9533 ## Solution * Added `Val::ZERO` as a constant which is defined as `Val::Px(0.)`. * Added manual `PartialEq` implementation for `Val` which allows any zero value to equal any other zero value. E.g., `Val::Px(0.) == Val::Percent(0.)` etc. This is technically a breaking change, as `Val::Px(0.) == Val::Percent(0.)` now equals `true` instead of `false` (as an example) * Replaced instances of `Val::Px(0.)`, `Val::Percent(0.)`, etc. with `Val::ZERO` * Fixed `bevy_ui::layout::convert::tests::test_convert_from` test to account for Taffy not equating `Points(0.)` and `Percent(0.)`. These tests now use `assert_eq!(...)` instead of `assert!(matches!(...))` which gives easier to diagnose error messages.
# Objective Fixes bevyengine#9509 ## Solution We use the assumption, that enum types are uppercase in contrast to module names. [`collapse_type_name`](crates/bevy_util/src/short_names) is now retaining the second last segment, if it starts with a uppercase character. --------- Co-authored-by: Emi <emanuel.boehm@gmail.com> Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective * `Local` and `SystemName` implement `Debug` manually, but they could derive it. * `QueryState` and `dyn System` have unconventional debug formatting.
…ne#9236) This is a continuation of this PR: bevyengine#8062 # Objective - Reorder render schedule sets to allow data preparation when phase item order is known to support improved batching - Part of the batching/instancing etc plan from here: bevyengine#89 (comment) - The original idea came from @inodentry and proved to be a good one. Thanks! - Refactor `bevy_sprite` and `bevy_ui` to take advantage of the new ordering ## Solution - Move `Prepare` and `PrepareFlush` after `PhaseSortFlush` - Add a `PrepareAssets` set that runs in parallel with other systems and sets in the render schedule. - Put prepare_assets systems in the `PrepareAssets` set - If explicit dependencies are needed on Mesh or Material RenderAssets then depend on the appropriate system. - Add `ManageViews` and `ManageViewsFlush` sets between `ExtractCommands` and Queue - Move `queue_mesh*_bind_group` to the Prepare stage - Rename them to `prepare_` - Put systems that prepare resources (buffers, textures, etc.) into a `PrepareResources` set inside `Prepare` - Put the `prepare_..._bind_group` systems into a `PrepareBindGroup` set after `PrepareResources` - Move `prepare_lights` to the `ManageViews` set - `prepare_lights` creates views and this must happen before `Queue` - This system needs refactoring to stop handling all responsibilities - Gather lights, sort, and create shadow map views. Store sorted light entities in a resource - Remove `BatchedPhaseItem` - Replace `batch_range` with `batch_size` representing how many items to skip after rendering the item or to skip the item entirely if `batch_size` is 0. - `queue_sprites` has been split into `queue_sprites` for queueing phase items and `prepare_sprites` for batching after the `PhaseSort` - `PhaseItem`s are still inserted in `queue_sprites` - After sorting adjacent compatible sprite phase items are accumulated into `SpriteBatch` components on the first entity of each batch, containing a range of vertex indices. The associated `PhaseItem`'s `batch_size` is updated appropriately. - `SpriteBatch` items are then drawn skipping over the other items in the batch based on the value in `batch_size` - A very similar refactor was performed on `bevy_ui` --- ## Changelog Changed: - Reordered and reworked render app schedule sets. The main change is that data is extracted, queued, sorted, and then prepared when the order of data is known. - Refactor `bevy_sprite` and `bevy_ui` to take advantage of the reordering. ## Migration Guide - Assets such as materials and meshes should now be created in `PrepareAssets` e.g. `prepare_assets<Mesh>` - Queueing entities to `RenderPhase`s continues to be done in `Queue` e.g. `queue_sprites` - Preparing resources (textures, buffers, etc.) should now be done in `PrepareResources`, e.g. `prepare_prepass_textures`, `prepare_mesh_uniforms` - Prepare bind groups should now be done in `PrepareBindGroups` e.g. `prepare_mesh_bind_group` - Any batching or instancing can now be done in `Prepare` where the order of the phase items is known e.g. `prepare_sprites` ## Next Steps - Introduce some generic mechanism to ensure items that can be batched are grouped in the phase item order, currently you could easily have `[sprite at z 0, mesh at z 0, sprite at z 0]` preventing batching. - Investigate improved orderings for building the MeshUniform buffer - Implementing batching across the rest of bevy --------- Co-authored-by: Robert Swain <robert.swain@gmail.com> Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
# Objective Sometimes you want to create a plugin with a custom run condition. In a function, you take the `Condition` trait and then make a `BoxedCondition` from it to store it. And then you want to add that condition to a system, but you can't, because there is only the `run_if` function available which takes `impl Condition<M>` instead of `BoxedCondition`. So you have to create a wrapper type for the `BoxedCondition` and implement the `System` and `ReadOnlySystem` traits for the wrapper (Like it's done in the picture below). It's very inconvenient and boilerplate. But there is an easy solution for that: make the `run_if_inner` system that takes a `BoxedCondition` public. Also, it makes sense to make `in_set_inner` function public as well with the same motivation.  A chunk of the source code of the `bevy-inspector-egui` crate. ## Solution Make `run_if_inner` function public. Rename `run_if_inner` to `run_if_dyn`. Make `in_set_inner` function public. Rename `in_set_inner` to `in_set_dyn`. ## Changelog Changed visibility of `run_if_inner` from `pub(crate)` to `pub`. Renamed `run_if_inner` to `run_if_dyn`. Changed visibility of `in_set_inner` from `pub(crate)` to `pub`. Renamed `in_set_inner` to `in_set_dyn`. ## Migration Guide --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
# Objective - break up large build_schedule system to make it easier to read - Clean up related error messages. - I have a follow up PR that adds the schedule name to the error messages, but wanted to break this up from that. ## Changelog - refactor `build_schedule` to be easier to read ## Sample Error Messages Dependency Cycle ```text thread 'main' panicked at 'System dependencies contain cycle(s). schedule has 1 before/after cycle(s): cycle 1: system set 'A' must run before itself system set 'A' ... which must run before system set 'B' ... which must run before system set 'A' ', crates\bevy_ecs\src\schedule\schedule.rs:228:13 ``` ```text thread 'main' panicked at 'System dependencies contain cycle(s). schedule has 1 before/after cycle(s): cycle 1: system 'foo' must run before itself system 'foo' ... which must run before system 'bar' ... which must run before system 'foo' ', crates\bevy_ecs\src\schedule\schedule.rs:228:13 ``` Hierarchy Cycle ```text thread 'main' panicked at 'System set hierarchy contains cycle(s). schedule has 1 in_set cycle(s): cycle 1: set 'A' contains itself set 'A' ... which contains set 'B' ... which contains set 'A' ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` System Type Set ```text thread 'main' panicked at 'Tried to order against `SystemTypeSet(fn foo())` in a schedule that has more than one `SystemTypeSet(fn foo())` instance. `SystemTypeSet(fn foo())` is a `SystemTypeSet` and cannot be used for ordering if ambiguous. Use a different set without this restriction.', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Hierarchy Redundancy ```text thread 'main' panicked at 'System set hierarchy contains redundant edges. hierarchy contains redundant edge(s) -- system set 'X' cannot be child of set 'A', longer path exists ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Systems have ordering but interset ```text thread 'main' panicked at '`A` and `C` have a `before`-`after` relationship (which may be transitive) but share systems.', crates\bevy_ecs\src\schedule\schedule.rs:227:51 ``` Cross Dependency ```text thread 'main' panicked at '`A` and `B` have both `in_set` and `before`-`after` relationships (these might be transitive). This combination is unsolvable as a system cannot run before or after a set it belongs to.', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ``` Ambiguity ```text thread 'main' panicked at 'Systems with conflicting access have indeterminate run order. 1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these: -- res_mut and res_ref conflict on: ["bevymark::ambiguity::X"] ', crates\bevy_ecs\src\schedule\schedule.rs:230:13 ```
# Objective - Fix the `borders`, `ui` and `text_wrap_debug` examples. ## Solution - Swap `TransparentUi` to use a stable sort --- This is the smallest change to fix the examples but ideally this is fixed by setting better sort keys for the UI elements such that we can swap back to an unstable sort.
…gine#8526) # Objective Any time we wish to transform the output of a system, we currently use system piping to do so: ```rust my_system.pipe(|In(x)| do_something(x)) ``` Unfortunately, system piping is not a zero cost abstraction. Each call to `.pipe` requires allocating two extra access sets: one for the second system and one for the combined accesses of both systems. This also adds extra work to each call to `update_archetype_component_access`, which stacks as one adds multiple layers of system piping. ## Solution Add the `AdapterSystem` abstraction: similar to `CombinatorSystem`, this allows you to implement a trait to generically control how a system is run and how its inputs and outputs are processed. Unlike `CombinatorSystem`, this does not have any overhead when computing world accesses which makes it ideal for simple operations such as inverting or ignoring the output of a system. Add the extension method `.map(...)`: this is similar to `.pipe(...)`, only it accepts a closure as an argument instead of an `In<T>` system. ```rust my_system.map(do_something) ``` This has the added benefit of making system names less messy: a system that ignores its output will just be called `my_system`, instead of `Pipe(my_system, ignore)` --- ## Changelog TODO ## Migration Guide The `system_adapter` functions have been deprecated: use `.map` instead, which is a lightweight alternative to `.pipe`. ```rust // Before: my_system.pipe(system_adapter::ignore) my_system.pipe(system_adapter::unwrap) my_system.pipe(system_adapter::new(T::from)) // After: my_system.map(std::mem::drop) my_system.map(Result::unwrap) my_system.map(T::from) // Before: my_system.pipe(system_adapter::info) my_system.pipe(system_adapter::dbg) my_system.pipe(system_adapter::warn) my_system.pipe(system_adapter::error) // After: my_system.map(bevy_utils::info) my_system.map(bevy_utils::dbg) my_system.map(bevy_utils::warn) my_system.map(bevy_utils::error) ``` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective Fixes bevyengine#8840 Make the cursor position more consistent, right now the cursor position is *sometimes* outside of the window and returns the position and *sometimes* `None`. Even in the cases where someone might be using that position that is outside of the window, it'll probably require some manual transformations for it to actually be useful. ## Solution Check the windows width and height for out of bounds positions. --- ## Changelog - Cursor position is now always `None` when outside of the window. --------- Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
# Objective Added `AnimationPlayer` API UX improvements. - Succestor to bevyengine#5912 - Fixes bevyengine#5848 _(Credits to @asafigan for filing bevyengine#5848, creating the initial pull request, and the discussion in bevyengine#5912)_ ## Solution - Created `RepeatAnimation` enum to describe an animation repetition behavior. - Added `is_finished()`, `set_repeat()`, and `is_playback_reversed()` methods to the animation player. - ~~Made the animation clip optional as per the comment from bevyengine#5912~~ > ~~My problem is that the default handle [used the initialize a `PlayingAnimation`] could actually refer to an actual animation if an AnimationClip is set for the default handle, which leads me to ask, "Should animation_clip should be an Option?"~~ - Added an accessor for the animation clip `animation_clip()` to the animation player. To determine if an animation is finished, we use the number of times the animation has completed and the repetition behavior. If the animation is playing in reverse then `elapsed < 0.0` counts as a completion. Otherwise, `elapsed > animation.duration` counts as a completion. This is what I would expect, personally. If there's any ambiguity, perhaps we could add some `AnimationCompletionBehavior`, to specify that kind of completion behavior to use. Update: Previously `PlayingAnimation::elapsed` was being used as the seek time into the animation clip. This was misleading because if you increased the speed of the animation it would also increase (or decrease) the elapsed time. In other words, the elapsed time was not actually the elapsed time. To solve this, we introduce `PlayingAnimation::seek_time` to serve as the value we manipulate the move between keyframes. Consequently, `elapsed()` now returns the actual elapsed time, and is not effected by the animation speed. Because `set_elapsed` was being used to manipulate the displayed keyframe, we introduce `AnimationPlayer::seek_to` and `AnimationPlayer::replay` to provide this functionality. ## Migration Guide - Removed `set_elapsed`. - Removed `stop_repeating` in favour of `AnimationPlayer::set_repeat(RepeatAnimation::Never)`. - Introduced `seek_to` to seek to a given timestamp inside of the animation. - Introduced `seek_time` accessor for the `PlayingAnimation::seek_to`. - Introduced `AnimationPlayer::replay` to reset the `PlayingAnimation` to a state where no time has elapsed. --------- Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com> Co-authored-by: François <mockersf@gmail.com>
## Objective - `bevy_text/src/pipeline.rs` had some crufty code. ## Solution Remove the cruft. - `&mut self` argument was unused by `TextPipeline::create_text_measure`, so we replace it with a constructor `TextMeasureInfo::from_text`. - We also pass a `&Text` to `from_text` since there is no reason to split the struct before passing it as argument. - from_text also checks beforehand that every Font exist in the Assets<Font>. This allows rust to skip the drop code on the Vecs we create in the method, since there is no early exit. - We also remove the scaled_fonts field on `TextMeasureInfo`. This avoids an additional allocation. We can re-use the font on `fonts` instead in `compute_size`. Building a `ScaledFont` seems fairly cheap, when looking at the ab_glyph internals. - We also implement ToSectionText on TextMeasureSection, this let us skip creating a whole new Vec each time we call compute_size. - This let us remove compute_size_from_section_text, since its only purpose was to not have to allocate the Vec we just made redundant. - Make some immutabe `Vec<T>` into `Box<[T]>` and `String` into `Box<str>` - `{min,max}_width_content_size` fields of `TextMeasureInfo` have name `width` in them, yet the contain information on both width and height. - `TextMeasureInfo::linebreak_behaviour` -> `linebreak_behavior` ## Migration Guide - The `ResMut<TextPipeline>` argument to `measure_text_system` doesn't exist anymore. If you were calling this system manually, you should remove the argument. - The `{min,max}_width_content_size` fields of `TextMeasureInfo` are renamed to `min` and `max` respectively - Other changes to `TextMeasureInfo` may also break your code if you were manually building it. Please consider using the new `TextMeasureInfo::from_text` to build one instead. - `TextPipeline::create_text_measure` has been removed in favor of `TextMeasureInfo::from_text`
…ne#9136) # Objective This PR's first aim is to fix a mistake in `HalfSpace`'s documentation. When defining a `Frustum` myself in bevy_basic_portals, I realised that the "distance" of the `HalfSpace` is not, as the current doc defines, the "distance from the origin along the normal", but actually the opposite of that. See the example I gave in this PR. This means one of two things: 1. The documentation about `HalfSpace` is wrong (it is either way because of the `n.p + d > 0` formula given later anyway, which is how it behaves, but in that formula `d` is indeed the opposite of the "distance from the origin along the normal", otherwise it should be `n.p > d`) 2. The distance is supposed to be the "distance from the origin along the normal" but when used in a Frustum it's used as the opposite, and it is a mistake 3. Same as 2, but it is somehow intended Since I think `HalfSpace` is only used for `Frustum`, and it's easier to fix documentation than code, I assumed for this PR we're in case number 1. If we're in case number 3, the documentation of `Frustum` needs to change, and in case number 2, the code needs to be fixed. While I was at it, I also : - Tried to improve the documentation for `Frustum`, `Aabb`, and `VisibilitySystems`, among others, since they're all related to `Frustum`. - Fixed documentation about frustum culling not applying to 2d objects, which is not true since bevyengine#7885 ## Remarks and questions - What about a `HalfSpace` with an infinite distance, is it allowed and does it represents the whole space? If so it should probably be mentioned. - I referenced the `update_frusta` system in `bevy_render::view::visibility` directly instead of referencing its system set, should I reference the system set instead? It's a bit annoying since it's in 3 sets. - `visibility_propagate` is not public for some reason, I think it probably should be, but for now I only documented its system set, should I make it public? I don't think that would count as a breaking change? - Why is `Aabb` inserted by a system, with `NoFrustumCulling` as an opt-out, instead of having it inserted by default in `PbrBundle` for example and then the system calculating it when it's added? Is it because there is still no way to have an optional component inside a bundle? --------- Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective Similar to bevyengine#6344, but contains only `ReflectBundle` changes. Useful for scripting. The implementation has also been updated to look exactly like `ReflectComponent`. --- ## Changelog ### Added - Reflection for bundles. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective Make code relating to event more readable. Currently the `impl` block of `Events` is split in two, and the big part of its implementations are put at the end of the file, far from the definition of the `struct`. ## Solution - Move and merge the `impl` blocks of `Events` next to its definition. - Move the `EventSequence` definition and implementations before the `Events`, because they're pretty trivial and help understand how `Events` work, rather than being buried bellow `Events`. I separated those two steps in two commits to not be too confusing. I didn't modify any code of documentation. I want to do a second PR with such modifications after this one is merged.
# Objective - Meshes with a higher number of joints than `MAX_JOINTS` are crashing - Fixes partly bevyengine#9021 (doesn't crash anymore, but the mesh is not correctly displayed) ## Solution - Only take up to `MAX_JOINTS` joints when extending the buffer
# Objective fixes bevyengine#8357 gltf animations can affect multiple "root" nodes (i.e. top level nodes within a gltf scene). the current loader adds an AnimationPlayer to each root node which is affected by any animation. when a clip which affects multiple root nodes is played on a root node player, the root node name is not checked, leading to potentially incorrect weights being applied. also, the `AnimationClip::compatible_with` method will never return true for those clips, as it checks that all paths start with the root node name - not all paths start with the same name so it can't return true. ## Solution - check the first path node name matches the given root - change compatible_with to return true if `any` match is found a better alternative would probably be to attach the player to the scene root instead of the first child, and then walk the full path from there. this would be breaking (and would stop multiple animations that *don't* overlap from being played concurrently), but i'm happy to modify to that if it's preferred. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
…bevyengine#9421) # Objective Fixes bevyengine#9420 ## Solution Remove one of the two `AppExit` event checks in the `ScheduleRunnerPlugin`'s main loop. Specificially, the check that happens immediately before calling `App.update()`, to be consistent with the `WinitPlugin`.
# Objective - Fixes bevyengine#9321 ## Solution - `EntityMap` has been replaced by a simple `HashMap<Entity, Entity>`. --- ## Changelog - `EntityMap::world_scope` has been replaced with `World::world_scope` to avoid creating a new trait. This is a public facing change to the call semantics, but has no effect on results or behaviour. - `EntityMap`, as a `HashMap`, now operates on `&Entity` rather than `Entity`. This changes many standard access functions (e.g, `.get`) in a public-facing way. ## Migration Guide - Calls to `EntityMap::world_scope` can be directly replaced with the following: `map.world_scope(&mut world)` -> `world.world_scope(&mut map)` - Calls to legacy `EntityMap` methods such as `EntityMap::get` must explicitly include de/reference symbols: `let entity = map.get(parent);` -> `let &entity = map.get(&parent);`
…evyengine#9486) # Objective * There is no way to read the fields of `GridPlacement` once set. * Values of `0` for `GridPlacement`'s fields are invalid but can be set. * A non-zero representation would be half the size. fixes bevyengine#9474 ## Solution * Add `get_start`, `get_end` and `get_span` accessor methods. * Change`GridPlacement`'s constructor functions to panic on arguments of zero. * Use non-zero types instead of primitives for `GridPlacement`'s fields. --- ## Changelog `bevy_ui::ui_node::GridPlacement`: * Field types have been changed to `Option<NonZeroI16>` and `Option<NonZeroU16>`. This is because zero values are not valid for `GridPlacement`. Previously, Taffy interpreted these as auto variants. * Constructor functions for `GridPlacement` panic on arguments of `0`. * Added accessor functions: `get_start`, `get_end`, and `get_span`. These return the inner primitive value (if present) of the respective fields. ## Migration Guide `GridPlacement`'s constructor functions no longer accept values of `0`. Given any argument of `0` they will panic with a `GridPlacementError`.
…#9490) Fixes bevyengine#9458. On case-insensitive filesystems (Windows, Mac, NTFS mounted in Linux, etc.), a path can be represented in a multiple ways: - `c:\users\user\rust\assets\hello\world` - `c:/users/user/rust/assets/hello/world` - `C:\USERS\USER\rust\assets\hello\world` If user specifies a path variant that doesn't match asset folder path bevy calculates, `path.strip_prefix()` will fail, as demonstrated below: ```rs dbg!(Path::new("c:/foo/bar/baz").strip_prefix("c:/foo")); // Ok("bar/baz") dbg!(Path::new("c:/FOO/bar/baz").strip_prefix("c:/foo")); // StripPrefixError(()) ``` This commit rewrites the code in question in a way that prefix stripping is no longer necessary. I've tested with the following paths on my computer: ```rs let res = asset_server.load_folder("C:\\Users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("c:\\users\\user\\rust\\assets\\foo\\bar"); dbg!(res); let res = asset_server.load_folder("C:/Users/user/rust/assets/foo/bar"); dbg!(res); ```
…tities query (bevyengine#9518) # Objective `sync_simple_transforms` only checks for removed parents and doesn't filter for `Without<Parent>`, so it overwrites the `GlobalTransform` of non-orphan entities that were orphaned and then reparented since the last update. Introduced by bevyengine#7264 ## Solution Filter for `Without<Parent>`. Fixes bevyengine#9517, bevyengine#9492 ## Changelog `sync_simple_transforms`: * Added a `Without<Parent>` filter to the orphaned entities query.
# Objective These new defaults match what is used by `Camera2dBundle::default()`, removing a potential footgun from overriding a field in the projection component of the bundle. ## Solution Adjusted the near clipping plane of `OrthographicProjection::default()` to `-1000.`. --- ## Changelog Changed: `OrthographicProjection::default()` now matches the value used in `Camera2dBundle::default()` ## Migration Guide Workarounds used to keep the projection consistent with the bundle defaults are no longer required. Meanwhile, uses of `OrthographicProjection` in 2D scenes may need to be adjusted; the `near` clipping plane default was changed from `0.0` to `-1000.0`.
# Objective Fixes bevyengine#9550 ## Solution Removes a check that asserts that _all_ attribute metas are path-only, rather than just the `#[deref]` attribute itself. --- ## Changelog - Fixes an issue where deriving `Deref` with `#[deref]` on a field causes other attributes to sometimes result in a compile error --------- Co-authored-by: François <mockersf@gmail.com>
# Objective A Bezier curve is a curve defined by two or more control points. In the simplest form, it's just a line. The (arguably) most common type of Bezier curve is a cubic Bezier, defined by four control points. These are often used in animation, etc. Bevy has a Bezier curve struct called `Bezier`. However, this is technically a misnomer as it only represents cubic Bezier curves. ## Solution This PR changes the struct name to `CubicBezier` to more accurately reflect the struct's usage. Since it's exposed in Bevy's prelude, it can potentially collide with other `Bezier` implementations. While that might instead be an argument for removing it from the prelude, there's also something to be said for adding a more general `Bezier` into Bevy, in which case we'd likely want to use the name `Bezier`. As a final motivator, not only is the struct located in `cubic_spines.rs`, there are also several other spline-related structs which follow the `CubicXxx` naming convention where applicable. For example, `CubicSegment` represents a cubic Bezier curve (with coefficients pre-baked). --- ## Migration Guide - Change all `Bezier` references to `CubicBezier`
# Objective - have errors in configure_set and configure_sets show the line number of the user calling location rather than pointing to schedule.rs - use display formatting for the errors ## Example Error Text ```text // dependency loop // before thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: DependencyLoop("A")', crates\bevy_ecs\src\schedule\schedule.rs:682:39 // after thread 'main' panicked at 'System set `A` depends on itself.', examples/stress_tests/bevymark.rs:16:9 // hierarchy loop // before thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: HierarchyLoop("A")', crates\bevy_ecs\src\schedule\schedule.rs:682:3 // after thread 'main' panicked at 'System set `A` contains itself.', examples/stress_tests/bevymark.rs:16:9 // configuring a system type set // before thread 'main' panicked at 'configuring system type sets is not allowed', crates\bevy_ecs\src\schedule\config.rs:394:9 //after thread 'main' panicked at 'configuring system type sets is not allowed', examples/stress_tests/bevymark.rs:16:9 ``` Code to produce errors: ```rust use bevy::prelude::*; #[derive(SystemSet, Clone, Debug, PartialEq, Eq, Hash)] enum TestSet { A, } fn main() { fn foo() {} let mut app = App::empty(); // Hierarchy Loop app.configure_set(Main, TestSet::A.in_set(TestSet::A)); // Dependency Loop app.configure_set(Main, TestSet::A.after(TestSet::A)); // Configure System Type Set app.configure_set(Main, foo.into_system_set()); } ```
# Objective bevyengine#5483 allows for the creation of non-`Sync` locals. However, it's not actually possible to use these types as there is a `Sync` bound on the `Deref` impls. ## Solution Remove the unnecessary bounds.
# Objective The name `ManualEventIterator` is long and unnecessary, as this is the iterator type used for both `EventReader` and `ManualEventReader`. ## Solution Rename `ManualEventIterator` to `EventIterator`. To ease migration, add a deprecated type alias with the old name. --- ## Changelog - The types `ManualEventIterator{WithId}` have been renamed to `EventIterator{WithId}`. ## Migration Guide The type `ManualEventIterator` has been renamed to `EventIterator`. Additonally, `ManualEventIteratorWithId` has been renamed to `EventIteratorWithId`.
# Objective - Fixes bevyengine#4917 - Replaces bevyengine#9602 ## Solution - Replaced `EntityCommand` implementation for `FnOnce` to apply to `FnOnce(EntityMut)` instead of `FnOnce(Entity, &mut World)` --- ## Changelog - `FnOnce(Entity, &mut World)` no longer implements `EntityCommand`. This is a breaking change. ## Migration Guide ### 1. New-Type `FnOnce` Create an `EntityCommand` type which implements the method you previously wrote: ```rust pub struct ClassicEntityCommand<F>(pub F); impl<F> EntityCommand for ClassicEntityCommand<F> where F: FnOnce(Entity, &mut World) + Send + 'static, { fn apply(self, id: Entity, world: &mut World) { (self.0)(id, world); } } commands.add(ClassicEntityCommand(|id: Entity, world: &mut World| { /* ... */ })); ``` ### 2. Extract `(Entity, &mut World)` from `EntityMut` The method `into_world_mut` can be used to gain access to the `World` from an `EntityMut`. ```rust let old = |id: Entity, world: &mut World| { /* ... */ }; let new = |mut entity: EntityMut| { let id = entity.id(); let world = entity.into_world_mut(); /* ... */ }; ```
# Objective Remove `Val`'s `try_*` arithmetic methods. fixes bevyengine#9571 ## Changelog Removed these methods from `bevy_ui::ui_node::Val`: - `try_add` - `try_sub` - `try_add_assign_with_size` - `try_sub_assign_with_size` - `try_add_assign` - `try_sub_assign` - `try_add_assign_with_size` - `try_sub_assign_with_size` ## Migration Guide `Val`'s `try_*` arithmetic methods have been removed. To perform arithmetic on `Val`s deconstruct them using pattern matching.
# Objective - Fixes [bevyengine#8835](bevyengine#8835) ## Solution - Added a note to the `set_volume` docstring which explains how volume is interpreted. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: GitGhillie <jillisnoordhoek@gmail.com> Co-authored-by: François <mockersf@gmail.com>
# Objective To enable non exclusive system usage of reflected components and make reflection more ergonomic to use by making it more in line with standard entity commands. ## Solution - Implements a new `EntityCommands` extension trait for reflection related functions in the reflect module of bevy_ecs. - Implements 4 new commands, `insert_reflect`, `insert_reflect_with_registry`, `remove_reflect`, and `remove_reflect_with_registry`. Both insert commands take a `Box<dyn Reflect>` component while the remove commands take the component type name. - Made `EntityCommands` fields pub(crate) to allow access in the reflect module. (Might be worth making these just public to enable user end custom entity commands in a different pr) - Added basic tests to ensure the commands are actually working. - Documentation of functions. --- ## Changelog Added: - Implements 4 new commands on the new entity commands extension. - `insert_reflect` - `remove_reflect` - `insert_reflect_with_registry` - `remove_reflect_with_registry` The commands operate the same except the with_registry commands take a generic parameter for a resource that implements `AsRef<TypeRegistry>`. Otherwise the default commands use the `AppTypeRegistry` for reflection data. Changed: - Made `EntityCommands` fields pub(crate) to allow access in the reflect module. > Hopefully this time it works. Please don't make me rebase again ☹
…ettings` to all schedules (bevyengine#9514) # Objective - Fixes: bevyengine#9508 - Fixes: bevyengine#9526 ## Solution - Adds ```rust fn configure_schedules(&mut self, schedule_build_settings: ScheduleBuildSettings) ``` to `Schedules`, and `App` to simplify applying `ScheduleBuildSettings` to all schedules. --- ## Migration Guide - No breaking changes. - Adds `Schedule::get_build_settings()` getter for the schedule's `ScheduleBuildSettings`. - Can replaced manual configuration of all schedules: ```rust // Old for (_, schedule) in app.world.resource_mut::<Schedules>().iter_mut() { schedule.set_build_settings(build_settings); } // New app.configure_schedules(build_settings); ```
# Objective Doc comment for the `global_transform` field in `NodeBundle` says: ``` /// This field is automatically managed by the UI layout system. ``` The `GlobalTransform` component is the thing being managed, not the `global_transform` field, and the `TransformPropagate` systems do the managing, not the UI layout system.
…vyengine#9542) # Objective Every frame, `Events::update` gets called, which clears out any old events from the buffer. There should be a way of taking ownership of these old events instead of throwing them away. My use-case is dumping old events into a debug menu so they can be inspected later. One potential workaround is to just have a system that clones any incoming events and stores them in a list -- however, this requires the events to implement `Clone`. ## Solution Add `Events::update_drain`, which returns an iterator of the events that were removed from the buffer.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Schedule
to allow the schedule name to be used for errors and tracing in Schedule methodsSolution
Schedule
and adjust api's onWorld
andSchedule
to not pass explicit label where it makes sense to.Schedule::new
now takes a label so either add the label or useSchedule::default
which uses a default label.default
is mostly used in doc examples and tests.Changelog
Schedule
to improve error message and logging for schedules.Migration Guide
Schedule::new
andApp::add_schedule
if you aren't using a label and are using the schedule struct directly you can use the default constructor.
Schedules:insert
World::add_schedule