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_remote feature enable serialize feature #17260

Merged

Conversation

mweatherley
Copy link
Contributor

Objective

bevy_remote's reflection deserialization basically requires ReflectDeserialize registrations in order to work correctly. In the context of bevy (the library), this means that using bevy_remote without using the serialize feature is a footgun, since #[reflect(Serialize)] etc. are gated behind this feature.

The goal of this PR is to avoid this mistake by default.

Solution

Make the bevy_remote feature enable the serialize feature, so that it works as expected.


Migration Guide

The bevy_remote feature of bevy now enables the serialize feature automatically. If you wish to use bevy_remote without enabling the serialize feature for Bevy subcrates, you must import bevy_remote on its own.

@mweatherley mweatherley added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Reflection Runtime information about types S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through labels Jan 9, 2025
@mweatherley
Copy link
Contributor Author

I'm not sure if there is some protocol about whether this should be done at the level of bevy or bevy_internal, but either is fine with me :)

@mweatherley mweatherley added the D-Trivial Nice and easy! A great choice to get started with Bevy label Jan 9, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 10, 2025
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 10, 2025
Merged via the queue into bevyengine:main with commit a5279d3 Jan 10, 2025
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy M-Needs-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 X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants