Specs upgrade `World` initialization

(Azriel Hoh) #1

Problem

With the specs 0.15 upgrade, Systems now initialize fields inside their new function instead of System::setup:

System initialization: Before / After

Before:

pub struct MySystem {
    transform_events_id: Option<ReaderId<ComponentEvent>>,
}

impl<'s> System<'s> for MySystem {
    //..

    fn setup(&mut self, world: &mut World) {
        Self::SystemData::setup(world);
        let mut transforms = WriteStorage::<Transform>::fetch(&world);
        self.transform_events_id = Some(transforms.register_reader());
    }
}

After:

pub struct MySystem {
    transform_events_id: ReaderId<ComponentEvent>,
}

impl MySystem {
    fn new(world: &mut World) -> Self {
        <Self as System<'_>>::SystemData::setup(world);
        let mut transforms = WriteStorage::<Transform>::fetch(&world);
        let transform_events_id = Some(transforms.register_reader());

        Self { transform_events_id }
    }
}

impl<'s> System<'s> for MySystem {
    // ..
}

This is fine and dandy, until a system decides to use a resource normally initialized inside ApplicationBuilder::new, such as registering a reader for EventChannel<Event> (winit::Event).

The order of SystemData initialization has changed. Previously:

  • Systems are instantiated without SystemData initialization.
  • ApplicationBuilder::new is called, inserting resources with special limits.
  • System::setup is called, using those resources

With the upgrade:

  • Systems are instantiated, invoking SystemData initialization.
  • ApplicationBuilder::new is called, inserting resources with special limits, overwriting systems’ SystemData initialization.
  • System::setup is called, initializing whatever SystemData was not initialized in System::new (in case a System did not call SystemData::setup(world) in new).

Current State

In the upgrade, world is initialized before GameData, and later passed in to Application:

let mut world = World::new();
let game_data = GameDataBuilder::default().with_bundle(
    &mut world,
    RenderingBundle::<DefaultBackend>::new()
        // ..
)?;

let assets_dir = app_root.join("examples/assets/");
let mut game = Application::new(assets_dir, Pong, game_data, world)?;

Potential Solution

Solution 1: Additional AmethystWorldExt trait for all resources

This extends World to be instantiated with all resources from the current ApplicationBuilder:

use amethyst::{ecs::World, AmethystWorldExt};

let assets_dir = app_root.join("examples/assets/");
let world = World::with_application_resources(assets_dir)?;

let game_data = GameDataBuilder::default().with_bundle(
    &mut world,
    RenderingBundle::<DefaultBackend>::new()
        // ..
)?;

let mut game = Application::new(Pong, game_data, world)?;

Note:

  • assets_dir is now passed into the World trait ext constructor, not the Application.
  • The constructor returns amethyst::Result<World>.
Trait Implementation
use std::{env, path::Path, sync::Arc};

use log::debug;
use rayon::ThreadPoolBuilder;
use winit::Event;

use crate::{
    assets::Loader,
    callback_queue::CallbackQueue,
    core::{
        frame_limiter::FrameLimiter,
        shrev::EventChannel,
        timing::{Stopwatch, Time},
        ArcThreadPool, Named,
    },
    ecs::{World, WorldExt},
    game_data::DataDispose,
    state::TransEvent,
    state_event::StateEvent,
    ui::UiEvent,
};

/// Extends the `World` to be initialized with resources to run an Amethyst application.
///
/// # Examples
///
/// ```rust
/// use amethyst::{ecs::World, AmethystWorldExt};
///
/// fn main() -> amethyst::Result<()> {
///     let app_root = application_root_dir()?;
///     let assets_dir = app_root.join("examples/assets/");
///     let world = World::with_application_resources(assets_dir)?;
/// }
/// ```
pub trait AmethystWorldExt: private::Sealed {
    /// Returns a `World` with application resources.
    fn with_application_resources<T, P>(assets_dir: P) -> crate::Result<World>
    where
        T: DataDispose + 'static,
        P: AsRef<Path>;
}

