Merge amethyst#1882

1882: Maintenance/update amethyst test r=distransient,jojolepro a=azriel91

## Description

Updated `amethyst_test::AmethystApplication` to take both `System` and `SystemDesc`

(currently sits on top of amethyst#1881 so that CI succeeds).

## Additions

* `AmethystApplication` takes in `SystemDesc`s through `with_system_desc`.
* `AmethystApplication::with_thread_local_desc` takes in `RunNowDesc`.

## Modifications

* `AmethystApplication` takes in a `System` instead of a closure for `with_system`.
* `AmethystApplication::with_thread_local` constraint relaxed to `RunNow` (previously `System`).

## PR Checklist

By placing an x in the boxes I certify that I have:

- [x] Updated the content of the book if this PR would make the book outdated.
- [x] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
- **n/a** Added unit tests for new code added in this PR.
- [x] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.

If this modified or created any rs files:

- [x] Ran `cargo +stable fmt --all`
- [x] Ran `cargo clippy --all --features "empty"`
- [x] Ran `cargo test --all --features "empty"`


Co-authored-by: Azriel Hoh <[email protected]>

Author: bors[bot]

amethyst_controls/src/systems.rs

@@ -230,13 +230,6 @@ impl<'a, 'b> SystemDesc<'a, 'b, CursorHideSystem> for CursorHideSystemDesc {
     fn build(self, world: &mut World) -> CursorHideSystem {
         <CursorHideSystem as System<'_>>::SystemData::setup(world);
 
-        let win = world.fetch::<Window>();
-
-        if let Err(err) = win.grab_cursor(true) {
-            log::error!("Unable to grab the cursor. Error: {:?}", err);
-        }
-        win.hide_cursor(true);
-
         CursorHideSystem::new()
     }
 }

amethyst_derive/tests/test.rs

@@ -108,7 +108,7 @@ mod tests {
     macro_rules! assert_prefab {
         ($prefab_type:ident, $prefab:expr, $assertion:expr) => {
             assert!(AmethystApplication::blank()
-                .with_system(
+                .with_system_desc(
                     PrefabLoaderSystemDesc::<$prefab_type>::default(),
                     "test_loader",
                     &[]

amethyst_gltf/Cargo.toml

@@ -23,7 +23,7 @@ amethyst_rendy = { path = "../amethyst_rendy", version = "0.2.0" }
 err-derive = "0.1"
 base64 = "0.10"
 fnv = "1"
-gltf = "0.12"
+gltf = "0.13"
 gfx = "0.17"
 hibitset = { version = "0.5.1", features = ["parallel"] }
 itertools = "0.7"

amethyst_test/src/amethyst_application.rs

@@ -2,7 +2,7 @@ use std::{any::Any, marker::PhantomData, panic, path::PathBuf, sync::Mutex};
 
 use amethyst::{
     self,
-    core::{transform::TransformBundle, EventReader, SystemBundle, SystemDesc},
+    core::{transform::TransformBundle, EventReader, RunNowDesc, SystemBundle, SystemDesc},
     ecs::prelude::*,
     error::Error,
     input::{BindingTypes, InputBundle},
@@ -17,8 +17,8 @@ use derivative::Derivative;
 use lazy_static::lazy_static;
 
 use crate::{
-    CustomDispatcherStateBuilder, FunctionState, GameUpdate, SequencerState, SystemInjectionBundle,
-    ThreadLocalInjectionBundle,
+    CustomDispatcherStateBuilder, FunctionState, GameUpdate, SequencerState,
+    SystemDescInjectionBundle, SystemInjectionBundle, ThreadLocalInjectionBundle,
 };
 
 type BundleAddFn = Box<
@@ -366,8 +366,8 @@ where
         self.resource_add_fns
             .push(Box::new(move |world: &mut World| {
                 let resource = resource_opt.take();
-                if resource.is_some() {
-                    world.insert(resource.unwrap());
+                if let Some(resource) = resource {
+                    world.insert(resource);
                 }
             }));
         self
@@ -389,41 +389,82 @@ where
         self
     }
 
+    /// Registers a `System` into this application's `GameData`.
+    ///
+    /// # Parameters
+    ///
+    /// * `system`: `System` to run.
+    /// * `name`: Name to register the system with, used for dependency ordering.
+    /// * `deps`: Names of systems that must run before this system.
+    pub fn with_system<S>(
+        self,
+        system: S,
+        name: &'static str,
+        deps: &'static [&'static str],
+    ) -> Self
+    where
+        S: for<'sys_local> System<'sys_local> + Send + 'static,
+    {
+        self.with_bundle_fn(move || SystemInjectionBundle::new(system, name, deps))
+    }
+
     /// Registers a `System` into this application's `GameData`.
     ///
     /// # Parameters
     ///
     /// * `system_desc`: Descriptor to instantiate the `System`.
     /// * `name`: Name to register the system with, used for dependency ordering.
     /// * `deps`: Names of systems that must run before this system.
-    pub fn with_system<N, SD, S>(self, system_desc: SD, name: N, deps: &[N]) -> Self
+    pub fn with_system_desc<SD, S>(
+        self,
+        system_desc: SD,
+        name: &'static str,
+        deps: &'static [&'static str],
+    ) -> Self
     where
-        N: Into<String> + Clone,
         SD: SystemDesc<'static, 'static, S> + Send + Sync + 'static,
         S: for<'sys_local> System<'sys_local> + Send + 'static,
     {
-        let name = name.into();
-        let deps = deps
-            .iter()
-            .map(|dep| dep.clone().into())
-            .collect::<Vec<String>>();
-        self.with_bundle_fn(move || SystemInjectionBundle::new(system_desc, name, deps))
+        self.with_bundle_fn(move || SystemDescInjectionBundle::new(system_desc, name, deps))
     }
 
     /// Registers a thread local `System` into this application's `GameData`.
     ///
     /// # Parameters
     ///
-    /// * `system_desc`: Descriptor to instantiate the thread local system.
-    pub fn with_thread_local<SD, S>(self, system_desc: SD) -> Self
+    /// * `run_now_desc`: Descriptor to instantiate the thread local system.
+    pub fn with_thread_local<RNDesc, RN>(self, run_now_desc: RNDesc) -> Self
     where
-        SD: SystemDesc<'static, 'static, S> + Send + Sync + 'static,
-        S: for<'sys_local> System<'sys_local> + Send + 'static,
-        // Ideally we can use the following lesser bound, but this would cause a duplication of
-        // traits and types which may not be worth it at this point in time.
-        // S: for<'sys_local> RunNow<'sys_local> + Send + 'static,
+        RNDesc: RunNowDesc<'static, 'static, RN> + Send + Sync + 'static,
+        RN: for<'sys_local> RunNow<'sys_local> + Send + 'static,
     {
-        self.with_bundle_fn(move || ThreadLocalInjectionBundle::new(system_desc))
+        self.with_bundle_fn(move || ThreadLocalInjectionBundle::new(run_now_desc))
+    }
+
+    /// Registers a `System` to run in a `CustomDispatcherState`.
+    ///
+    /// This will run the system once in a dedicated `State`, allowing you to inspect the effects of
+    /// the system after setting up the world to a desired state.
+    ///
+    /// # Parameters
+    ///
+    /// * `system`: `System` to run.
+    /// * `name`: Name to register the system with, used for dependency ordering.
+    /// * `deps`: Names of systems that must run before this system.
+    pub fn with_system_single<S>(
+        self,
+        system: S,
+        name: &'static str,
+        deps: &'static [&'static str],
+    ) -> Self
+    where
+        S: for<'sys_local> System<'sys_local> + Send + Sync + 'static,
+    {
+        self.with_state(move || {
+            CustomDispatcherStateBuilder::new()
+                .with_system(system, name, deps)
+                .build()
+        })
     }
 
     /// Registers a `System` to run in a `CustomDispatcherState`.
@@ -436,20 +477,19 @@ where
     /// * `system_desc`: Descriptor to instantiate the `System`.
     /// * `name`: Name to register the system with, used for dependency ordering.
     /// * `deps`: Names of systems that must run before this system.
-    pub fn with_system_single<N, SD, S>(self, system_desc: SD, name: N, deps: &[N]) -> Self
+    pub fn with_system_desc_single<SD, S>(
+        self,
+        system_desc: SD,
+        name: &'static str,
+        deps: &'static [&'static str],
+    ) -> Self
     where
-        N: Into<String> + Clone,
         SD: SystemDesc<'static, 'static, S> + Send + Sync + 'static,
         S: for<'sys_local> System<'sys_local> + Send + Sync + 'static,
     {
-        let name = name.into();
-        let deps = deps
-            .iter()
-            .map(|dep| dep.clone().into())
-            .collect::<Vec<String>>();
         self.with_state(move || {
             CustomDispatcherStateBuilder::new()
-                .with(system_desc, name, deps)
+                .with_system_desc(system_desc, name, deps)
                 .build()
         })
     }

amethyst_test/src/lib.rs

@@ -117,13 +117,12 @@
 //!
 //! ```rust,no_run
 //! # use amethyst::{
-//! #     core::{bundle::SystemBundle, SystemDesc},
-//! #     derive::SystemDesc,
+//! #     core::bundle::SystemBundle,
 //! #     ecs::prelude::*,
 //! #     prelude::*,
 //! # };
 //! #
-//! # #[derive(Debug, SystemDesc)]
+//! # #[derive(Debug)]
 //! # struct MySystem;
 //! #
 //! # impl<'s> System<'s> for MySystem {
@@ -174,17 +173,15 @@
 //! ```rust
 //! # use amethyst_test::prelude::*;
 //! # use amethyst::{
-//! #     core::{bundle::SystemBundle, SystemDesc},
-//! #     derive::SystemDesc,
+//! #     core::bundle::SystemBundle,
 //! #     ecs::prelude::*,
 //! #     prelude::*,
 //! # };
 //! #
 //! # #[derive(Debug)]
 //! # struct ApplicationResource;
 //! #
-//! # #[derive(Debug, SystemDesc)]
-//! # #[system_desc(insert(ApplicationResource))]
+//! # #[derive(Debug)]
 //! # struct MySystem;
 //! #
 //! # impl<'s> System<'s> for MySystem {
@@ -198,7 +195,8 @@
 //! # impl<'a, 'b> SystemBundle<'a, 'b> for MyBundle {
 //! #     fn build(self, world: &mut World, builder: &mut DispatcherBuilder<'a, 'b>)
 //! #     -> amethyst::Result<()> {
-//! #         builder.add(MySystem.build(world), "my_system", &[]);
+//! #         world.insert(ApplicationResource);
+//! #         builder.add(MySystem, "my_system", &[]);
 //! #         Ok(())
 //! #     }
 //! # }
@@ -224,8 +222,6 @@
 //! ```rust
 //! # use amethyst_test::prelude::*;
 //! # use amethyst::{
-//! #     core::SystemDesc,
-//! #     derive::SystemDesc,
 //! #     ecs::prelude::*,
 //! #     prelude::*,
 //! # };
@@ -236,7 +232,7 @@
 //! #     type Storage = DenseVecStorage<Self>;
 //! # }
 //! #
-//! # #[derive(Debug, SystemDesc)]
+//! # #[derive(Debug)]
 //! # struct MySystem;
 //! #
 //! # impl<'s> System<'s> for MySystem {
@@ -285,16 +281,14 @@
 //! ```rust
 //! # use amethyst_test::prelude::*;
 //! # use amethyst::{
-//! #     core::SystemDesc,
-//! #     derive::SystemDesc,
 //! #     ecs::prelude::*,
 //! #     prelude::*,
 //! # };
 //! #
 //! # // !Default
 //! # struct MyResource(pub i32);
 //! #
-//! # #[derive(Debug, SystemDesc)]
+//! # #[derive(Debug)]
 //! # struct MySystem;
 //! #
 //! # impl<'s> System<'s> for MySystem {
@@ -340,6 +334,7 @@ pub use crate::{
     },
 };
 pub(crate) use crate::{
+    system_desc_injection_bundle::SystemDescInjectionBundle,
     system_injection_bundle::SystemInjectionBundle,
     thread_local_injection_bundle::ThreadLocalInjectionBundle,
 };
@@ -350,5 +345,6 @@ mod fixture;
 mod game_update;
 pub mod prelude;
 mod state;
+mod system_desc_injection_bundle;
 mod system_injection_bundle;
 mod thread_local_injection_bundle;