Skip to content

multi_threaded feature rename#12997

Merged
mockersf merged 2 commits intobevyengine:mainfrom
andristarr:multi_threaded.feature.rename
May 6, 2024
Merged

multi_threaded feature rename#12997
mockersf merged 2 commits intobevyengine:mainfrom
andristarr:multi_threaded.feature.rename

Conversation

@andristarr
Copy link
Contributor

@andristarr andristarr commented Apr 16, 2024 •
edited
Loading

Objective

Fixes #12966

Solution

Renaming multi_threaded feature to match snake case

Migration Guide

Bevy feature multi-threaded should be refered to multi_threaded from now on.

@andristarr
Copy link
Contributor Author

andristarr commented Apr 16, 2024

basis-universal and the symphonia-* features seem to be package names so I suspect having kebab case there is fine?

Do we also want to rename the internal name of the packages? Not sure the exact technical term, but do we want bevy_internal to export multi_threaded or we are content with kebab case there?

@andristarr andristarr force-pushed the multi_threaded.feature.rename branch from ab5c689 to baefa7f Compare April 16, 2024 16:08
@james7132 james7132 added A-ECS Entities, components, systems, and events C-Code-Quality A section of code that is hard to understand or change A-Tasks Tools for parallel and async work M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Apr 16, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Apr 16, 2024

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

james7132
james7132 requested changes Apr 17, 2024
View reviewed changes
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the cfg annotations that rely on the feature should be updated too.

@mockersf
Copy link
Member

mockersf commented Apr 17, 2024

This PR only rename the feature on the bevy crate, so any use is not impacted.

But I would prefer to rename the feature in all crates for consistency.

@andristarr andristarr force-pushed the multi_threaded.feature.rename branch 3 times, most recently from 91ed79d to 91f3d61 Compare April 18, 2024 09:32
james7132
james7132 reviewed Apr 19, 2024
View reviewed changes
@andristarr andristarr force-pushed the multi_threaded.feature.rename branch from 91f3d61 to 5bc5626 Compare April 22, 2024 12:16
@james7132 james7132 requested a review from mockersf April 23, 2024 00:37
mockersf
mockersf reviewed Apr 23, 2024
View reviewed changes
crates/bevy_tasks/README.md Outdated
Comment on lines +28 to +32
- For CPU-intensive work (tasks that generally spin until completion) we have a standard
[`ComputeTaskPool`] and an [`AsyncComputeTaskPool`]. Work that does not need to be completed to
present the next frame should go to the [`AsyncComputeTaskPool`].

* For IO-intensive work (tasks that spend very little time in a "woken" state) we have an
- For IO-intensive work (tasks that spend very little time in a "woken" state) we have an
Copy link
Member

@mockersf mockersf Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those changes seems unrelated

Copy link
Contributor Author

@andristarr andristarr Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now addressed

@andristarr andristarr force-pushed the multi_threaded.feature.rename branch from 5bc5626 to e1294fd Compare April 25, 2024 15:53
@andristarr
Copy link
Contributor Author

andristarr commented May 6, 2024

