Skip to content

[Merged by Bors] - Lighter no default features #5447

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[Merged by Bors] - Lighter no default features #5447

wants to merge 2 commits into from

Conversation

@mockersf
Copy link
Member

@mockersf mockersf commented Jul 24, 2022 •
edited
Loading

Objective

  • Even though it's marked as optional, it is no longer possible to not depend on bevy_render as it's a dependency of bevy_scene

Solution

  • Make bevy_scene optional
  • For the minimalist among us, also make bevy_asset optional

@Nilirad
Copy link
Contributor

Nilirad commented Jul 25, 2022

What part of bevy_render is needed by bevy_scene? Wouldn't it be better to outsource the those parts in a more neutral crate such that bevy_scene depends on that instead of bevy_render?

@mockersf
Copy link
Member Author

mockersf commented Jul 25, 2022 •
edited
Loading

It's used for the visibility components. They could be pulled into their own crate like Transform was pulled into bevy_transform, but that's a bigger refactor.

For my use case I mostly want a very light default-featureless Bevy, so it makes sense to have bevy_scene as optional anyway. I would hope to get it in 0.8 because not being able to disable bevy_render is going to be blocking for some use cases

@Nilirad
Copy link
Contributor

Nilirad commented Jul 25, 2022

We may consider having a bevy_spatial crate or something in the future, encompassing both transform and visibility.

I agree though that as a hot fix this should be merged to avoid pulling in bevy_render in a minimal build.

@alice-i-cecile alice-i-cecile added A-Meta About the project itself 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 labels Jul 25, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 25, 2022

bors r+

bors bot pushed a commit that referenced this pull request Jul 25, 2022
@mockersf
Lighter no default features (#5447)
231894a 
# Objective

- Even though it's marked as optional, it is no longer possible to not depend on `bevy_render` as it's a dependency of `bevy_scene`

## Solution

- Make `bevy_scene` optional
- For the minimalist among us, also make `bevy_asset` optional
@bors
Copy link
Contributor

bors bot commented Jul 25, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Lighter no default features [Merged by Bors] - Lighter no default features Jul 25, 2022
@bors bors bot closed this Jul 25, 2022
@Nilirad Nilirad added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jul 26, 2022
@Nilirad
Copy link
Contributor

Nilirad commented Jul 26, 2022

This introduces a breaking change.

Migration guide

Assets and scenes are now behind a feature flag. So this migration is only needed if you are depending to the bevy crate with default-features = false in Cargo.toml and you are using assets or scenes. You will have to add the feature bevy_asset or bevy_scene to your bevy dependency (and also the dev-dependency if you have it).

@Nilirad Nilirad mentioned this pull request Jul 26, 2022
Merged
@alice-i-cecile alice-i-cecile mentioned this pull request Jul 28, 2022
Merged
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
@mockersf @inodentry
Lighter no default features (bevyengine#5447)
9c31ca1 
# Objective

- Even though it's marked as optional, it is no longer possible to not depend on `bevy_render` as it's a dependency of `bevy_scene`

## Solution

- Make `bevy_scene` optional
- For the minimalist among us, also make `bevy_asset` optional
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@mockersf @james7132
Lighter no default features (bevyengine#5447)
5d6bb7e 
# Objective

- Even though it's marked as optional, it is no longer possible to not depend on `bevy_render` as it's a dependency of `bevy_scene`

## Solution

- Make `bevy_scene` optional
- For the minimalist among us, also make `bevy_asset` optional
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@mockersf @ItsDoot
Lighter no default features (bevyengine#5447)
1a1fa56 
# Objective

- Even though it's marked as optional, it is no longer possible to not depend on `bevy_render` as it's a dependency of `bevy_scene`

## Solution

- Make `bevy_scene` optional
- For the minimalist among us, also make `bevy_asset` optional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@tim-blackbird tim-blackbird tim-blackbird approved these changes

@Nilirad Nilirad Nilirad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Meta About the project itself C-Usability A targeted quality-of-life change that makes Bevy easier to use 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.

4 participants

@mockersf @Nilirad @alice-i-cecile @tim-blackbird