Skip to content
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

Make bevy_render an optional dependency of bevy_scene #8136

Merged
merged 7 commits into from
Apr 3, 2023

Conversation

Neo-Zhixing
Copy link
Contributor

@Neo-Zhixing Neo-Zhixing commented Mar 20, 2023

Objective

bevy-scene does not have a reason to depend on bevy-render except to include the Visibility and ComputedVisibility components. Including that in the dependency chain is unnecessary for people not using bevy_render.

Also fixed a problem where compilation fails when the serialize feature was not enabled.

Solution

This was added in #5335 to address some of the problems caused by #5310.

Imo the user just always have to remember to include VisibilityBundle when they spawn SceneBundle or DynamicSceneBundle, but that will be a breaking change. This PR makes bevy_render an optional dependency of bevy_scene instead to respect the existing behavior.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Scenes Serialized ECS data stored on the disk labels Mar 20, 2023
crates/bevy_scene/Cargo.toml Outdated Show resolved Hide resolved
@mockersf
Copy link
Member

mockersf commented Mar 20, 2023

this doesn't makes it possible to enable the bevy_scene feature without depending on bevy_render

@Neo-Zhixing
Copy link
Contributor Author

@mockersf I've made it so that bevy_scene/bevy_render only gets enabled when bevy_render was enabled.

@james7132 james7132 requested a review from mockersf March 30, 2023 10:24
@james7132 james7132 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 Mar 30, 2023
@mockersf
Copy link
Member

@Neo-Zhixing there's just a small conflict, once you resolved it we can merge!

@mockersf mockersf added this pull request to the merge queue Apr 3, 2023
Merged via the queue into bevyengine:main with commit 2aaaed7 Apr 3, 2023
Estus-Dev pushed a commit to Estus-Dev/bevy that referenced this pull request Jul 10, 2023
# Objective

bevy-scene does not have a reason to depend on bevy-render except to
include the `Visibility` and `ComputedVisibility` components. Including
that in the dependency chain is unnecessary for people not using
`bevy_render`.

Also fixed a problem where compilation fails when the `serialize`
feature was not enabled.

## Solution

This was added in bevyengine#5335 to address some of the problems caused by bevyengine#5310.

Imo the user just always have to remember to include `VisibilityBundle`
when they spawn `SceneBundle` or `DynamicSceneBundle`, but that will be
a breaking change. This PR makes `bevy_render` an optional dependency of
`bevy_scene` instead to respect the existing behavior.
@Neo-Zhixing Neo-Zhixing deleted the bevy_scene_remove_render branch December 28, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-Scenes Serialized ECS data stored on the disk C-Usability A targeted quality-of-life change that makes Bevy easier to use 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
Development

Successfully merging this pull request may close these issues.

5 participants