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

Expose SystemMeta's access field as part of public API #16625

Merged
merged 4 commits into from
Dec 5, 2024

Conversation

vil-mo
Copy link
Contributor

@vil-mo vil-mo commented Dec 3, 2024

Objective

Outside of the bevy_ecs crate it's hard to implement SystemParam trait on params that require access to the World, because init_state expects user to extend access in SystemMeta and access-related fields of SystemMeta are private.

Solution

Expose those fields as a functions

Copy link
Contributor

@BenjaminBrienen BenjaminBrienen left a comment

Choose a reason for hiding this comment

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

^_^

@BenjaminBrienen BenjaminBrienen added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 3, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Great docs, and I think we should expose this. That said, the mutable accessors need to be unsafe. If you lie about the access you can get UB by allowing aliased mutability at both the schedule and system levels.

@alice-i-cecile alice-i-cecile added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Unsafe Touches with unsafe code in some way S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 3, 2024
@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 and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Dec 4, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15.1 milestone Dec 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 5, 2024
Merged via the queue into bevyengine:main with commit 67bd2b0 Dec 5, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Unsafe Touches with unsafe code in some way 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.

4 participants