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

Hecs World currently allows for "unsafe" component access via immutable references #82

Closed
cart opened this issue Jul 29, 2020 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@cart
Copy link
Member

cart commented Jul 29, 2020

This isn't actually a memory safety problem because hecs also has runtime safety checks, but in some contexts it is useful to provide "immutable only" world references. For example, render graph nodes currently receive an &World reference, which was intended to be an immutable-only reference. Graph nodes are scheduled to run in parallel based on graph dependencies, so mutable access to components could cause undefined behavior. With legion &World was truly immutable. Which hecs, &World is "mutable with runtime safety checks".

To fix this i think we have two choices:

  • modify World to label unsafe access as literally "unsafe"
  • add an ImmutableWorld wrapper that only allows read actions
@cart cart added the C-Bug An unexpected or incorrect behavior label Jul 29, 2020
@Ratysz
Copy link
Contributor

Ratysz commented Aug 11, 2020

For what it's worth, I opted for a wrapper to solve a similar problem in yaks (I wanted to disallow doing anything that would break invariants the executor assumes within a system).

@karroffel karroffel added the A-ECS Entities, components, systems, and events label Aug 12, 2020
@DJMcNab
Copy link
Member

DJMcNab commented Jan 22, 2021

I think this is fixed now (in #504?). We ended up taking approach 1.

@cart
Copy link
Member Author

cart commented Jan 22, 2021

Yeah now that we rely on lifetimes in "safe" code to protect storage access, we can close this. Thanks for the follow up!

@cart cart closed this as completed Jan 22, 2021
hymm referenced this issue in hymm/bevy Jan 23, 2023
just have extract schedule live in render app schedules
tychedelia pushed a commit to tychedelia/bevy that referenced this issue Nov 23, 2024
fixes bevyengine#81

ignore comment markers in `"` quotes (unless the quote is already in a
comment).

also fix `*/` incorrectly stripping the marker and returning ` `, and
`/* //*` failing to recognise the second block start marker (thanks
@stefnotch for the tests).

---------

Co-authored-by: stefnotch <stefnotch@users.noreply.github.com>
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-Bug An unexpected or incorrect behavior
Projects
None yet
Development

No branches or pull requests

4 participants