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

Fix bevy_window failing with serialize feature #14298

Merged
merged 4 commits into from
Jul 15, 2024

Conversation

BD103
Copy link
Member

@BD103 BD103 commented Jul 14, 2024

Objective

  • flag-frenzy found an issue where bevy_window would fail to build when its serialize feature is enabled.
    • See here for the specific log.

Solution

  • Turns out it was failing because the bevy_ecs/serialize feature was not enabled. This error can be fixed by adding the flag as a dependency.

Testing

cargo check -p bevy_window -F serialize
# Or if you're very cool...
flag-frenzy --manifest-path path/to/bevy/Cargo.toml --config config -p bevy_window

@BD103 BD103 added C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy A-Windowing Platform-agnostic interface layer to run your app in C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024
Copy link
Contributor

@benfrankel benfrankel left a comment

Choose a reason for hiding this comment

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

I only read the PR description and reviewed the code diff. Have not investigated whether this is correct or the potential implications.

@MrGVSV MrGVSV added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 14, 2024
@BD103
Copy link
Member Author

BD103 commented Jul 14, 2024

@alice-i-cecile should the bug fix be backported to 0.14.1? If so, I should probably split this into two different PRs.

@alice-i-cecile
Copy link
Member

Yes, please split this for backporting.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed 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 14, 2024
@BD103 BD103 added this to the 0.14.1 milestone Jul 14, 2024
@BD103 BD103 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 14, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 15, 2024
Merged via the queue into bevyengine:main with commit b6f61b3 Jul 15, 2024
31 checks passed
@BD103 BD103 deleted the windows-features branch July 15, 2024 16:32
github-merge-queue bot pushed a commit that referenced this pull request Jul 15, 2024
# Objective

- Extracted from #14298.
- `bevy_window` has an empty `default` feature that does not enable
anything, which is equivalent to not having any default features.

## Solution

- Remove it :)
- This is technically a breaking change, but specifying `features =
["default"]` manually in `Cargo.toml` is highly discouraged, so the
impact is low.

---

## Migration Guide

`bevy_window` had an empty default feature flag that did not do
anything, so it was removed. You may have to remove any references to it
if you specified it manually.

```toml
# 0.14
[dependencies]
bevy_window = { version = "0.14", default-features = false, features = ["default"] }

# 0.15
[dependencies]
bevy_window = { version = "0.15", default-features = false }
```
mockersf pushed a commit that referenced this pull request Aug 2, 2024
# Objective

- [`flag-frenzy`](https://github.com/TheBevyFlock/flag-frenzy) found an
issue where `bevy_window` would fail to build when its `serialize`
feature is enabled.
- See
[here](https://github.com/TheBevyFlock/flag-frenzy/actions/runs/9924187577/job/27415224405)
for the specific log.

## Solution

- Turns out it was failing because the `bevy_ecs/serialize` feature was
not enabled. This error can be fixed by adding the flag as a dependency.

## Testing

```bash
cargo check -p bevy_window -F serialize
# Or if you're very cool...
flag-frenzy --manifest-path path/to/bevy/Cargo.toml --config config -p bevy_window
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy 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