Skip to content

Conversation

@PixelDust22
Copy link
Contributor

@PixelDust22 PixelDust22 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
@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

@PixelDust22
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.
@PixelDust22 PixelDust22 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