// Disallow `AmethystWorldExt` from being implemented by other types.
mod private {
    use crate::ecs::World;

    pub trait Sealed {}
    impl Sealed for World {}
}

impl AmethystWorldExt for World {
    fn with_application_resources<T, P>(assets_dir: P) -> crate::Result<World>
    where
        T: DataDispose + 'static,
        P: AsRef<Path>,
    {
        let mut world = World::new();

        let thread_count: Option<usize> = env::var("AMETHYST_NUM_THREADS")
            .as_ref()
            .map(|s| {
                s.as_str()
                    .parse()
                    .expect("AMETHYST_NUM_THREADS was provided but is not a valid number!")
            })
            .ok();

        let thread_pool_builder = ThreadPoolBuilder::new();
        #[cfg(feature = "profiler")]
        let thread_pool_builder = thread_pool_builder.start_handler(|_index| {
            register_thread_with_profiler();
        });
        let pool: ArcThreadPool;
        if let Some(thread_count) = thread_count {
            debug!("Running Amethyst with fixed thread pool: {}", thread_count);
            pool = thread_pool_builder
                .num_threads(thread_count)
                .build()
                .map(Arc::new)?;
        } else {
            pool = thread_pool_builder.build().map(Arc::new)?;
        }
        world.insert(Loader::new(assets_dir.as_ref().to_owned(), pool.clone()));
        world.insert(pool);
        world.insert(EventChannel::<Event>::with_capacity(2000));
        world.insert(EventChannel::<UiEvent>::with_capacity(40));
        world.insert(EventChannel::<TransEvent<T, StateEvent>>::with_capacity(2));
        world.insert(FrameLimiter::default());
        world.insert(Stopwatch::default());
        world.insert(Time::default());
        world.insert(CallbackQueue::default());

        world.register::<Named>();

        Ok(world)
    }
}

Solution 2: Additional AmethystWorldExt trait for non-heavy-loading resources

Similar to solution 1, but exclude the Loader and thread pool. Systems shouldn’t hold a reference to the thread pool, but instead just pull it via SystemData.

Solution 3: your solution here

…

2 Likes
(Paweł Grabarz) #2

The whole problem really comes from the fact that systems are created immediately on the spot, instead of being created in controlled manner inside the dispatcher builder.

This isn’t really a bad thing. For minimal changes, i would pick solution 1. The problem with that is that it leaves us with this ugly &mut world everywhere which adds a lot of nose in the declaration. There is a way to fix both at the same time, but I’m still not sure how to execute that idea without adding a lot of noise on the system end.

Anyway, it is as follows (the most basic general idea):

Split systems into two traits responsible for “setup” and… uh… “systeming” :smiley:

Let’s call those traits SystemDesc and System for now (this is btw exactly what we have for render graph).

Our GameDataBuilder would receive instances of SystemDesc (via generic or trait object), and can create actual system from it. It would be defined something like this

trait SystemDesc {
    fn build<'a>(self: Box<Self>, world: &mut World) -> Box<dyn SystemExecSend<'a>>;
}

That SystemExecSend is just a shred way to turn systems into trait objects for storage and execution.

Now you can put all your setup business inside that build implementation and create a system from it.

The downside is that now we have to create two structs for every system and implement a trait for each of them. But i believe this can be reduced with clever defaults or some macros in the worst case.

I would play with something along those lines for easy setup for systems that don’t need extra variables for setup.

impl<'a, S: System<'a> + Default> SystemDesc for S {
    fn build<'a>(self: Box<Self>, world: &mut World) -> Box<dyn SystemExecSend<'a>> {
        Box::new(S::default())
    }
}

Additionally, we could have a SystemWithSetup kind of trait that keeps the old method as it was and implements SystemDesc to basically emulate the old behaviour. This is fine for all cases where you only want to add some resources to the world or something.

2 Likes
(Azriel Hoh) #3

Solution 1 is implemented on https://github.com/jojolepro/amethyst/pull/24, but it may be easier to implement the SystemBuilder plugin system from current master.