8c5604e88a
10 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Joona Aalto
|
7b1c9f192e
|
Adopt consistent FooSystems naming convention for system sets (#18900)
# Objective Fixes a part of #14274. Bevy has an incredibly inconsistent naming convention for its system sets, both internally and across the ecosystem. <img alt="System sets in Bevy" src="https://github.com/user-attachments/assets/d16e2027-793f-4ba4-9cc9-e780b14a5a1b" width="450" /> *Names of public system set types in Bevy* Most Bevy types use a naming of `FooSystem` or just `Foo`, but there are also a few `FooSystems` and `FooSet` types. In ecosystem crates on the other hand, `FooSet` is perhaps the most commonly used name in general. Conventions being so wildly inconsistent can make it harder for users to pick names for their own types, to search for system sets on docs.rs, or to even discern which types *are* system sets. To reign in the inconsistency a bit and help unify the ecosystem, it would be good to establish a common recommended naming convention for system sets in Bevy itself, similar to how plugins are commonly suffixed with `Plugin` (ex: `TimePlugin`). By adopting a consistent naming convention in first-party Bevy, we can softly nudge ecosystem crates to follow suit (for types where it makes sense to do so). Choosing a naming convention is also relevant now, as the [`bevy_cli` recently adopted lints](https://github.com/TheBevyFlock/bevy_cli/pull/345) to enforce naming for plugins and system sets, and the recommended naming used for system sets is still a bit open. ## Which Name To Use? Now the contentious part: what naming convention should we actually adopt? This was discussed on the Bevy Discord at the end of last year, starting [here](<https://discord.com/channels/691052431525675048/692572690833473578/1310659954683936789>). `FooSet` and `FooSystems` were the clear favorites, with `FooSet` very narrowly winning an unofficial poll. However, it seems to me like the consensus was broadly moving towards `FooSystems` at the end and after the poll, with Cart ([source](https://discord.com/channels/691052431525675048/692572690833473578/1311140204974706708)) and later Alice ([source](https://discord.com/channels/691052431525675048/692572690833473578/1311092530732859533)) and also me being in favor of it. Let's do a quick pros and cons list! Of course these are just what I thought of, so take it with a grain of salt. `FooSet`: - Pro: Nice and short! - Pro: Used by many ecosystem crates. - Pro: The `Set` suffix comes directly from the trait name `SystemSet`. - Pro: Pairs nicely with existing APIs like `in_set` and `configure_sets`. - Con: `Set` by itself doesn't actually indicate that it's related to systems *at all*, apart from the implemented trait. A set of what? - Con: Is `FooSet` a set of `Foo`s or a system set related to `Foo`? Ex: `ContactSet`, `MeshSet`, `EnemySet`... `FooSystems`: - Pro: Very clearly indicates that the type represents a collection of systems. The actual core concept, system(s), is in the name. - Pro: Parallels nicely with `FooPlugins` for plugin groups. - Pro: Low risk of conflicts with other names or misunderstandings about what the type is. - Pro: In most cases, reads *very* nicely and clearly. Ex: `PhysicsSystems` and `AnimationSystems` as opposed to `PhysicsSet` and `AnimationSet`. - Pro: Easy to search for on docs.rs. - Con: Usually results in longer names. - Con: Not yet as widely used. Really the big problem with `FooSet` is that it doesn't actually describe what it is. It describes what *kind of thing* it is (a set of something), but not *what it is a set of*, unless you know the type or check its docs or implemented traits. `FooSystems` on the other hand is much more self-descriptive in this regard, at the cost of being a bit longer to type. Ultimately, in some ways it comes down to preference and how you think of system sets. Personally, I was originally in favor of `FooSet`, but have been increasingly on the side of `FooSystems`, especially after seeing what the new names would actually look like in Avian and now Bevy. I prefer it because it usually reads better, is much more clearly related to groups of systems than `FooSet`, and overall *feels* more correct and natural to me in the long term. For these reasons, and because Alice and Cart also seemed to share a preference for it when it was previously being discussed, I propose that we adopt a `FooSystems` naming convention where applicable. ## Solution Rename Bevy's system set types to use a consistent `FooSet` naming where applicable. - `AccessibilitySystem` → `AccessibilitySystems` - `GizmoRenderSystem` → `GizmoRenderSystems` - `PickSet` → `PickingSystems` - `RunFixedMainLoopSystem` → `RunFixedMainLoopSystems` - `TransformSystem` → `TransformSystems` - `RemoteSet` → `RemoteSystems` - `RenderSet` → `RenderSystems` - `SpriteSystem` → `SpriteSystems` - `StateTransitionSteps` → `StateTransitionSystems` - `RenderUiSystem` → `RenderUiSystems` - `UiSystem` → `UiSystems` - `Animation` → `AnimationSystems` - `AssetEvents` → `AssetEventSystems` - `TrackAssets` → `AssetTrackingSystems` - `UpdateGizmoMeshes` → `GizmoMeshSystems` - `InputSystem` → `InputSystems` - `InputFocusSet` → `InputFocusSystems` - `ExtractMaterialsSet` → `MaterialExtractionSystems` - `ExtractMeshesSet` → `MeshExtractionSystems` - `RumbleSystem` → `RumbleSystems` - `CameraUpdateSystem` → `CameraUpdateSystems` - `ExtractAssetsSet` → `AssetExtractionSystems` - `Update2dText` → `Text2dUpdateSystems` - `TimeSystem` → `TimeSystems` - `AudioPlaySet` → `AudioPlaybackSystems` - `SendEvents` → `EventSenderSystems` - `EventUpdates` → `EventUpdateSystems` A lot of the names got slightly longer, but they are also a lot more consistent, and in my opinion the majority of them read much better. For a few of the names I took the liberty of rewording things a bit; definitely open to any further naming improvements. There are still also cases where the `FooSystems` naming doesn't really make sense, and those I left alone. This primarily includes system sets like `Interned<dyn SystemSet>`, `EnterSchedules<S>`, `ExitSchedules<S>`, or `TransitionSchedules<S>`, where the type has some special purpose and semantics. ## Todo - [x] Should I keep all the old names as deprecated type aliases? I can do this, but to avoid wasting work I'd prefer to first reach consensus on whether these renames are even desired. - [x] Migration guide - [x] Release notes |
||
|
Gino Valente
|
9b32e09551
|
bevy_reflect: Add clone registrations project-wide (#18307)
# Objective Now that #13432 has been merged, it's important we update our reflected types to properly opt into this feature. If we do not, then this could cause issues for users downstream who want to make use of reflection-based cloning. ## Solution This PR is broken into 4 commits: 1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that are also `Clone`. This is mandatory as these types would otherwise cause the cloning operation to fail for any type that contains it at any depth. 2. Update the reflection example to suggest adding `#[reflect(Clone)]` on opaque types. 3. Add `#[reflect(clone)]` attributes on all fields marked `#[reflect(ignore)]` that are also `Clone`. This prevents the ignored field from causing the cloning operation to fail. Note that some of the types that contain these fields are also `Clone`, and thus can be marked `#[reflect(Clone)]`. This makes the `#[reflect(clone)]` attribute redundant. However, I think it's safer to keep it marked in the case that the `Clone` impl/derive is ever removed. I'm open to removing them, though, if people disagree. 4. Finally, I added `#[reflect(Clone)]` on all types that are also `Clone`. While not strictly necessary, it enables us to reduce the generated output since we can just call `Clone::clone` directly instead of calling `PartialReflect::reflect_clone` on each variant/field. It also means we benefit from any optimizations or customizations made in the `Clone` impl, including directly dereferencing `Copy` values and increasing reference counters. Along with that change I also took the liberty of adding any missing registrations that I saw could be applied to the type as well, such as `Default`, `PartialEq`, and `Hash`. There were hundreds of these to edit, though, so it's possible I missed quite a few. That last commit is **_massive_**. There were nearly 700 types to update. So it's recommended to review the first three before moving onto that last one. Additionally, I can break the last commit off into its own PR or into smaller PRs, but I figured this would be the easiest way of doing it (and in a timely manner since I unfortunately don't have as much time as I used to for code contributions). ## Testing You can test locally with a `cargo check`: ``` cargo check --workspace --all-features ``` |
||
|
newclarityex
|
ecccd57417
|
Generic system config (#17962)
# Objective Prevents duplicate implementation between IntoSystemConfigs and IntoSystemSetConfigs using a generic, adds a NodeType trait for more config flexibility (opening the door to implement https://github.com/bevyengine/bevy/issues/14195?). ## Solution Followed writeup by @ItsDoot: https://hackmd.io/@doot/rJeefFHc1x Removes IntoSystemConfigs and IntoSystemSetConfigs, instead using IntoNodeConfigs with generics. ## Testing Pending --- ## Showcase N/A ## Migration Guide SystemSetConfigs -> NodeConfigs<InternedSystemSet> SystemConfigs -> NodeConfigs<ScheduleSystem> IntoSystemSetConfigs -> IntoNodeConfigs<InternedSystemSet, M> IntoSystemConfigs -> IntoNodeConfigs<ScheduleSystem, M> --------- Co-authored-by: Christian Hughes <9044780+ItsDoot@users.noreply.github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
raldone01
|
1b7db895b7
|
Harden proc macro path resolution and add integration tests. (#17330)
This pr uses the `extern crate self as` trick to make proc macros behave the same way inside and outside bevy. # Objective - Removes noise introduced by `crate as` in the whole bevy repo. - Fixes #17004. - Hardens proc macro path resolution. ## TODO - [x] `BevyManifest` needs cleanup. - [x] Cleanup remaining `crate as`. - [x] Add proper integration tests to the ci. ## Notes - `cargo-manifest-proc-macros` is written by me and based/inspired by the old `BevyManifest` implementation and [`bkchr/proc-macro-crate`](https://github.com/bkchr/proc-macro-crate). - What do you think about the new integration test machinery I added to the `ci`? More and better integration tests can be added at a later stage. The goal of these integration tests is to simulate an actual separate crate that uses bevy. Ideally they would lightly touch all bevy crates. ## Testing - Needs RA test - Needs testing from other users - Others need to run at least `cargo run -p ci integration-test` and verify that they work. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Blazepaws
|
379696a468
|
Reflect derived traits on all components and resources: bevy_gizmos (#15217)
Solves https://github.com/bevyengine/bevy/issues/15187 for bevy_gizmos |
||
|
Alice Cecile
|
4ac2a63556
|
Remove all existing system order ambiguities in DefaultPlugins (#15031)
# Objective As discussed in https://github.com/bevyengine/bevy/issues/7386, system order ambiguities within `DefaultPlugins` are a source of bugs in the engine and badly pollute diagnostic output for users. We should eliminate them! This PR is an alternative to #15027: with all external ambiguities silenced, this should be much less prone to merge conflicts and the test output should be much easier for authors to understand. Note that system order ambiguities are still permitted in the `RenderApp`: these need a bit of thought in terms of how to test them, and will be fairly involved to fix. While these aren't *good*, they'll generally only cause graphical bugs, not logic ones. ## Solution All remaining system order ambiguities have been resolved. Review this PR commit-by-commit to see how each of these problems were fixed. ## Testing `cargo run --example ambiguity_detection` passes with no panics or logging! |
||
|
Alice Cecile
|
599e5e4e76
|
Migrate from LegacyColor to bevy_color::Color (#12163)
# Objective - As part of the migration process we need to a) see the end effect of the migration on user ergonomics b) check for serious perf regressions c) actually migrate the code - To accomplish this, I'm going to attempt to migrate all of the remaining user-facing usages of `LegacyColor` in one PR, being careful to keep a clean commit history. - Fixes #12056. ## Solution I've chosen to use the polymorphic `Color` type as our standard user-facing API. - [x] Migrate `bevy_gizmos`. - [x] Take `impl Into<Color>` in all `bevy_gizmos` APIs - [x] Migrate sprites - [x] Migrate UI - [x] Migrate `ColorMaterial` - [x] Migrate `MaterialMesh2D` - [x] Migrate fog - [x] Migrate lights - [x] Migrate StandardMaterial - [x] Migrate wireframes - [x] Migrate clear color - [x] Migrate text - [x] Migrate gltf loader - [x] Register color types for reflection - [x] Remove `LegacyColor` - [x] Make sure CI passes Incidental improvements to ease migration: - added `Color::srgba_u8`, `Color::srgba_from_array` and friends - added `set_alpha`, `is_fully_transparent` and `is_fully_opaque` to the `Alpha` trait - add and immediately deprecate (lol) `Color::rgb` and friends in favor of more explicit and consistent `Color::srgb` - standardized on white and black for most example text colors - added vector field traits to `LinearRgba`: ~~`Add`, `Sub`, `AddAssign`, `SubAssign`,~~ `Mul<f32>` and `Div<f32>`. Multiplications and divisions do not scale alpha. `Add` and `Sub` have been cut from this PR. - added `LinearRgba` and `Srgba` `RED/GREEN/BLUE` - added `LinearRgba_to_f32_array` and `LinearRgba::to_u32` ## Migration Guide Bevy's color types have changed! Wherever you used a `bevy::render::Color`, a `bevy::color::Color` is used instead. These are quite similar! Both are enums storing a color in a specific color space (or to be more precise, using a specific color model). However, each of the different color models now has its own type. TODO... - `Color::rgba`, `Color::rgb`, `Color::rbga_u8`, `Color::rgb_u8`, `Color::rgb_from_array` are now `Color::srgba`, `Color::srgb`, `Color::srgba_u8`, `Color::srgb_u8` and `Color::srgb_from_array`. - `Color::set_a` and `Color::a` is now `Color::set_alpha` and `Color::alpha`. These are part of the `Alpha` trait in `bevy_color`. - `Color::is_fully_transparent` is now part of the `Alpha` trait in `bevy_color` - `Color::r`, `Color::set_r`, `Color::with_r` and the equivalents for `g`, `b` `h`, `s` and `l` have been removed due to causing silent relatively expensive conversions. Convert your `Color` into the desired color space, perform your operations there, and then convert it back into a polymorphic `Color` enum. - `Color::hex` is now `Srgba::hex`. Call `.into` or construct a `Color::Srgba` variant manually to convert it. - `WireframeMaterial`, `ExtractedUiNode`, `ExtractedDirectionalLight`, `ExtractedPointLight`, `ExtractedSpotLight` and `ExtractedSprite` now store a `LinearRgba`, rather than a polymorphic `Color` - `Color::rgb_linear` and `Color::rgba_linear` are now `Color::linear_rgb` and `Color::linear_rgba` - The various CSS color constants are no longer stored directly on `Color`. Instead, they're defined in the `Srgba` color space, and accessed via `bevy::color::palettes::css`. Call `.into()` on them to convert them into a `Color` for quick debugging use, and consider using the much prettier `tailwind` palette for prototyping. - The `LIME_GREEN` color has been renamed to `LIMEGREEN` to comply with the standard naming. - Vector field arithmetic operations on `Color` (add, subtract, multiply and divide by a f32) have been removed. Instead, convert your colors into `LinearRgba` space, and perform your operations explicitly there. This is particularly relevant when working with emissive or HDR colors, whose color channel values are routinely outside of the ordinary 0 to 1 range. - `Color::as_linear_rgba_f32` has been removed. Call `LinearRgba::to_f32_array` instead, converting if needed. - `Color::as_linear_rgba_u32` has been removed. Call `LinearRgba::to_u32` instead, converting if needed. - Several other color conversion methods to transform LCH or HSL colors into float arrays or `Vec` types have been removed. Please reimplement these externally or open a PR to re-add them if you found them particularly useful. - Various methods on `Color` such as `rgb` or `hsl` to convert the color into a specific color space have been removed. Convert into `LinearRgba`, then to the color space of your choice. - Various implicitly-converting color value methods on `Color` such as `r`, `g`, `b` or `h` have been removed. Please convert it into the color space of your choice, then check these properties. - `Color` no longer implements `AsBindGroup`. Store a `LinearRgba` internally instead to avoid conversion costs. --------- Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com> Co-authored-by: Afonso Lage <lage.afonso@gmail.com> Co-authored-by: Rob Parrett <robparrett@gmail.com> Co-authored-by: Zachary Harrold <zac@harrold.com.au> |
||
|
Zachary Harrold
|
043041f3aa
|
bevy_color: Add sequence_dispersed to Hsla, Lcha, Oklcha (#12173)
# Objective - Fixes #12170 ## Solution - Moved the existing `color_from_entity` internals into `Hsla::sequence_dispersed` which generates a randomly distributed but deterministic color sequence based. - Replicated the method for `Lcha` and `Oklcha` as well. ## Examples ### Getting a few colours for a quick palette ```rust let palette = Hsla::sequence_dispersed().take(5).collect::<Vec<_>>(); /*[ Hsla::hsl(0.0, 1., 0.5), Hsla::hsl(222.49225, 1., 0.5), Hsla::hsl(84.984474, 1., 0.5), Hsla::hsl(307.4767, 1., 0.5), Hsla::hsl(169.96895, 1., 0.5), ]*/ ``` ### Getting a colour from an `Entity` ```rust let color = Oklcha::sequence_dispersed().nth(entity.index() as u32).unwrap(); ``` ## Notes This was previously a private function exclusively for `Entity` types. I've decided it should instead be public and operate on a `u32` directly, since this function may have broader uses for debugging purposes. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|
Alice Cecile
|
de004da8d5
|
Rename bevy_render::Color to LegacyColor (#12069)
# Objective The migration process for `bevy_color` (#12013) will be fairly involved: there will be hundreds of affected files, and a large number of APIs. ## Solution To allow us to proceed granularly, we're going to keep both `bevy_color::Color` (new) and `bevy_render::Color` (old) around until the migration is complete. However, simply doing this directly is confusing! They're both called `Color`, making it very hard to tell when a portion of the code has been ported. As discussed in #12056, by renaming the old `Color` type, we can make it easier to gradually migrate over, one API at a time. ## Migration Guide THIS MIGRATION GUIDE INTENTIONALLY LEFT BLANK. This change should not be shipped to end users: delete this section in the final migration guide! --------- Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com> |
||
|
jeliag
|
f6b40a6e43
|
Multiple Configurations for Gizmos (#10342)
# Objective This PR aims to implement multiple configs for gizmos as discussed in #9187. ## Solution Configs for the new `GizmoConfigGroup`s are stored in a `GizmoConfigStore` resource and can be accesses using a type based key or iterated over. This type based key doubles as a standardized location where plugin authors can put their own configuration not covered by the standard `GizmoConfig` struct. For example the `AabbGizmoGroup` has a default color and toggle to show all AABBs. New configs can be registered using `app.init_gizmo_group::<T>()` during startup. When requesting the `Gizmos<T>` system parameter the generic type determines which config is used. The config structs are available through the `Gizmos` system parameter allowing for easy access while drawing your gizmos. Internally, resources and systems used for rendering (up to an including the extract system) are generic over the type based key and inserted on registering a new config. ## Alternatives The configs could be stored as components on entities with markers which would make better use of the ECS. I also implemented this approach ([here](https://github.com/jeliag/bevy/tree/gizmo-multiconf-comp)) and believe that the ergonomic benefits of a central config store outweigh the decreased use of the ECS. ## Unsafe Code Implementing system parameter by hand is unsafe but seems to be required to access the config store once and not on every gizmo draw function call. This is critical for performance. ~Is there a better way to do this?~ ## Future Work New gizmos (such as #10038, and ideas from #9400) will require custom configuration structs. Should there be a new custom config for every gizmo type, or should we group them together in a common configuration? (for example `EditorGizmoConfig`, or something more fine-grained) ## Changelog - Added `GizmoConfigStore` resource and `GizmoConfigGroup` trait - Added `init_gizmo_group` to `App` - Added early returns to gizmo drawing increasing performance when gizmos are disabled - Changed `GizmoConfig` and aabb gizmos to use new `GizmoConfigStore` - Changed `Gizmos` system parameter to use type based key to retrieve config - Changed resources and systems used for gizmo rendering to be generic over type based key - Changed examples (3d_gizmos, 2d_gizmos) to showcase new API ## Migration Guide - `GizmoConfig` is no longer a resource and has to be accessed through `GizmoConfigStore` resource. The default config group is `DefaultGizmoGroup`, but consider using your own custom config group if applicable. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com> |