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

bevy_utils: Export generate_composite_uuid utility function #10496

Merged
merged 1 commit into from
Nov 25, 2023

Conversation

MrGVSV
Copy link
Member

@MrGVSV MrGVSV commented Nov 10, 2023

Objective

The generate_composite_uuid utility function hidden in bevy_reflect::__macro_exports could be generally useful to users.

For example, I previously relied on Hash to generate a u64 to create a deterministic HandleId. In v0.12, HandleId has been replaced by AssetId which now requires a Uuid, which I could generate with this function.

Solution

Relocate generate_composite_uuid from bevy_reflect::__macro_exports to bevy_utils::uuid.

It is still re-exported under bevy_reflect::__macro_exports so there should not be any breaking changes (although, users should generally not rely on pseudo-private/hidden modules like __macro_exports).

I chose to keep it in bevy_reflect::__macro_exports so as to not clutter up our public API and to reduce the number of changes in this PR. We could have also marked the export as #[doc(hidden)], but personally I like that we have a dedicated module for this (makes it clear what is public and what isn't when just looking at the macro code).


Changelog

  • Moved generate_composite_uuid to bevy_utils::uuid and made it public
    • Note: it was technically already public, just hidden

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Utils Utility functions and types labels Nov 10, 2023
Copy link
Contributor

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

@alice-i-cecile
Copy link
Member

Fascinating, any idea why this would increase MSRV?

@MrGVSV
Copy link
Member Author

MrGVSV commented Nov 11, 2023

Fascinating, any idea why this would increase MSRV?

No clue haha.

The first CI run failed the MSRV check due to an ambiguous module name (uuid). Maybe the comment is from that failure and not the latest pass?

Any insight @mockersf?

@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 Nov 25, 2023
@alice-i-cecile
Copy link
Member

@MrGVSV can you please merge main in so you can get the new CI check?

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 25, 2023
Merged via the queue into bevyengine:main with commit 13f2749 Nov 25, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Utils Utility functions and types 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.

3 participants