Skip to content

Commit

Permalink
Deprecate .system (#3302)
Browse files Browse the repository at this point in the history
# Objective

- Using `.system()` is no longer idiomatic.

## Solution

- Give a warning when using it
  • Loading branch information
DJMcNab committed Feb 8, 2022
1 parent 2f11c9d commit 6615b7b
Show file tree
Hide file tree
Showing 17 changed files with 157 additions and 128 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl App {
/// }
///
/// App::new()
/// .add_startup_system(my_startup_system.system());
/// .add_startup_system(my_startup_system);
/// ```
pub fn add_startup_system<Params>(
&mut self,
Expand Down
63 changes: 31 additions & 32 deletions crates/bevy_asset/src/asset_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ pub fn free_unused_assets_system(asset_server: Res<AssetServer>) {
mod test {
use super::*;
use crate::{loader::LoadedAsset, update_asset_storage_system};
use bevy_app::App;
use bevy_ecs::prelude::*;
use bevy_reflect::TypeUuid;
use bevy_utils::BoxedFuture;
Expand Down Expand Up @@ -771,21 +772,13 @@ mod test {
asset_server.add_loader(FakePngLoader);
let assets = asset_server.register_asset_type::<PngAsset>();

let mut world = World::new();
world.insert_resource(assets);
world.insert_resource(asset_server);

let mut tick = {
let mut free_unused_assets_system = free_unused_assets_system.system();
free_unused_assets_system.initialize(&mut world);
let mut update_asset_storage_system = update_asset_storage_system::<PngAsset>.system();
update_asset_storage_system.initialize(&mut world);

move |world: &mut World| {
free_unused_assets_system.run((), world);
update_asset_storage_system.run((), world);
}
};
#[derive(SystemLabel, Clone, Hash, Debug, PartialEq, Eq)]
struct FreeUnusedAssets;
let mut app = App::new();
app.insert_resource(assets);
app.insert_resource(asset_server);
app.add_system(free_unused_assets_system.label(FreeUnusedAssets));
app.add_system(update_asset_storage_system::<PngAsset>.after(FreeUnusedAssets));

fn load_asset(path: AssetPath, world: &World) -> HandleUntyped {
let asset_server = world.get_resource::<AssetServer>().unwrap();
Expand Down Expand Up @@ -813,37 +806,43 @@ mod test {
// ---

let path: AssetPath = "fake.png".into();
assert_eq!(LoadState::NotLoaded, get_load_state(path.get_id(), &world));
assert_eq!(
LoadState::NotLoaded,
get_load_state(path.get_id(), &app.world)
);

// load the asset
let handle = load_asset(path.clone(), &world);
let handle = load_asset(path.clone(), &app.world);
let weak_handle = handle.clone_weak();

// asset is loading
assert_eq!(LoadState::Loading, get_load_state(&handle, &world));
assert_eq!(LoadState::Loading, get_load_state(&handle, &app.world));

tick(&mut world);
app.update();
// asset should exist and be loaded at this point
assert_eq!(LoadState::Loaded, get_load_state(&handle, &world));
assert!(get_asset(&handle, &world).is_some());
assert_eq!(LoadState::Loaded, get_load_state(&handle, &app.world));
assert!(get_asset(&handle, &app.world).is_some());

// after dropping the handle, next call to `tick` will prepare the assets for removal.
drop(handle);
tick(&mut world);
assert_eq!(LoadState::Loaded, get_load_state(&weak_handle, &world));
assert!(get_asset(&weak_handle, &world).is_some());
app.update();
assert_eq!(LoadState::Loaded, get_load_state(&weak_handle, &app.world));
assert!(get_asset(&weak_handle, &app.world).is_some());

// second call to tick will actually remove the asset.
tick(&mut world);
assert_eq!(LoadState::Unloaded, get_load_state(&weak_handle, &world));
assert!(get_asset(&weak_handle, &world).is_none());
app.update();
assert_eq!(
LoadState::Unloaded,
get_load_state(&weak_handle, &app.world)
);
assert!(get_asset(&weak_handle, &app.world).is_none());

// finally, reload the asset
let handle = load_asset(path.clone(), &world);
assert_eq!(LoadState::Loading, get_load_state(&handle, &world));
tick(&mut world);
assert_eq!(LoadState::Loaded, get_load_state(&handle, &world));
assert!(get_asset(&handle, &world).is_some());
let handle = load_asset(path.clone(), &app.world);
assert_eq!(LoadState::Loading, get_load_state(&handle, &app.world));
app.update();
assert_eq!(LoadState::Loaded, get_load_state(&handle, &app.world));
assert!(get_asset(&handle, &app.world).is_some());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core/src/time/fixed_timestep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl Default for FixedTimestep {
fn default() -> Self {
Self {
state: LocalFixedTimestepState::default(),
internal_system: Box::new(Self::prepare_system.system()),
internal_system: Box::new(IntoSystem::into_system(Self::prepare_system)),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ impl<'w, 's, T: Fetch<'w, 's>> Fetch<'w, 's> for OptionFetch<T> {
/// }
/// }
/// }
/// # print_moving_objects_system.system();
/// # bevy_ecs::system::assert_is_system(print_moving_objects_system);
/// ```
#[derive(Clone)]
pub struct ChangeTrackers<T: Component> {
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/query/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ where
/// println!("{} is looking lovely today!", name.name);
/// }
/// }
/// # compliment_entity_system.system();
/// # bevy_ecs::system::assert_is_system(compliment_entity_system);
/// ```
pub struct With<T>(PhantomData<T>);

Expand Down Expand Up @@ -188,7 +188,7 @@ unsafe impl<T> ReadOnlyFetch for WithFetch<T> {}
/// println!("{} has no permit!", name.name);
/// }
/// }
/// # no_permit_system.system();
/// # bevy_ecs::system::assert_is_system(no_permit_system);
/// ```
pub struct Without<T>(PhantomData<T>);

Expand Down Expand Up @@ -317,7 +317,7 @@ unsafe impl<T> ReadOnlyFetch for WithoutFetch<T> {}
/// println!("Entity {:?} got a new style or color", entity);
/// }
/// }
/// # print_cool_entity_system.system();
/// # bevy_ecs::system::assert_is_system(print_cool_entity_system);
/// ```
pub struct Or<T>(pub T);

Expand Down Expand Up @@ -619,7 +619,7 @@ impl_tick_filter!(
/// }
/// }
///
/// # print_add_name_component.system();
/// # bevy_ecs::system::assert_is_system(print_add_name_component);
/// ```
Added,
/// The [`FetchState`] of [`Added`].
Expand Down Expand Up @@ -662,7 +662,7 @@ impl_tick_filter!(
/// }
/// }
///
/// # print_moving_objects_system.system();
/// # bevy_ecs::system::assert_is_system(print_moving_objects_system);
/// ```
Changed,
/// The [`FetchState`] of [`Changed`].
Expand Down
13 changes: 8 additions & 5 deletions crates/bevy_ecs/src/schedule/run_criteria.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ where
{
fn into(self) -> RunCriteriaDescriptorOrLabel {
RunCriteriaDescriptorOrLabel::Descriptor(new_run_criteria_descriptor(Box::new(
self.system(),
IntoSystem::into_system(self),
)))
}
}
Expand Down Expand Up @@ -333,19 +333,20 @@ where
S: IntoSystem<(), ShouldRun, Param>,
{
fn label(self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
new_run_criteria_descriptor(Box::new(self.system())).label(label)
new_run_criteria_descriptor(Box::new(IntoSystem::into_system(self))).label(label)
}

fn label_discard_if_duplicate(self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
new_run_criteria_descriptor(Box::new(self.system())).label_discard_if_duplicate(label)
new_run_criteria_descriptor(Box::new(IntoSystem::into_system(self)))
.label_discard_if_duplicate(label)
}

fn before(self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
new_run_criteria_descriptor(Box::new(self.system())).before(label)
new_run_criteria_descriptor(Box::new(IntoSystem::into_system(self))).before(label)
}

fn after(self, label: impl RunCriteriaLabel) -> RunCriteriaDescriptor {
new_run_criteria_descriptor(Box::new(self.system())).after(label)
new_run_criteria_descriptor(Box::new(IntoSystem::into_system(self))).after(label)
}
}

Expand All @@ -366,6 +367,8 @@ impl RunCriteria {

pub trait RunCriteriaPiping {
/// See [`RunCriteria::pipe()`].
// TODO: Support `IntoSystem` here instead, and stop using
// `IntoSystem::into_system` in the call sites
fn pipe(self, system: impl System<In = ShouldRun, Out = ShouldRun>) -> RunCriteriaDescriptor;
}

Expand Down
23 changes: 11 additions & 12 deletions crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,16 @@ impl SystemStage {
mut self,
system: S,
) -> Self {
self.set_run_criteria(system.system());
self.set_run_criteria(system);
self
}

pub fn set_run_criteria<Param, S: IntoSystem<(), ShouldRun, Param>>(
&mut self,
system: S,
) -> &mut Self {
self.stage_run_criteria.set(Box::new(system.system()));
self.stage_run_criteria
.set(Box::new(IntoSystem::into_system(system)));
self
}

Expand Down Expand Up @@ -1513,17 +1514,15 @@ mod tests {
.after("0")
.with_run_criteria(every_other_time.label("every other time")),
)
.with_system(make_parallel(2).label("2").after("1").with_run_criteria(
RunCriteria::pipe("every other time", IntoSystem::into_system(eot_piped)),
))
.with_system(
make_parallel(2)
.label("2")
.after("1")
.with_run_criteria(RunCriteria::pipe("every other time", eot_piped.system())),
)
.with_system(
make_parallel(3)
.label("3")
.after("2")
.with_run_criteria("every other time".pipe(eot_piped.system()).label("piped")),
make_parallel(3).label("3").after("2").with_run_criteria(
"every other time"
.pipe(IntoSystem::into_system(eot_piped))
.label("piped"),
),
)
.with_system(make_parallel(4).after("3").with_run_criteria("piped"));
for _ in 0..4 {
Expand Down
13 changes: 7 additions & 6 deletions crates/bevy_ecs/src/schedule/system_descriptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ where
S: IntoSystem<(), (), Params>,
{
fn into_descriptor(self) -> SystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).into_descriptor()
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).into_descriptor()
}
}

Expand Down Expand Up @@ -174,23 +174,24 @@ where
self,
run_criteria: impl IntoRunCriteria<Marker>,
) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).with_run_criteria(run_criteria)
new_parallel_descriptor(Box::new(IntoSystem::into_system(self)))
.with_run_criteria(run_criteria)
}

fn label(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).label(label)
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).label(label)
}

fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).before(label)
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).before(label)
}

fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).after(label)
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).after(label)
}

fn in_ambiguity_set(self, set: impl AmbiguitySetLabel) -> ParallelSystemDescriptor {
new_parallel_descriptor(Box::new(self.system())).in_ambiguity_set(set)
new_parallel_descriptor(Box::new(IntoSystem::into_system(self))).in_ambiguity_set(set)
}
}

Expand Down
Loading

0 comments on commit 6615b7b

Please sign in to comment.