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

Local<T> declares coarse mutable access #483

Closed
farnoy opened this issue Sep 12, 2020 · 4 comments
Closed

Local<T> declares coarse mutable access #483

farnoy opened this issue Sep 12, 2020 · 4 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@farnoy
Copy link

farnoy commented Sep 12, 2020

Having two distinct systems use Local<T> (for the same T) creates a dependency between them. I believe this is due to the mutable access being declared the same way for ResMut<T> as with Local<T>.

fn access() -> TypeAccess {
let mut access = TypeAccess::default();
access.mutable.insert(TypeId::of::<T>());
access
}

fn access() -> TypeAccess {
let mut access = TypeAccess::default();
access.mutable.insert(TypeId::of::<T>());
access
}

Is there a way to make this more granular and remove dependencies between such systems?

@karroffel karroffel added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Sep 13, 2020
@fu5ha
Copy link
Contributor

fu5ha commented Sep 25, 2020

You can newtype-wrap whatever the data is that you want to store in a Local<>

The intended use case is pretty thin -- basically making a struct that is specific only that one system which you want to store state in.

@cart
Copy link
Member

cart commented Sep 25, 2020

I agree that a newtype is a quick fix for this (and it will work with the current Bevy releases). But I also think we might be able to solve this problem by just removing the T access entirely (and the borrow locks). The instance of the type is guaranteed to be uniquely accessed by the system, so theres no need to synchronize here. The only way to get an aliased reference is to directly access the Resources collection, which can only happen from a "thread local system" (which is guaranteed to have unique Resources access).

@farnoy
Copy link
Author

farnoy commented Sep 25, 2020

That is what I ended up doing, yeah.

I think TypeAccess should be extended with Option<SystemId>. The actual access is already scoped by that parameter, IMO it's confusing that hazard inspection doesn't know about it.

@memoryruins
Copy link
Contributor

Resolved by #864

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

5 participants