From 8cf68d87520c8c356a42252ef3c9c3b47d4ec144 Mon Sep 17 00:00:00 2001 From: mvlabat Date: Sun, 18 Oct 2020 19:54:54 +0300 Subject: [PATCH 1/3] Implement a minimal working example of SystemResources --- codegen/src/derive.rs | 306 ++++++++++++++++++ codegen/src/lib.rs | 22 ++ src/internals/cons.rs | 23 ++ src/internals/query/view/mod.rs | 1 + .../query/view/system_resources_view.rs | 15 + src/internals/systems/resources.rs | 13 +- src/internals/systems/system.rs | 35 +- src/lib.rs | 9 +- src/system_data.rs | 35 ++ tests/system_resources.rs | 32 ++ 10 files changed, 487 insertions(+), 4 deletions(-) create mode 100644 codegen/src/derive.rs create mode 100644 src/internals/query/view/system_resources_view.rs create mode 100644 src/system_data.rs create mode 100644 tests/system_resources.rs diff --git a/codegen/src/derive.rs b/codegen/src/derive.rs new file mode 100644 index 00000000..023d9986 --- /dev/null +++ b/codegen/src/derive.rs @@ -0,0 +1,306 @@ +use syn::{ + punctuated::Punctuated, token::Comma, Data, DataStruct, DeriveInput, Field, Fields, + FieldsNamed, FieldsUnnamed, GenericArgument, Ident, Lifetime, PathArguments, Type, TypePath, + WhereClause, WherePredicate, +}; + +/// Used to `#[derive]` the trait `IntoQuery`. +/// +/// You need to have the following items included in the current scope: +/// +/// * `IntoQuery` +/// * `Query` + +pub fn impl_system_resources(ast: &DeriveInput) -> proc_macro2::TokenStream { + let name = &ast.ident; + let mut generics = ast.generics.clone(); + + let (_fetch_return, tys) = gen_from_body(&ast.data, name); + let tys = &tys; + // Assumes that the first lifetime is the fetch lt + let def_fetch_lt = ast + .generics + .lifetimes() + .next() + .expect("There has to be at least one lifetime"); + let impl_fetch_lt = &def_fetch_lt.lifetime; + + { + let where_clause = generics.make_where_clause(); + constrain_system_data_types(where_clause, impl_fetch_lt, tys); + } + // Reads and writes are taken from the same types, + // but need to be cloned before. + + let (_impl_generics, ty_generics, _where_clause) = generics.split_for_impl(); + + let system_resources_info = system_resources_info_from_ast(&ast.data); + let (write_resources, read_resources) = + partition_resources(&system_resources_info.resource_types); + + let mut resource_types_iter = system_resources_info + .resource_types + .iter() + .cloned() + .rev() + .map(|resource_info| (resource_info.resource_type, resource_info.mutable)); + + let (mut cons_concat_arg_type, mut cons_concat_expr) = resource_types_iter.next().map_or( + (quote! { () }, quote! { () }), + |(resource_type, mutable)| { + if mutable { + ( + quote! { (Write< #resource_type >, ()) }, + quote! { (Write< #resource_type >::default(), ()) }, + ) + } else { + ( + quote! { (Read< #resource_type >, ()) }, + quote! { (Read::< #resource_type >::default(), ()) }, + ) + } + }, + ); + + for (resource_type, mutable) in resource_types_iter { + cons_concat_arg_type = if mutable { + quote! { (Write< #resource_type >, #cons_concat_arg_type) } + } else { + quote! { (Read< #resource_type >, #cons_concat_arg_type) } + }; + + cons_concat_expr = if mutable { + quote! { (Write::< #resource_type >::default(), #cons_concat_expr) } + } else { + quote! { (Read::< #resource_type >::default(), #cons_concat_expr) } + }; + } + + let field_names = system_resources_info.field_names.clone(); + let resource_fetches = system_resources_info + .resource_types + .iter() + .cloned() + .map(|resource_info| (resource_info.resource_type, resource_info.mutable)) + .zip(field_names.iter()) + .map(|((resource_type, mutable), field_name)| { + if mutable { + quote! { + #field_name: { + let type_id = &::legion::systems::ResourceTypeId::of::< #resource_type >(); + resources.get(&type_id).unwrap().get_mut::< #resource_type >().unwrap() + }, + } + } else { + quote! { + #field_name: { + let type_id = &::legion::systems::ResourceTypeId::of::< #resource_type >(); + resources.get(&type_id).unwrap().get::< #resource_type >().unwrap() + }, + } + } + }) + .collect::>(); + + quote! { + impl #ty_generics SystemResources<#impl_fetch_lt> for #name #ty_generics { + type ConsConcatArg = #cons_concat_arg_type; + + fn resources() -> Self::ConsConcatArg { + #cons_concat_expr + } + + fn read_resource_type_ids() -> Vec<::legion::systems::ResourceTypeId> { + let mut resource_type_ids = Vec::new(); + #( + resource_type_ids.push(::legion::systems::ResourceTypeId::of::< #read_resources >()); + )* + resource_type_ids + } + + fn write_resource_type_ids() -> Vec<::legion::systems::ResourceTypeId> { + let mut resource_type_ids = Vec::new(); + #( + resource_type_ids.push(::legion::systems::ResourceTypeId::of::< #write_resources >()); + )* + resource_type_ids + } + + unsafe fn fetch_unchecked(resources: & #impl_fetch_lt ::legion::systems::UnsafeResources) -> Self { + // TODO: support tuple structs + Self { #( + #resource_fetches + )* } + } + } + } +} + +fn collect_field_types(fields: &Punctuated) -> Vec { + fields.iter().map(|x| x.ty.clone()).collect() +} + +fn gen_identifiers(fields: &Punctuated) -> Vec { + fields.iter().map(|x| x.ident.clone().unwrap()).collect() +} + +/// Adds a `IntoQuery<'lt>` bound on each of the system data types. +fn constrain_system_data_types(clause: &mut WhereClause, fetch_lt: &Lifetime, tys: &[Type]) { + for ty in tys.iter() { + let where_predicate: WherePredicate = parse_quote!(#ty : IntoQuery< #fetch_lt >); + clause.predicates.push(where_predicate); + } +} + +#[derive(Clone)] +enum DataType { + Struct, + Tuple, +} + +#[derive(Clone)] +struct ResourceInfo { + mutable: bool, + resource_type: Type, +} + +#[derive(Clone)] +struct SystemResourcesInfo { + data_type: DataType, + field_names: Vec, + struct_types: Vec, + // Component names (stripped out references from struct_fields). + resource_types: Vec, +} + +fn partition_resources(resource_types: &[ResourceInfo]) -> (Vec, Vec) { + let (write_resources, read_resources) = resource_types + .iter() + .cloned() + .partition::, _>(|resource_type| resource_type.mutable); + + fn map_type(resource_info: ResourceInfo) -> Type { + resource_info.resource_type + } + + let write_resources = write_resources.into_iter().map(map_type).collect(); + let read_resources = read_resources.into_iter().map(map_type).collect(); + + (write_resources, read_resources) +} + +fn system_resources_info_from_ast(ast: &Data) -> SystemResourcesInfo { + let (data_type, fields) = match *ast { + Data::Struct(DataStruct { + fields: Fields::Named(FieldsNamed { named: ref x, .. }), + .. + }) => (DataType::Struct, x), + Data::Struct(DataStruct { + fields: Fields::Unnamed(FieldsUnnamed { unnamed: ref x, .. }), + .. + }) => (DataType::Tuple, x), + _ => panic!("Enums are not supported"), + }; + + let field_names = if let DataType::Struct = data_type { + fields + .iter() + .map(|field| { + field + .ident + .clone() + .expect("Expected a name for a named struct field") + }) + .collect() + } else { + Vec::new() + }; + + let mut struct_types = Vec::with_capacity(fields.len()); + let mut resource_types = Vec::with_capacity(fields.len()); + + for field in fields { + match &field.ty { + Type::Reference(_) => { + panic!("Only Fetch or FetchMut are supported"); + } + Type::Path(type_path) => { + let resources_type = try_fetch_type(type_path.clone()); + struct_types.push(Type::Path(type_path.clone())); + resource_types.push(resources_type); + } + _ => panic!("Only reference or Option types are supported"), + } + } + + SystemResourcesInfo { + data_type, + field_names, + struct_types, + resource_types, + } +} + +fn try_fetch_type(type_path: TypePath) -> ResourceInfo { + let last_path_segment = type_path.path.segments.last().unwrap(); + // TODO: support for custom paths, if it's possible to pass with macro attributes + let mutable = match last_path_segment.ident.to_string().as_ref() { + "Fetch" => false, + "FetchMut" => true, + _ => panic!("meh"), + }; + + let resource_type = + if let PathArguments::AngleBracketed(generic_arguments) = &last_path_segment.arguments { + if let GenericArgument::Type(type_arg) = generic_arguments.args.last().unwrap() { + type_arg.clone() + } else { + panic!("Expected a type as the last generic argument for Fetch or FetchMut"); + } + } else { + panic!("Expected generic arguments for Fetch or FetchMut"); + }; + + ResourceInfo { + mutable, + resource_type, + } +} + +fn gen_from_body(ast: &Data, name: &Ident) -> (proc_macro2::TokenStream, Vec) { + let (body, fields) = match *ast { + Data::Struct(DataStruct { + fields: Fields::Named(FieldsNamed { named: ref x, .. }), + .. + }) => (DataType::Struct, x), + Data::Struct(DataStruct { + fields: Fields::Unnamed(FieldsUnnamed { unnamed: ref x, .. }), + .. + }) => (DataType::Tuple, x), + _ => panic!("Enums are not supported"), + }; + + let tys = collect_field_types(fields); + + let fetch_return = match body { + DataType::Struct => { + let identifiers = gen_identifiers(fields); + + quote! { + #name { + #( #identifiers: IntoQuery::fetch(world) ),* + } + } + } + DataType::Tuple => { + let count = tys.len(); + let fetch = vec![quote! { IntoQuery::fetch(world) }; count]; + + quote! { + #name ( #( #fetch ),* ) + } + } + }; + + (fetch_return, tys) +} diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index c2fefc39..235096e0 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,4 +1,14 @@ +#![recursion_limit = "256"] + extern crate proc_macro; +extern crate proc_macro2; +#[macro_use] +extern crate quote; +#[macro_use] +extern crate syn; + +mod derive; + use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote, quote_spanned}; @@ -7,6 +17,8 @@ use syn::{ ItemFn, Lit, Meta, PathArguments, Signature, Type, Visibility, }; +// pub mod derive; + /// Wraps a function in a system, and generates a new function which constructs that system. /// /// There are three types of systems: `simple` (default), `for_each` and `par_for_each`. @@ -169,6 +181,16 @@ pub fn system(attr: TokenStream, item: TokenStream) -> TokenStream { TokenStream::from(output) } +#[proc_macro_derive(SystemResources)] +// TODO: accept legion crate name parameter +pub fn system_resources(input: TokenStream) -> TokenStream { + let ast = syn::parse(input).unwrap(); + + let gen = derive::impl_system_resources(&ast); + + gen.into() +} + #[derive(thiserror::Error, Debug)] enum Error { #[error("system types must be one of `simple`, `for_each` or `par_for_each`")] diff --git a/src/internals/cons.rs b/src/internals/cons.rs index 72d56761..6344555f 100644 --- a/src/internals/cons.rs +++ b/src/internals/cons.rs @@ -71,6 +71,29 @@ impl> ConsAppend for (A, B) { } } +/// Concat a cons list with another one. +pub trait ConsConcat { + /// Result of concat + type Output; + /// Concat to runtime cons value + fn concat(self, t: T) -> Self::Output; +} + +impl ConsConcat for () { + type Output = T; + fn concat(self, t: T) -> Self::Output { + t + } +} + +impl> ConsConcat for (A, B) { + type Output = (A, >::Output); + fn concat(self, t: T) -> Self::Output { + let (a, b) = self; + (a, b.concat(t)) + } +} + /// transform cons list into a flat tuple pub trait ConsFlatten { /// Flattened tuple diff --git a/src/internals/query/view/mod.rs b/src/internals/query/view/mod.rs index c4f5e037..7185834f 100644 --- a/src/internals/query/view/mod.rs +++ b/src/internals/query/view/mod.rs @@ -22,6 +22,7 @@ use std::marker::PhantomData; pub mod entity; pub mod read; +pub mod system_resources_view; pub mod try_read; pub mod try_write; pub mod write; diff --git a/src/internals/query/view/system_resources_view.rs b/src/internals/query/view/system_resources_view.rs new file mode 100644 index 00000000..f1a4b041 --- /dev/null +++ b/src/internals/query/view/system_resources_view.rs @@ -0,0 +1,15 @@ +#![doc(hidden)] + +use super::ReadOnly; + +use derivative::Derivative; +use std::marker::PhantomData; + +/// TODO: better name? +#[derive(Derivative, Debug, Copy, Clone)] +#[derivative(Default(bound = ""))] +pub struct SystemResourcesView(PhantomData<*const T>); + +unsafe impl Send for SystemResourcesView {} +unsafe impl Sync for SystemResourcesView {} +unsafe impl ReadOnly for SystemResourcesView {} diff --git a/src/internals/systems/resources.rs b/src/internals/systems/resources.rs index d91080d8..ae0d44de 100644 --- a/src/internals/systems/resources.rs +++ b/src/internals/systems/resources.rs @@ -18,6 +18,8 @@ use std::{ ops::{Deref, DerefMut}, sync::atomic::AtomicIsize, }; +use crate::internals::query::view::system_resources_view::SystemResourcesView; +use crate::system_data::SystemResources; /// Unique ID for a resource. #[derive(Copy, Clone, Debug, Eq, PartialOrd, Ord)] @@ -143,6 +145,14 @@ impl<'a, T: Resource> ResourceSet<'a> for Write { } } +impl<'a, S: 'a + SystemResources<'a>> ResourceSet<'a> for SystemResourcesView { + type Result = S; + + unsafe fn fetch_unchecked(resources: &'a UnsafeResources) -> Self::Result { + S::fetch_unchecked(resources) + } +} + macro_rules! resource_tuple { ($head_ty:ident) => { impl_resource_tuple!($head_ty); @@ -358,7 +368,8 @@ impl UnsafeResources { self.map.remove(type_id).map(|cell| cell.into_inner()) } - fn get(&self, type_id: &ResourceTypeId) -> Option<&ResourceCell> { + /// TODO + pub fn get(&self, type_id: &ResourceTypeId) -> Option<&ResourceCell> { self.map.get(type_id) } diff --git a/src/internals/systems/system.rs b/src/internals/systems/system.rs index c28fe187..bd17c9d8 100644 --- a/src/internals/systems/system.rs +++ b/src/internals/systems/system.rs @@ -5,6 +5,8 @@ use super::{ resources::{Resource, ResourceSet, ResourceTypeId, UnsafeResources}, schedule::Runnable, }; + + use crate::internals::{ cons::{ConsAppend, ConsFlatten}, permissions::Permissions, @@ -20,8 +22,10 @@ use crate::internals::{ subworld::{ArchetypeAccess, ComponentAccess, SubWorld}, world::{World, WorldId}, }; +use crate::system_data::{SystemResources}; use bit_set::BitSet; use std::{any::TypeId, borrow::Cow, collections::HashMap, marker::PhantomData}; +use crate::internals::query::view::system_resources_view::SystemResourcesView; /// Provides an abstraction across tuples of queries for system closures. pub trait QuerySet: Send + Sync { @@ -131,7 +135,7 @@ pub struct System { impl Runnable for System where - R: for<'a> ResourceSet<'a>, + R: ResourceSet<'static>, Q: QuerySet, F: SystemFn, { @@ -408,6 +412,33 @@ where self } + /// A proxy function to `SystemResources::register`. + /// TODO: better docs + pub fn register_system_resources( + mut self, + ) -> SystemBuilder>>::Output> + where + S: 'static + SystemResources<'static>, + R: ConsAppend>, + >>::Output: ConsFlatten, + { + for resource_type_id in S::read_resource_type_ids() { + self.resource_access.push_read(resource_type_id); + } + for resource_type_id in S::write_resource_type_ids() { + self.resource_access.push(resource_type_id); + } + + SystemBuilder { + name: self.name, + queries: self.queries, + resources: ConsAppend::append(self.resources, SystemResourcesView::default()), + resource_access: self.resource_access, + component_access: self.component_access, + access_all_archetypes: self.access_all_archetypes, + } + } + /// Builds a system which is not `Schedulable`, as it is not thread safe (!Send and !Sync), /// but still implements all the calling infrastructure of the `Runnable` trait. This provides /// a way for legion consumers to leverage the `System` construction and type-handling of @@ -417,7 +448,7 @@ where run_fn: F, ) -> System<::Output, ::Output, F> where - ::Output: for<'a> ResourceSet<'a>, + ::Output: ResourceSet<'static>, ::Output: QuerySet, F: FnMut( &mut CommandBuffer, diff --git a/src/lib.rs b/src/lib.rs index d1c7cdcc..ce801b7c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -190,10 +190,17 @@ pub mod query; pub mod storage; pub mod systems; pub mod world; +pub mod system_data; #[cfg(feature = "serialize")] pub mod serialize; +// useful for users who want to construct their wrappers around SystemBuilder +/// TODO: this is probably a bit weird way to re-export these +pub mod cons { + pub use super::internals::cons::{ConsFlatten, ConsAppend, ConsPrepend}; +} + // re-export most common types into the root pub use crate::{ query::{ @@ -206,7 +213,7 @@ pub use crate::{ }; #[cfg(feature = "codegen")] -pub use legion_codegen::system; +pub use legion_codegen::{system, SystemResources}; #[cfg(feature = "serialize")] pub use crate::serialize::Registry; diff --git a/src/system_data.rs b/src/system_data.rs new file mode 100644 index 00000000..72db1617 --- /dev/null +++ b/src/system_data.rs @@ -0,0 +1,35 @@ +//! TODO + + +use crate::internals::systems::resources::{ResourceTypeId, UnsafeResources}; + + + +/// TODO +pub trait SystemResources<'a> { + /// TODO + type ConsConcatArg; + + /// TODO + fn resources() -> Self::ConsConcatArg; + + /// TODO + unsafe fn fetch_unchecked(resources: &'a UnsafeResources) -> Self; + + /// TODO + fn read_resource_type_ids() -> Vec; + + /// TODO + fn write_resource_type_ids() -> Vec; +} + + + +/// TODO +pub trait SystemView { + // /// TODO + // fn register(system_builder: SystemBuilder) -> SystemBuilder + // where + // Q: 'static + Send + ConsFlatten, + // R: 'static + ConsFlatten; +} diff --git a/tests/system_resources.rs b/tests/system_resources.rs new file mode 100644 index 00000000..e6b47732 --- /dev/null +++ b/tests/system_resources.rs @@ -0,0 +1,32 @@ +use crate::system_data::SystemResources; +use legion::systems::Fetch; +use legion::*; + +#[test] +#[cfg(feature = "codegen")] +fn basic_system() { + pub struct TestA(usize); + pub struct TestB(usize); + + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_a: Fetch<'a, TestA>, + test_b: Fetch<'a, TestB>, + } + + let test = SystemBuilder::new("test") + .register_system_resources::>() + .build(|_, _, test_resources, _| { + assert_eq!(test_resources.test_a.0, 1); + assert_eq!(test_resources.test_b.0, 2); + }); + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(test).build(); + + schedule.execute(&mut world, &mut resources); +} From a548c2e30aa87dacfde5fc2b5c9acdd2727a14ab Mon Sep 17 00:00:00 2001 From: mvlabat Date: Wed, 21 Oct 2020 00:41:24 +0300 Subject: [PATCH 2/3] Add support for #[system] macro --- codegen/src/lib.rs | 52 +++++++++- src/internals/systems/resources.rs | 4 +- src/internals/systems/system.rs | 8 +- src/lib.rs | 4 +- src/system_data.rs | 5 - src/systems.rs | 3 +- tests/failures/value_argument.stderr | 2 +- tests/system_resources.rs | 143 +++++++++++++++++++++++---- 8 files changed, 182 insertions(+), 39 deletions(-) diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index 235096e0..3040a0a5 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -207,7 +207,8 @@ enum Error { InvalidOptionArgument(Span, String), #[error( "system function parameters must be `CommandBuffer` or `SubWorld` references, \ - [optioned] component references, state references, or resource references" + [optioned] component references, state references, resource references, \ + or references to structs deriving SystemResources" )] InvalidArgument(Span), #[error("expected component type")] @@ -309,6 +310,7 @@ struct Sig { query: Vec, read_resources: Vec, write_resources: Vec, + system_resources: Vec, state_args: Vec, generics: Generics, } @@ -319,6 +321,7 @@ impl Sig { let mut query = Vec::::new(); let mut read_resources = Vec::new(); let mut write_resources = Vec::new(); + let mut system_resources = Vec::new(); let mut state_args = Vec::new(); for param in &mut item.inputs { match param { @@ -405,6 +408,17 @@ impl Sig { read_resources.push(ty.elem.as_ref().clone()); } } + Some(ArgAttr::SystemResources) => { + if mutable { + parameters.push(Parameter::SystemResourcesMut( + system_resources.len(), + )); + } else { + parameters + .push(Parameter::SystemResources(system_resources.len())); + } + system_resources.push(ty.elem.as_ref().clone()); + } Some(ArgAttr::State) => { if mutable { parameters.push(Parameter::StateMut(state_args.len())); @@ -436,6 +450,7 @@ impl Sig { query, read_resources, write_resources, + system_resources, state_args, }) } @@ -447,6 +462,10 @@ impl Sig { attributes.remove(i); return Some(ArgAttr::Resource); } + Some(ident) if ident == "system_resources" => { + attributes.remove(i); + return Some(ArgAttr::SystemResources); + } Some(ident) if ident == "state" => { attributes.remove(i); return Some(ArgAttr::State); @@ -460,6 +479,7 @@ impl Sig { enum ArgAttr { Resource, + SystemResources, State, } @@ -499,6 +519,8 @@ enum Parameter { Component(usize), Resource(usize), ResourceMut(usize), + SystemResources(usize), + SystemResourcesMut(usize), State(usize), StateMut(usize), } @@ -668,8 +690,10 @@ impl Config { // construct function arguments let has_query = !signature.query.is_empty(); - let single_resource = - (signature.read_resources.len() + signature.write_resources.len()) == 1; + let single_resource = (signature.read_resources.len() + + signature.write_resources.len() + + signature.system_resources.len()) + == 1; let mut call_params = Vec::new(); let mut fn_params = Vec::new(); let mut world = None; @@ -718,6 +742,24 @@ impl Config { let idx = Index::from(*idx + signature.read_resources.len()); call_params.push(quote!(&mut *resources.#idx)); } + Parameter::SystemResources(_) if single_resource => { + call_params.push(quote!(&*resources)) + } + Parameter::SystemResourcesMut(_) if single_resource => { + call_params.push(quote!(&mut *resources)) + } + Parameter::SystemResources(idx) => { + let idx = Index::from( + *idx + signature.read_resources.len() + signature.write_resources.len(), + ); + call_params.push(quote!(&resources.#idx)); + } + Parameter::SystemResourcesMut(idx) => { + let idx = Index::from( + *idx + signature.read_resources.len() + signature.write_resources.len(), + ); + call_params.push(quote!(&mut resources.#idx)); + } Parameter::State(idx) => { let arg_name = format_ident!("state_{}", idx); let arg_type = &signature.state_args[*idx]; @@ -775,6 +817,7 @@ impl Config { }; let read_resources = &signature.read_resources; let write_resources = &signature.write_resources; + let system_resources = &signature.system_resources; let builder = quote! { use legion::IntoQuery; #generic_parameter_names @@ -783,8 +826,9 @@ impl Config { #(.write_component::<#write_components>())* #(.read_resource::<#read_resources>())* #(.write_resource::<#write_resources>())* + #(.register_system_resources::<#system_resources>())* #query - .build(move |cmd, world, resources, query| { + .build(move |cmd, world, mut resources, query| { #body }) }; diff --git a/src/internals/systems/resources.rs b/src/internals/systems/resources.rs index ae0d44de..d56687f7 100644 --- a/src/internals/systems/resources.rs +++ b/src/internals/systems/resources.rs @@ -3,10 +3,12 @@ //! Use resources to share persistent data between systems or to provide a system with state //! external to entities. +use crate::internals::query::view::system_resources_view::SystemResourcesView; use crate::internals::{ hash::ComponentTypeIdHasher, query::view::{read::Read, write::Write, ReadOnly}, }; +use crate::system_data::SystemResources; use downcast_rs::{impl_downcast, Downcast}; use std::{ any::TypeId, @@ -18,8 +20,6 @@ use std::{ ops::{Deref, DerefMut}, sync::atomic::AtomicIsize, }; -use crate::internals::query::view::system_resources_view::SystemResourcesView; -use crate::system_data::SystemResources; /// Unique ID for a resource. #[derive(Copy, Clone, Debug, Eq, PartialOrd, Ord)] diff --git a/src/internals/systems/system.rs b/src/internals/systems/system.rs index bd17c9d8..40d8fc75 100644 --- a/src/internals/systems/system.rs +++ b/src/internals/systems/system.rs @@ -6,13 +6,14 @@ use super::{ schedule::Runnable, }; - use crate::internals::{ cons::{ConsAppend, ConsFlatten}, permissions::Permissions, query::{ filter::EntityFilter, - view::{read::Read, write::Write, IntoView, View}, + view::{ + read::Read, system_resources_view::SystemResourcesView, write::Write, IntoView, View, + }, Query, }, storage::{ @@ -22,10 +23,9 @@ use crate::internals::{ subworld::{ArchetypeAccess, ComponentAccess, SubWorld}, world::{World, WorldId}, }; -use crate::system_data::{SystemResources}; +use crate::system_data::SystemResources; use bit_set::BitSet; use std::{any::TypeId, borrow::Cow, collections::HashMap, marker::PhantomData}; -use crate::internals::query::view::system_resources_view::SystemResourcesView; /// Provides an abstraction across tuples of queries for system closures. pub trait QuerySet: Send + Sync { diff --git a/src/lib.rs b/src/lib.rs index ce801b7c..d065fd8c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -188,9 +188,9 @@ mod internals; // public API organized into logical modules pub mod query; pub mod storage; +pub mod system_data; pub mod systems; pub mod world; -pub mod system_data; #[cfg(feature = "serialize")] pub mod serialize; @@ -198,7 +198,7 @@ pub mod serialize; // useful for users who want to construct their wrappers around SystemBuilder /// TODO: this is probably a bit weird way to re-export these pub mod cons { - pub use super::internals::cons::{ConsFlatten, ConsAppend, ConsPrepend}; + pub use super::internals::cons::{ConsAppend, ConsFlatten, ConsPrepend}; } // re-export most common types into the root diff --git a/src/system_data.rs b/src/system_data.rs index 72db1617..f00774e2 100644 --- a/src/system_data.rs +++ b/src/system_data.rs @@ -1,10 +1,7 @@ //! TODO - use crate::internals::systems::resources::{ResourceTypeId, UnsafeResources}; - - /// TODO pub trait SystemResources<'a> { /// TODO @@ -23,8 +20,6 @@ pub trait SystemResources<'a> { fn write_resource_type_ids() -> Vec; } - - /// TODO pub trait SystemView { // /// TODO diff --git a/src/systems.rs b/src/systems.rs index 86c58c1b..ef8053a7 100644 --- a/src/systems.rs +++ b/src/systems.rs @@ -3,7 +3,8 @@ pub use crate::internals::systems::{ command::{CommandBuffer, WorldWritable}, resources::{ - Fetch, Resource, ResourceSet, ResourceTypeId, Resources, SyncResources, UnsafeResources, + Fetch, FetchMut, Resource, ResourceSet, ResourceTypeId, Resources, SyncResources, + UnsafeResources, }, schedule::{Builder, Executor, ParallelRunnable, Runnable, Schedule, Step}, system::{QuerySet, System, SystemAccess, SystemBuilder, SystemFn, SystemId}, diff --git a/tests/failures/value_argument.stderr b/tests/failures/value_argument.stderr index 9abdb2d2..020d1d40 100644 --- a/tests/failures/value_argument.stderr +++ b/tests/failures/value_argument.stderr @@ -1,4 +1,4 @@ -error: system function parameters must be `CommandBuffer` or `SubWorld` references, [optioned] component references, state references, or resource references +error: system function parameters must be `CommandBuffer` or `SubWorld` references, [optioned] component references, state references, resource references, or references to structs deriving SystemResources --> $DIR/value_argument.rs:4:23 | 4 | fn value_arguement(_: usize) {} diff --git a/tests/system_resources.rs b/tests/system_resources.rs index e6b47732..21b4fa70 100644 --- a/tests/system_resources.rs +++ b/tests/system_resources.rs @@ -1,32 +1,135 @@ -use crate::system_data::SystemResources; -use legion::systems::Fetch; -use legion::*; - -#[test] #[cfg(feature = "codegen")] -fn basic_system() { +mod tests { + use legion::system_data::SystemResources; + use legion::systems::{Fetch, FetchMut}; + use legion::*; + pub struct TestA(usize); pub struct TestB(usize); - #[derive(SystemResources)] - pub struct TestSystemResources<'a> { - test_a: Fetch<'a, TestA>, - test_b: Fetch<'a, TestB>, + #[test] + fn basic() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_a: Fetch<'a, TestA>, + test_b: Fetch<'a, TestB>, + } + + let test = SystemBuilder::new("test") + .register_system_resources::>() + .build(|_, _, test_resources, _| { + let test_resources: &TestSystemResources = test_resources; + assert_eq!(test_resources.test_a.0, 1); + assert_eq!(test_resources.test_b.0, 2); + }); + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(test).build(); + + schedule.execute(&mut world, &mut resources); + } + + #[test] + fn with_immutable() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_a: Fetch<'a, TestA>, + test_b: Fetch<'a, TestB>, + } + + #[system] + fn basic(#[system_resources] test_resources: &TestSystemResources<'static>) { + assert_eq!(test_resources.test_a.0, 1); + assert_eq!(test_resources.test_b.0, 2); + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + } + + #[test] + fn with_mutable() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_a: FetchMut<'a, TestA>, + test_b: Fetch<'a, TestB>, + } + + #[system] + fn basic(#[system_resources] test_resources: &mut TestSystemResources<'static>) { + test_resources.test_a.0 = test_resources.test_b.0; + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + + assert_eq!(resources.get::().unwrap().0, 2); } - let test = SystemBuilder::new("test") - .register_system_resources::>() - .build(|_, _, test_resources, _| { + #[test] + fn with_other_resources() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_b: Fetch<'a, TestB>, + } + + #[system] + fn basic( + #[system_resources] test_resources: &TestSystemResources<'static>, + #[resource] test_a: &mut TestA, + ) { + test_a.0 = test_resources.test_b.0; + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + + assert_eq!(resources.get::().unwrap().0, 2); + } + + #[test] + fn with_for_each() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a> { + test_a: Fetch<'a, TestA>, + test_b: Fetch<'a, TestB>, + } + + #[system(for_each)] + fn basic(_: &Entity, #[system_resources] test_resources: &TestSystemResources<'static>) { assert_eq!(test_resources.test_a.0, 1); assert_eq!(test_resources.test_b.0, 2); - }); + } - let mut resources = Resources::default(); - resources.insert(TestA(1)); - resources.insert(TestB(2)); + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); - let mut world = World::default(); - let mut schedule = Schedule::builder().add_system(test).build(); + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); - schedule.execute(&mut world, &mut resources); + schedule.execute(&mut world, &mut resources); + } } From f9f32c28a0fd1b30d031c82d7bab802dc277eb7f Mon Sep 17 00:00:00 2001 From: mvlabat Date: Wed, 21 Oct 2020 11:14:14 +0300 Subject: [PATCH 3/3] Add support for tuple structs and generics --- codegen/src/derive.rs | 192 +++++++++++++------------------------- codegen/src/lib.rs | 10 -- src/system_data.rs | 9 -- tests/system_resources.rs | 85 ++++++++++++++++- 4 files changed, 146 insertions(+), 150 deletions(-) diff --git a/codegen/src/derive.rs b/codegen/src/derive.rs index 023d9986..0594734e 100644 --- a/codegen/src/derive.rs +++ b/codegen/src/derive.rs @@ -1,22 +1,12 @@ +use quote::quote; use syn::{ - punctuated::Punctuated, token::Comma, Data, DataStruct, DeriveInput, Field, Fields, - FieldsNamed, FieldsUnnamed, GenericArgument, Ident, Lifetime, PathArguments, Type, TypePath, - WhereClause, WherePredicate, + Data, DataStruct, DeriveInput, Fields, FieldsNamed, FieldsUnnamed, GenericArgument, Ident, + PathArguments, Type, TypePath, }; -/// Used to `#[derive]` the trait `IntoQuery`. -/// -/// You need to have the following items included in the current scope: -/// -/// * `IntoQuery` -/// * `Query` - pub fn impl_system_resources(ast: &DeriveInput) -> proc_macro2::TokenStream { let name = &ast.ident; - let mut generics = ast.generics.clone(); - let (_fetch_return, tys) = gen_from_body(&ast.data, name); - let tys = &tys; // Assumes that the first lifetime is the fetch lt let def_fetch_lt = ast .generics @@ -25,89 +15,97 @@ pub fn impl_system_resources(ast: &DeriveInput) -> proc_macro2::TokenStream { .expect("There has to be at least one lifetime"); let impl_fetch_lt = &def_fetch_lt.lifetime; - { - let where_clause = generics.make_where_clause(); - constrain_system_data_types(where_clause, impl_fetch_lt, tys); - } - // Reads and writes are taken from the same types, - // but need to be cloned before. - - let (_impl_generics, ty_generics, _where_clause) = generics.split_for_impl(); + let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl(); let system_resources_info = system_resources_info_from_ast(&ast.data); let (write_resources, read_resources) = partition_resources(&system_resources_info.resource_types); - let mut resource_types_iter = system_resources_info + let (cons_append_arg_type, cons_append_expr) = system_resources_info .resource_types .iter() .cloned() .rev() - .map(|resource_info| (resource_info.resource_type, resource_info.mutable)); - - let (mut cons_concat_arg_type, mut cons_concat_expr) = resource_types_iter.next().map_or( - (quote! { () }, quote! { () }), - |(resource_type, mutable)| { - if mutable { - ( - quote! { (Write< #resource_type >, ()) }, - quote! { (Write< #resource_type >::default(), ()) }, - ) - } else { - ( - quote! { (Read< #resource_type >, ()) }, - quote! { (Read::< #resource_type >::default(), ()) }, - ) - } - }, - ); - - for (resource_type, mutable) in resource_types_iter { - cons_concat_arg_type = if mutable { - quote! { (Write< #resource_type >, #cons_concat_arg_type) } - } else { - quote! { (Read< #resource_type >, #cons_concat_arg_type) } - }; - - cons_concat_expr = if mutable { - quote! { (Write::< #resource_type >::default(), #cons_concat_expr) } - } else { - quote! { (Read::< #resource_type >::default(), #cons_concat_expr) } - }; - } + .map(|resource_info| (resource_info.resource_type, resource_info.mutable)) + .fold( + (quote! { () }, quote! { () }), + |(mut cons_append_arg_type, mut cons_append_expr), (resource_type, mutable)| { + cons_append_arg_type = if mutable { + quote! { (Write< #resource_type >, #cons_append_arg_type) } + } else { + quote! { (Read< #resource_type >, #cons_append_arg_type) } + }; + + cons_append_expr = if mutable { + quote! { (Write::< #resource_type >::default(), #cons_append_expr) } + } else { + quote! { (Read::< #resource_type >::default(), #cons_append_expr) } + }; + + (cons_append_arg_type, cons_append_expr) + }, + ); let field_names = system_resources_info.field_names.clone(); + let resource_fetches = system_resources_info .resource_types .iter() .cloned() .map(|resource_info| (resource_info.resource_type, resource_info.mutable)) - .zip(field_names.iter()) - .map(|((resource_type, mutable), field_name)| { + .map(|(resource_type, mutable)| { if mutable { quote! { - #field_name: { + { let type_id = &::legion::systems::ResourceTypeId::of::< #resource_type >(); resources.get(&type_id).unwrap().get_mut::< #resource_type >().unwrap() - }, + } } } else { quote! { - #field_name: { + { let type_id = &::legion::systems::ResourceTypeId::of::< #resource_type >(); resources.get(&type_id).unwrap().get::< #resource_type >().unwrap() - }, + } } } }) .collect::>(); + let return_statement = match system_resources_info.data_type { + DataType::Struct => { + let named_args = resource_fetches + .iter() + .zip(field_names.iter()) + .map(|(resource_fetch, field_name)| { + quote! { #field_name: #resource_fetch } + }) + .collect::>(); + quote! { + Self { + #( + #named_args, + )* + } + } + } + DataType::Tuple => { + quote! { + Self( + #( + #resource_fetches, + )* + ) + } + } + }; + quote! { - impl #ty_generics SystemResources<#impl_fetch_lt> for #name #ty_generics { - type ConsConcatArg = #cons_concat_arg_type; + impl #impl_generics SystemResources<#impl_fetch_lt> for #name #ty_generics #where_clause { + type ConsConcatArg = #cons_append_arg_type; fn resources() -> Self::ConsConcatArg { - #cons_concat_expr + #cons_append_expr } fn read_resource_type_ids() -> Vec<::legion::systems::ResourceTypeId> { @@ -127,31 +125,12 @@ pub fn impl_system_resources(ast: &DeriveInput) -> proc_macro2::TokenStream { } unsafe fn fetch_unchecked(resources: & #impl_fetch_lt ::legion::systems::UnsafeResources) -> Self { - // TODO: support tuple structs - Self { #( - #resource_fetches - )* } + #return_statement } } } } -fn collect_field_types(fields: &Punctuated) -> Vec { - fields.iter().map(|x| x.ty.clone()).collect() -} - -fn gen_identifiers(fields: &Punctuated) -> Vec { - fields.iter().map(|x| x.ident.clone().unwrap()).collect() -} - -/// Adds a `IntoQuery<'lt>` bound on each of the system data types. -fn constrain_system_data_types(clause: &mut WhereClause, fetch_lt: &Lifetime, tys: &[Type]) { - for ty in tys.iter() { - let where_predicate: WherePredicate = parse_quote!(#ty : IntoQuery< #fetch_lt >); - clause.predicates.push(where_predicate); - } -} - #[derive(Clone)] enum DataType { Struct, @@ -221,15 +200,12 @@ fn system_resources_info_from_ast(ast: &Data) -> SystemResourcesInfo { for field in fields { match &field.ty { - Type::Reference(_) => { - panic!("Only Fetch or FetchMut are supported"); - } Type::Path(type_path) => { let resources_type = try_fetch_type(type_path.clone()); struct_types.push(Type::Path(type_path.clone())); resource_types.push(resources_type); } - _ => panic!("Only reference or Option types are supported"), + _ => panic!("Only Fetch or FetchMut are supported"), } } @@ -243,11 +219,11 @@ fn system_resources_info_from_ast(ast: &Data) -> SystemResourcesInfo { fn try_fetch_type(type_path: TypePath) -> ResourceInfo { let last_path_segment = type_path.path.segments.last().unwrap(); - // TODO: support for custom paths, if it's possible to pass with macro attributes + // TODO: support for custom paths if it's possible to pass them with macro attributes. let mutable = match last_path_segment.ident.to_string().as_ref() { "Fetch" => false, "FetchMut" => true, - _ => panic!("meh"), + _ => panic!("Only Fetch or FetchMut are supported"), }; let resource_type = @@ -266,41 +242,3 @@ fn try_fetch_type(type_path: TypePath) -> ResourceInfo { resource_type, } } - -fn gen_from_body(ast: &Data, name: &Ident) -> (proc_macro2::TokenStream, Vec) { - let (body, fields) = match *ast { - Data::Struct(DataStruct { - fields: Fields::Named(FieldsNamed { named: ref x, .. }), - .. - }) => (DataType::Struct, x), - Data::Struct(DataStruct { - fields: Fields::Unnamed(FieldsUnnamed { unnamed: ref x, .. }), - .. - }) => (DataType::Tuple, x), - _ => panic!("Enums are not supported"), - }; - - let tys = collect_field_types(fields); - - let fetch_return = match body { - DataType::Struct => { - let identifiers = gen_identifiers(fields); - - quote! { - #name { - #( #identifiers: IntoQuery::fetch(world) ),* - } - } - } - DataType::Tuple => { - let count = tys.len(); - let fetch = vec![quote! { IntoQuery::fetch(world) }; count]; - - quote! { - #name ( #( #fetch ),* ) - } - } - }; - - (fetch_return, tys) -} diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index 3040a0a5..0a072790 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,12 +1,5 @@ #![recursion_limit = "256"] -extern crate proc_macro; -extern crate proc_macro2; -#[macro_use] -extern crate quote; -#[macro_use] -extern crate syn; - mod derive; use proc_macro::TokenStream; @@ -17,8 +10,6 @@ use syn::{ ItemFn, Lit, Meta, PathArguments, Signature, Type, Visibility, }; -// pub mod derive; - /// Wraps a function in a system, and generates a new function which constructs that system. /// /// There are three types of systems: `simple` (default), `for_each` and `par_for_each`. @@ -182,7 +173,6 @@ pub fn system(attr: TokenStream, item: TokenStream) -> TokenStream { } #[proc_macro_derive(SystemResources)] -// TODO: accept legion crate name parameter pub fn system_resources(input: TokenStream) -> TokenStream { let ast = syn::parse(input).unwrap(); diff --git a/src/system_data.rs b/src/system_data.rs index f00774e2..f71f6712 100644 --- a/src/system_data.rs +++ b/src/system_data.rs @@ -19,12 +19,3 @@ pub trait SystemResources<'a> { /// TODO fn write_resource_type_ids() -> Vec; } - -/// TODO -pub trait SystemView { - // /// TODO - // fn register(system_builder: SystemBuilder) -> SystemBuilder - // where - // Q: 'static + Send + ConsFlatten, - // R: 'static + ConsFlatten; -} diff --git a/tests/system_resources.rs b/tests/system_resources.rs index 21b4fa70..2b0598ed 100644 --- a/tests/system_resources.rs +++ b/tests/system_resources.rs @@ -67,7 +67,7 @@ mod tests { #[system] fn basic(#[system_resources] test_resources: &mut TestSystemResources<'static>) { - test_resources.test_a.0 = test_resources.test_b.0; + test_resources.test_a.0 += test_resources.test_b.0 * 2; } let mut resources = Resources::default(); @@ -79,7 +79,84 @@ mod tests { schedule.execute(&mut world, &mut resources); - assert_eq!(resources.get::().unwrap().0, 2); + assert_eq!(resources.get::().unwrap().0, 5); + } + + #[test] + fn with_tuple_struct() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a>(FetchMut<'a, TestA>, Fetch<'a, TestB>); + + #[system] + fn basic(#[system_resources] test_resources: &mut TestSystemResources<'static>) { + (test_resources.0).0 += (test_resources.1).0 * 2; + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + + assert_eq!(resources.get::().unwrap().0, 5); + } + + #[test] + fn with_generics() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a, R1, R2: 'static> + where + R1: 'static, + { + test_a: FetchMut<'a, R1>, + test_b: Fetch<'a, R2>, + } + + #[system] + fn basic( + #[system_resources] test_resources: &mut TestSystemResources<'static, TestA, TestB>, + ) { + test_resources.test_a.0 += test_resources.test_b.0 * 2; + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + + assert_eq!(resources.get::().unwrap().0, 5); + } + + #[test] + fn with_several_system_resources() { + #[derive(SystemResources)] + pub struct TestSystemResources<'a, R: 'static>(FetchMut<'a, R>); + + #[system] + fn basic( + #[system_resources] test_resources_a: &mut TestSystemResources<'static, TestA>, + #[system_resources] test_resources_b: &mut TestSystemResources<'static, TestB>, + ) { + (test_resources_a.0).0 += (test_resources_b.0).0 * 2; + } + + let mut resources = Resources::default(); + resources.insert(TestA(1)); + resources.insert(TestB(2)); + + let mut world = World::default(); + let mut schedule = Schedule::builder().add_system(basic_system()).build(); + + schedule.execute(&mut world, &mut resources); + + assert_eq!(resources.get::().unwrap().0, 5); } #[test] @@ -94,7 +171,7 @@ mod tests { #[system_resources] test_resources: &TestSystemResources<'static>, #[resource] test_a: &mut TestA, ) { - test_a.0 = test_resources.test_b.0; + test_a.0 += test_resources.test_b.0 * 2; } let mut resources = Resources::default(); @@ -106,7 +183,7 @@ mod tests { schedule.execute(&mut world, &mut resources); - assert_eq!(resources.get::().unwrap().0, 2); + assert_eq!(resources.get::().unwrap().0, 5); } #[test]