@mockersf this should be ready for re-review

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label May 6, 2024
@mockersf mockersf added this pull request to the merge queue May 6, 2024
Merged via the queue into bevyengine:main with commit bb76a2c May 6, 2024
ChristopherBiscardi added a commit to ChristopherBiscardi/bevy_ecs_tilemap that referenced this pull request Jun 7, 2024
@ChristopherBiscardi
Update to 0.14.0-rc.2
cf3df98 
* [12997](bevyengine/bevy#12997): rename `multi-threaded` to `multi_threaded`
@ChristopherBiscardi ChristopherBiscardi mentioned this pull request Jun 7, 2024
Merged
rparrett added a commit to StarArawn/bevy_ecs_tilemap that referenced this pull request Jul 5, 2024
@ChristopherBiscardi @rparrett
Update to 0.14.0 (#537)
aee4779 
* Update to 0.14.0-rc.2

* [12997](bevyengine/bevy#12997): rename `multi-threaded` to `multi_threaded`

* RenderAssets<Image> is now RenderAssets<GpuImage>

Implemented in [12827](bevyengine/bevy#12827)

* FloatOrd is now in bevy_math

implemented in [12732](bevyengine/bevy#12732)

* convert Transparent2d::dynamic_offset to extra_index

[12889](bevyengine/bevy#12889) Gpu Frustum Culling removed the dynamic_offset of Transparent2d and it became `extra_index` with the special value `PhaseItemExtraIndex::NONE`, which indicates the `None` that was here previously

* RenderPhase<Transparent2d> -> ViewSortedRenderPhases<Transparent2d>

[12453](https://github.com/StarArawn/bevy_ecs_tilemap/pull/bevyengine/bevy#12453): Render phases are now binned or sorted.

Following the changes in the `mesh2d_manual` [example](https://github.com/bevyengine/bevy/blob/ecdd1624f302c5f71aaed95b0984cbbecf8880b7/examples/2d/mesh2d_manual.rs#L357-L358): use the `ViewSortedRenderPhases` resource.

* get_sub_app_mut is now an Option

in [9202](https://github.com/StarArawn/bevy_ecs_tilemap/pull/bevyengine/bevy/pull/9202) SubApp access has changed

* GpuImage::size f32 -> u32 via UVec2

[11698](bevyengine/bevy#11698) changed `GpuImage::size` to `UVec2`.

Right above this, `Extent3d` does the same thing, so I'm taking a small leap and assuming can `as`.

* GpuMesh::primitive_topology -> key_bits/BaseMeshPipeline

[12791](bevyengine/bevy#12791) the `primitive_topology` field on `GpuMesh` was removed in favor of `key_bits` which can be constructed using `BaseMeshPipeline::from_primitive_topology`

* RenderChunk2d::prepare requires &mut MeshVertexBufferLayouts now

[12216](bevyengine/bevy#12216) introduced an argument `&mut MeshVertexBufferLayouts` to `get_mesh_vertex_buffer_layout`, which bevy_ecs_tilemap calls in `RenderChunk2d::prepare`

* into_linear_f32 -> color.0.linear().to_f32_array(),

[12163](bevyengine/bevy#12163) bevy_color was created and Color handling has changed. Specifically Color::as_linear_rgba_f32 has been removed.

LinearRgba is now its own type that can be accessed via [`linear()`](https://docs.rs/bevy/0.14.0-rc.2/bevy/color/enum.Color.html#method.linear) and then converted.

* Must specify type of VisibleEntities when accessing

[12582](bevyengine/bevy#12582) divided `VisibleEntities` into separate lists. So now we have to specify which kind of entity we want. I think we want the Mesh here, and I think we can get rid of the `.index` calls on Entity since Entity [already compares bits](https://docs.rs/bevy_ecs/0.14.0-rc.2/src/bevy_ecs/entity/mod.rs.html#173) for optimized codegen purposes. Waiting to do that until the other changes are in though so as to not change functionality until post-upgrade.

* app.world access is functions now

- [9202](bevyengine/bevy#9202) changed world access to functions. [relevent line](https://github.com/bevyengine/bevy/pull/9202/files#diff-b2fba3a0c86e496085ce7f0e3f1de5960cb754c7d215ed0f087aa556e529f97fR640)
- This also surfaced [12655](bevyengine/bevy#12655) which removed `Into<AssetId<T>>` for `Handle<T>`. using a reference or .id() is the solution here.

* We don't need `World::cell`, and it doesn't exist anymore

In [12551](bevyengine/bevy#12551) `WorldCell` was removed.

...but it turns out we don't need it or its replacement anyway.

* examples error out unless this bevy bug is addressed with these features being added

bevyengine/bevy#13728

* check_visibility is required for the entity that is renderable

As a result of [12582](bevyengine/bevy#12582) `check_visibility` must be implemented for the "renderable" tilemap entities. Doing this is trivial by taking advantage of the
existing `check_visibility` type arguments, which accept a [`QF: QueryFilter + 'static`](https://docs.rs/bevy/0.14.0-rc.2/bevy/render/view/fn.check_visibility.html).

The same `QueryFilter`` is used when checking `VisibleEntities`. I've chosen `With<TilemapRenderSettings` because presumably if the entity doesn't have a `TilemapRenderSettings` then it will not be rendering, but this could be as sophisticated or simple as we want.

For example `WithLight` is currently implemented as

```rust
pub type WithLight = Or<(With<PointLight>, With<SpotLight>, With<DirectionalLight>)>;
```

* view.view_proj -> view.clip_from_world

[13289](bevyengine/bevy#13489) introduced matrix naming changes, including `view_proj` which becomes `clip_from_world`

* color changes to make tests runnable

* clippy fix

* Update Cargo.toml

Co-authored-by: Rob Parrett <robparrett@gmail.com>

* Update Cargo.toml

Co-authored-by: Rob Parrett <robparrett@gmail.com>

* final clippy fixes

* Update Cargo.toml

Co-authored-by: Rob Parrett <robparrett@gmail.com>

* Simplify async loading in ldtk/tiled helpers

See Bevy #12550

* remove second allow lint

* rc.3 bump

* bump version for major release

* remove unused features

---------

Co-authored-by: Rob Parrett <robparrett@gmail.com>
@smoked-dev
Copy link

smoked-dev commented Nov 17, 2024

Hey! I know this is probably not Github etiquette, but nonetheless I felt compelled to comment that these sorts of changes are breaking, and they cause allot of pain and suffering for people.

I know it feels good to get a green dot on the github calendar and a "pull-shark" Github badge or whatever, but this sort of stuff hurts devs and wastes work hours. "multi-threaded" was more correct anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@james7132 james7132 james7132 approved these changes

@mockersf mockersf mockersf approved these changes

Assignees

No one assigned

Labels

A-ECS Entities, components, systems, and events A-Tasks Tools for parallel and async work C-Code-Quality A section of code that is hard to understand or change M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

multi-threaded feature name is kebab case but all others are snake case

4 participants

@andristarr @mockersf @smoked-dev @james7132