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

Remove Sync bound on Component and Resource #4680

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_ecs/macros/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn derive_component(input: TokenStream) -> TokenStream {
ast.generics
.make_where_clause()
.predicates
.push(parse_quote! { Self: Send + Sync + 'static });
.push(parse_quote! { Self: Send + 'static });

let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ use std::{any::TypeId, collections::HashMap};
/// bundle, in the _exact_ order that [`Bundle::get_components`] is called.
/// - [`Bundle::from_components`] must call `func` exactly once for each [`ComponentId`] returned by
/// [`Bundle::component_ids`].
pub unsafe trait Bundle: Send + Sync + 'static {
pub unsafe trait Bundle: Send + 'static {
/// Gets this [`Bundle`]'s component ids, in the order of this bundle's [`Component`]s
fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec<ComponentId>;

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::{
/// A component is data associated with an [`Entity`](crate::entity::Entity). Each entity can have
/// multiple different types of components, but only one of them per type.
///
/// Any type that is `Send + Sync + 'static` can implement `Component` using `#[derive(Component)]`.
/// Any type that is `Send + 'static` can implement `Component` using `#[derive(Component)]`.
///
/// In order to use foreign types as components, wrap them using a newtype pattern.
/// ```
Expand All @@ -31,7 +31,7 @@ use std::{
/// as one of the arguments.
///
/// Components can be grouped together into a [`Bundle`](crate::bundle::Bundle).
pub trait Component: Send + Sync + 'static {
pub trait Component: Send + 'static {
type Storage: ComponentStorage;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ fn map_instance_event<T>(event_instance: &EventInstance<T>) -> &T {

/// Reads events of type `T` in order and tracks which events have already been read.
#[derive(SystemParam)]
pub struct EventReader<'w, 's, T: Resource> {
pub struct EventReader<'w, 's, T: Resource + Sync> {
last_event_count: Local<'s, (usize, PhantomData<T>)>,
events: Res<'w, Events<T>>,
}
Expand Down Expand Up @@ -263,7 +263,7 @@ fn internal_event_reader<'a, T>(
.inspect(move |(_, id)| *last_event_count = (id.id + 1).max(*last_event_count))
}

impl<'w, 's, T: Resource> EventReader<'w, 's, T> {
impl<'w, 's, T: Resource + Sync> EventReader<'w, 's, T> {
/// Iterates over the events this [`EventReader`] has not seen yet. This updates the
/// [`EventReader`]'s event counter, which means subsequent event reads will not include events
/// that happened before now.
Expand Down
16 changes: 8 additions & 8 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ impl<'w> Fetch<'w> for EntityFetch<'w> {
}
}

impl<T: Component> WorldQuery for &T {
impl<T: Component + Sync> WorldQuery for &T {
type State = ReadState<T>;

fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
Expand All @@ -553,12 +553,12 @@ impl<T: Component> WorldQuery for &T {
#[doc(hidden)]
pub struct ReadState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
Copy link
Member

Choose a reason for hiding this comment

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

This aligns with my mental model of how this should be modelled, but this is subtle enough that it deserves a comment.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't really think it's needed. This is very much the standard way to express 'this represents something which can produce a T'.

It's no more strange than the initial use of PhantomData imo - maybe we should have a comment that we need to be able to 'reconstruct' the original T, so store it here. But we don't need to comment specifically on it being fn()->T

}

// SAFETY: component access and archetype component access are properly updated to reflect that T is
// read
unsafe impl<T: Component> FetchState for ReadState<T> {
unsafe impl<T: Component + Sync> FetchState for ReadState<T> {
fn init(world: &mut World) -> Self {
let component_id = world.init_component::<T>();
ReadState {
Expand Down Expand Up @@ -620,15 +620,15 @@ impl<T> Clone for ReadFetch<'_, T> {
}

/// SAFETY: access is read only
unsafe impl<'w, T: Component> ReadOnlyFetch for ReadFetch<'w, T> {}
unsafe impl<'w, T: Component + Sync> ReadOnlyFetch for ReadFetch<'w, T> {}

impl<'w, T: Component> WorldQueryGats<'w> for &T {
impl<'w, T: Component + Sync> WorldQueryGats<'w> for &T {
type Fetch = ReadFetch<'w, T>;
type ReadOnlyFetch = ReadFetch<'w, T>;
type _State = ReadState<T>;
}

impl<'w, T: Component> Fetch<'w> for ReadFetch<'w, T> {
impl<'w, T: Component + Sync> Fetch<'w> for ReadFetch<'w, T> {
type Item = &'w T;
type State = ReadState<T>;

Expand Down Expand Up @@ -790,7 +790,7 @@ impl<T> Clone for ReadOnlyWriteFetch<'_, T> {
#[doc(hidden)]
pub struct WriteState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

// SAFETY: component access and archetype component access are properly updated to reflect that T is
Expand Down Expand Up @@ -1256,7 +1256,7 @@ impl<T: Component> WorldQuery for ChangeTrackers<T> {
#[doc(hidden)]
pub struct ChangeTrackersState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

// SAFETY: component access and archetype component access are properly updated to reflect that T is
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pub struct WithFetch<T> {
#[doc(hidden)]
pub struct WithState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

// SAFETY: no component access or archetype component access
Expand Down Expand Up @@ -207,7 +207,7 @@ pub struct WithoutFetch<T> {
#[doc(hidden)]
pub struct WithoutState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

// SAFETY: no component access or archetype component access
Expand Down Expand Up @@ -497,7 +497,7 @@ macro_rules! impl_tick_filter {
$(#[$state_meta])*
pub struct $state_name<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

impl<T: Component> WorldQuery for $name<T> {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use std::fmt::Debug;
use super::IntoSystemDescriptor;

/// A type that can run as a step of a [`Schedule`](super::Schedule).
pub trait Stage: Downcast + Send + Sync {
pub trait Stage: Downcast + Send {
/// Runs the stage; this happens once per update.
/// Implementors must initialize all of their state and systems before running the first time.
fn run(&mut self, world: &mut World);
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::marker::PhantomData;
use super::Resource;

/// A [`World`] mutation.
pub trait Command: Send + Sync + 'static {
pub trait Command: Send + 'static {
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll also want to do the same transformation for Send at some point.

Note that doing that requires we keep proper main thread management - it requires a lot of care.

Copy link

Choose a reason for hiding this comment

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

Is the Send + Sync necessary when the target is wasm?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eventually we'll want Web Worker-based multithreading, so it will still be required.

fn write(self, world: &mut World);
}

Expand Down Expand Up @@ -261,7 +261,7 @@ impl<'w, 's> Commands<'w, 's> {
/// worked out to share an ID space (which doesn't happen by default).
pub fn insert_or_spawn_batch<I, B>(&mut self, bundles_iter: I)
where
I: IntoIterator + Send + Sync + 'static,
I: IntoIterator + Send + 'static,
I::IntoIter: Iterator<Item = (Entity, B)>,
B: Bundle,
{
Expand Down Expand Up @@ -640,7 +640,7 @@ where

pub struct InsertOrSpawnBatch<I, B>
where
I: IntoIterator + Send + Sync + 'static,
I: IntoIterator + Send + 'static,
B: Bundle,
I::IntoIter: Iterator<Item = (Entity, B)>,
{
Expand All @@ -649,7 +649,7 @@ where

impl<I, B> Command for InsertOrSpawnBatch<I, B>
where
I: IntoIterator + Send + Sync + 'static,
I: IntoIterator + Send + 'static,
B: Bundle,
I::IntoIter: Iterator<Item = (Entity, B)>,
{
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/exclusive_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
};
use std::borrow::Cow;

pub trait ExclusiveSystem: Send + Sync + 'static {
pub trait ExclusiveSystem: Send + 'static {
fn name(&self) -> Cow<'static, str>;

fn run(&mut self, world: &mut World);
Expand Down
5 changes: 4 additions & 1 deletion crates/bevy_ecs/src/system/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,10 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> {
/// # bevy_ecs::system::assert_is_system(print_selected_character_name_system);
/// ```
#[inline]
pub fn get_component<T: Component>(&self, entity: Entity) -> Result<&T, QueryComponentError> {
pub fn get_component<T: Component + Sync>(
&self,
entity: Entity,
) -> Result<&T, QueryComponentError> {
let world = self.world;
let entity_ref = world
.get_entity(entity)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/system/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::borrow::Cow;
/// Systems are executed in parallel, in opportunistic order; data access is managed automatically.
/// It's possible to specify explicit execution order between specific systems,
/// see [`SystemDescriptor`](crate::schedule::SystemDescriptor).
pub trait System: Send + Sync + 'static {
pub trait System: Send + 'static {
/// The system's input. See [`In`](crate::system::In) for
/// [`FunctionSystem`](crate::system::FunctionSystem)s.
type In;
Expand Down
38 changes: 19 additions & 19 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub type SystemParamItem<'w, 's, P> = <<P as SystemParam>::Fetch as SystemParamF
/// [`World`] access used by the [`SystemParamState`] (and associated [`SystemParamFetch`]).
/// Additionally, it is the implementor's responsibility to ensure there is no
/// conflicting access across all [`SystemParam`]'s.
pub unsafe trait SystemParamState: Send + Sync + 'static {
pub unsafe trait SystemParamState: Send + 'static {
fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self;
#[inline]
fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) {}
Expand Down Expand Up @@ -178,9 +178,9 @@ pub struct ParamSetState<T: for<'w, 's> SystemParamFetch<'w, 's>>(T);

impl_param_set!();

pub trait Resource: Send + Sync + 'static {}
pub trait Resource: Send + 'static {}

impl<T> Resource for T where T: Send + Sync + 'static {}
impl<T> Resource for T where T: Send + 'static {}

/// Shared borrow of a resource.
///
Expand All @@ -193,17 +193,17 @@ impl<T> Resource for T where T: Send + Sync + 'static {}
/// Panics when used as a [`SystemParameter`](SystemParam) if the resource does not exist.
///
/// Use `Option<Res<T>>` instead if the resource might not always exist.
pub struct Res<'w, T: Resource> {
pub struct Res<'w, T: Resource + Sync> {
value: &'w T,
ticks: &'w ComponentTicks,
last_change_tick: u32,
change_tick: u32,
}

// SAFE: Res only reads a single World resource
unsafe impl<T: Resource> ReadOnlySystemParamFetch for ResState<T> {}
unsafe impl<T: Resource + Sync> ReadOnlySystemParamFetch for ResState<T> {}

impl<'w, T: Resource> Debug for Res<'w, T>
impl<'w, T: Resource + Sync> Debug for Res<'w, T>
where
T: Debug,
{
Expand All @@ -212,7 +212,7 @@ where
}
}

impl<'w, T: Resource> Res<'w, T> {
impl<'w, T: Resource + Sync> Res<'w, T> {
/// Returns true if (and only if) this resource been added since the last execution of this
/// system.
pub fn is_added(&self) -> bool {
Expand All @@ -231,22 +231,22 @@ impl<'w, T: Resource> Res<'w, T> {
}
}

impl<'w, T: Resource> Deref for Res<'w, T> {
impl<'w, T: Resource + Sync> Deref for Res<'w, T> {
type Target = T;

fn deref(&self) -> &Self::Target {
self.value
}
}

impl<'w, T: Resource> AsRef<T> for Res<'w, T> {
impl<'w, T: Resource + Sync> AsRef<T> for Res<'w, T> {
#[inline]
fn as_ref(&self) -> &T {
self.deref()
}
}

impl<'w, T: Resource> From<ResMut<'w, T>> for Res<'w, T> {
impl<'w, T: Resource + Sync> From<ResMut<'w, T>> for Res<'w, T> {
fn from(res: ResMut<'w, T>) -> Self {
Self {
value: res.value,
Expand All @@ -264,13 +264,13 @@ pub struct ResState<T> {
marker: PhantomData<T>,
}

impl<'a, T: Resource> SystemParam for Res<'a, T> {
impl<'a, T: Resource + Sync> SystemParam for Res<'a, T> {
type Fetch = ResState<T>;
}

// SAFE: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res
// conflicts with any prior access, a panic will occur.
unsafe impl<T: Resource> SystemParamState for ResState<T> {
unsafe impl<T: Resource + Sync> SystemParamState for ResState<T> {
fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self {
let component_id = world.initialize_resource::<T>();
let combined_access = system_meta.component_access_set.combined_access_mut();
Expand All @@ -296,7 +296,7 @@ unsafe impl<T: Resource> SystemParamState for ResState<T> {
}
}

impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState<T> {
impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for ResState<T> {
type Item = Res<'w, T>;

#[inline]
Expand Down Expand Up @@ -329,20 +329,20 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for ResState<T> {
#[doc(hidden)]
pub struct OptionResState<T>(ResState<T>);

impl<'a, T: Resource> SystemParam for Option<Res<'a, T>> {
impl<'a, T: Resource + Sync> SystemParam for Option<Res<'a, T>> {
type Fetch = OptionResState<T>;
}

// SAFE: Only reads a single World resource
unsafe impl<T: Resource> ReadOnlySystemParamFetch for OptionResState<T> {}
unsafe impl<T: Resource + Sync> ReadOnlySystemParamFetch for OptionResState<T> {}

unsafe impl<T: Resource> SystemParamState for OptionResState<T> {
unsafe impl<T: Resource + Sync> SystemParamState for OptionResState<T> {
fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self {
Self(ResState::init(world, system_meta))
}
}

impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState<T> {
impl<'w, 's, T: Resource + Sync> SystemParamFetch<'w, 's> for OptionResState<T> {
type Item = Option<Res<'w, T>>;

#[inline]
Expand All @@ -367,7 +367,7 @@ impl<'w, 's, T: Resource> SystemParamFetch<'w, 's> for OptionResState<T> {
#[doc(hidden)]
pub struct ResMutState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

impl<'a, T: Resource> SystemParam for ResMut<'a, T> {
Expand Down Expand Up @@ -711,7 +711,7 @@ unsafe impl<T: Component> ReadOnlySystemParamFetch for RemovedComponentsState<T>
#[doc(hidden)]
pub struct RemovedComponentsState<T> {
component_id: ComponentId,
marker: PhantomData<T>,
marker: PhantomData<fn() -> T>,
}

impl<'a, T: Component> SystemParam for RemovedComponents<'a, T> {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'w> EntityRef<'w> {
}

#[inline]
pub fn get<T: Component>(&self) -> Option<&'w T> {
pub fn get<T: Component + Sync>(&self) -> Option<&'w T> {
// SAFE: entity location is valid and returned component is of type T
unsafe {
get_component_with_type(self.world, TypeId::of::<T>(), self.entity, self.location)
Expand Down Expand Up @@ -166,7 +166,7 @@ impl<'w> EntityMut<'w> {
}

#[inline]
pub fn get<T: Component>(&self) -> Option<&'_ T> {
pub fn get<T: Component + Sync>(&self) -> Option<&'_ T> {
// SAFE: lifetimes enforce correct usage of returned borrow
unsafe {
get_component_with_type(self.world, TypeId::of::<T>(), self.entity, self.location)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl World {
/// assert_eq!(position.x, 0.0);
/// ```
#[inline]
pub fn get<T: Component>(&self, entity: Entity) -> Option<&T> {
pub fn get<T: Component + Sync>(&self, entity: Entity) -> Option<&T> {
self.get_entity(entity)?.get()
}

Expand Down
Loading